[3/11] Add MIPS_MAX_REGISTER_SIZE (2/4)

Message ID 806B436F-EFA1-4200-AC54-9036D166C9B9@arm.com
State New, archived
Headers

Commit Message

Alan Hayward May 5, 2017, 8:04 a.m. UTC
  > On 11 Apr 2017, at 16:37, Yao Qi <qiyaoltc@gmail.com> wrote:
> 
> Alan Hayward <Alan.Hayward@arm.com> writes:
> 
> Hi Alan,
> There are different ways of getting rid of MAX_REGISTER_SIZE, let us try
> some simple approaches first.  Some uses of MAX_REGISTER_SIZE still
> can't be removed, but let us start from easy part.
> 

Patch split into multiple parts.

This part adds two new regcache functions: raw_supply_signed and 
raw_collect_signed.

Tested on a --enable-targets=all build using make check with board files
unix and native-gdbserver.
I do not have a MIPS machine to test on.

Ok to commit?

2017-05-05  Alan Hayward  <alan.hayward@arm.com>

	* mips-fbsd-tdep.c (mips_fbsd_supply_reg): Use raw_supply_signed.
	(mips_fbsd_collect_reg): Use raw_collect_signed.
	* mips-linux-tdep.c (supply_32bit_reg): Use raw_supply_signed.
	(mips64_fill_gregset): Use raw_collect_signed.
	(mips64_fill_fpregset): Use raw_supply_signed.
	* regcache.c (regcache::raw_supply_signed): New function.
	(regcache::raw_collect_signed): New function.
	* regcache.h (regcache::raw_supply_signed): New declaration.
	(regcache::raw_collect_signed): New declaration.


Last login: Fri May  5 08:55:03 on ttys000
alahay01@C02N40NHFH04:~
[08:56:55] $ cat Downloads/rb7439.patch
  

Comments

John Baldwin May 5, 2017, 7:32 p.m. UTC | #1
On Friday, May 05, 2017 08:04:04 AM Alan Hayward wrote:
> 
> > On 11 Apr 2017, at 16:37, Yao Qi <qiyaoltc@gmail.com> wrote:
> > 
> > Alan Hayward <Alan.Hayward@arm.com> writes:
> > 
> > Hi Alan,
> > There are different ways of getting rid of MAX_REGISTER_SIZE, let us try
> > some simple approaches first.  Some uses of MAX_REGISTER_SIZE still
> > can't be removed, but let us start from easy part.
> > 
> 
> Patch split into multiple parts.
> 
> This part adds two new regcache functions: raw_supply_signed and 
> raw_collect_signed.
> 
> Tested on a --enable-targets=all build using make check with board files
> unix and native-gdbserver.
> I do not have a MIPS machine to test on.
> 
> Ok to commit?

mips-fbsd-tdep.c changes look ok to me.
  
Yao Qi May 12, 2017, 8:53 a.m. UTC | #2
Alan Hayward <Alan.Hayward@arm.com> writes:

>  }
>
> +/* Supply register REGNUM, whose contents are stored in signed VAL, to
> +   REGCACHE.  */
> +
> +void
> +regcache::raw_supply_signed (int regnum, LONGEST val)

The unsigned version of this function is also needed, because I see such
pattern also exists,

      store_unsigned_integer (buf, 8, byte_order, sp + offset);
      regcache_raw_supply (regcache, AMD64_RSP_REGNUM, buf);

this leads me thinking we need to use function template to define
functions for both LONGEST and ULONGEST.

Secondly, this method can be named as raw_supply.

> +{
> +  enum bfd_endian byte_order = gdbarch_byte_order (m_descr->gdbarch);
> +  gdb_byte *regbuf;
> +  size_t size;
> +
> +  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
> +  gdb_assert (!m_readonly_p);
> +
> +  regbuf = register_buffer (regnum);
> +  size = m_descr->sizeof_register[regnum];
> +
> +  store_signed_integer (regbuf, size, byte_order, val);
> +  m_register_status[regnum] = REG_VALID;
> +}
> +
>  /* Collect register REGNUM from REGCACHE and store its contents in BUF.  */
>
>  void
> @@ -1251,6 +1271,23 @@ regcache::raw_collect (int regnum, void *buf) const
>    memcpy (buf, regbuf, size);
>  }
>
> +/* Collect register REGNUM from REGCACHE and extract its contents into a signed

This line is too long.

> +   LONGEST.  */
> +
> +LONGEST
> +regcache::raw_collect_signed (int regnum) const

We can define this method like this,

template<typename T>
using LongType = typename std::enable_if<(std::is_same<T, LONGEST>::value
					  || std::is_same<T, ULONGEST>::value),
					 T>::type;

  template<typename T>
  LongType<T> raw_collect (int regnum) const
  {
    ....
    if (std::is_signed<T>::value)
      return extract_signed_integer (regbuf, size, byte_order);
    else
      return extract_unsigned_integer (regbuf, size, byte_order);
  }

> +{
> +  enum bfd_endian byte_order = gdbarch_byte_order (m_descr->gdbarch);
> +  const gdb_byte *regbuf;
> +  size_t size;
> +
> +  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
> +
> +  regbuf = register_buffer (regnum);
> +  size = m_descr->sizeof_register[regnum];
> +  return extract_signed_integer (regbuf, size, byte_order);
> +}

If you want, we can add a function template for extract_signed_integer
and extract_unsigned_integer.

template<typename T>
LongType<T>
extract_integer (const gdb_byte *addr, int len, enum bfd_endian byte_order)
{
  T retval;

  ...
  if (std::is_signed<T>::value)
    {
      ...
    }
  else
    {
      ...
    }
  return retval;
}

so, the raw_collect above becomes, 

  template<typename T>
  LongType<T> raw_collect (int regnum) const
  {
    ....
    return extract_integer<T> (regbuf, size, byte_order);
  }

and raw_read_{signed,unsigned}, cooked_read_{signed,unsigned} can be
merged as function template too.
  

Patch

diff --git a/gdb/mips-fbsd-tdep.c b/gdb/mips-fbsd-tdep.c
index 00fae0ec60ddc9e645d3236efe29f2f9e9ceab5c..4785bf9a115fa74fb7f48eac388a7e3df2f0304d 100644
--- a/gdb/mips-fbsd-tdep.c
+++ b/gdb/mips-fbsd-tdep.c
@@ -63,13 +63,10 @@  mips_fbsd_supply_reg (struct regcache *regcache, int regnum, const void *addr,
   else
     {
       enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-      gdb_byte buf[MAX_REGISTER_SIZE];
       LONGEST val;

       val = extract_signed_integer ((const gdb_byte *) addr, len, byte_order);
-      store_signed_integer (buf, register_size (gdbarch, regnum), byte_order,
-			    val);
-      regcache_raw_supply (regcache, regnum, buf);
+      regcache->raw_supply_signed (regnum, val);
     }
 }

@@ -90,12 +87,7 @@  mips_fbsd_collect_reg (const struct regcache *regcache, int regnum, void *addr,
   else
     {
       enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-      gdb_byte buf[MAX_REGISTER_SIZE];
-      LONGEST val;
-
-      regcache_raw_collect (regcache, regnum, buf);
-      val = extract_signed_integer (buf, register_size (gdbarch, regnum),
-				    byte_order);
+      LONGEST val = regcache->raw_collect_signed (regnum);
       store_signed_integer ((gdb_byte *) addr, len, byte_order, val);
     }
 }
diff --git a/gdb/mips-linux-tdep.c b/gdb/mips-linux-tdep.c
index 48a582a16c934abe6e8f87c46a6009649c606d49..5325d75420a71451b63cbadbf4e33d4fcf233fa4 100644
--- a/gdb/mips-linux-tdep.c
+++ b/gdb/mips-linux-tdep.c
@@ -118,11 +118,10 @@  supply_32bit_reg (struct regcache *regcache, int regnum, const void *addr)
 {
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  gdb_byte buf[MAX_REGISTER_SIZE];
-  store_signed_integer (buf, register_size (gdbarch, regnum), byte_order,
-			extract_signed_integer ((const gdb_byte *) addr, 4,
-						byte_order));
-  regcache_raw_supply (regcache, regnum, buf);
+  LONGEST val;
+
+  val = extract_signed_integer ((const gdb_byte *) addr, 4, byte_order);
+  regcache->raw_supply_signed (regnum, val);
 }

 /* Unpack an elf_gregset_t into GDB's register cache.  */
@@ -460,12 +459,7 @@  mips64_fill_gregset (const struct regcache *regcache,

   if (regaddr != -1)
     {
-      gdb_byte buf[MAX_REGISTER_SIZE];
-      LONGEST val;
-
-      regcache_raw_collect (regcache, regno, buf);
-      val = extract_signed_integer (buf, register_size (gdbarch, regno),
-				    byte_order);
+      LONGEST val = regcache->raw_collect_signed (regno);
       dst = regp + regaddr;
       store_signed_integer ((gdb_byte *) dst, 8, byte_order, val);
     }
@@ -564,23 +558,13 @@  mips64_fill_fpregset (const struct regcache *regcache,
     }
   else if (regno == mips_regnum (gdbarch)->fp_control_status)
     {
-      gdb_byte buf[MAX_REGISTER_SIZE];
-      LONGEST val;
-
-      regcache_raw_collect (regcache, regno, buf);
-      val = extract_signed_integer (buf, register_size (gdbarch, regno),
-				    byte_order);
+      LONGEST val = regcache->raw_collect_signed (regno);
       to = (gdb_byte *) (*fpregsetp + 32);
       store_signed_integer (to, 4, byte_order, val);
     }
   else if (regno == mips_regnum (gdbarch)->fp_implementation_revision)
     {
-      gdb_byte buf[MAX_REGISTER_SIZE];
-      LONGEST val;
-
-      regcache_raw_collect (regcache, regno, buf);
-      val = extract_signed_integer (buf, register_size (gdbarch, regno),
-				    byte_order);
+      LONGEST val = regcache->raw_collect_signed (regno);
       to = (gdb_byte *) (*fpregsetp + 32) + 4;
       store_signed_integer (to, 4, byte_order, val);
     }
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 4dcfccbac70f0f962bf5e5596d035fda42322795..76f5bdfdf6cefd6d963be950aefcc275de7a820b 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -294,10 +294,14 @@  public:

   void raw_collect (int regnum, void *buf) const;

+  LONGEST raw_collect_signed (int regnum) const;
+
   void raw_supply (int regnum, const void *buf);

   void raw_supply_zeroed (int regnum);

+  void raw_supply_signed (int regnum, LONGEST val);
+
   void raw_copy (int regnum, struct regcache *src_regcache);

   enum register_status get_register_status (int regnum) const;
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 957b265c28d929376c3b7d8c100ea355d8292b94..c9fbc997a3fa33fb576db7f57da4501043ff4e9d 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -1228,6 +1228,26 @@  regcache::raw_supply_zeroed (int regnum)
   m_register_status[regnum] = REG_VALID;
 }

+/* Supply register REGNUM, whose contents are stored in signed VAL, to
+   REGCACHE.  */
+
+void
+regcache::raw_supply_signed (int regnum, LONGEST val)
+{
+  enum bfd_endian byte_order = gdbarch_byte_order (m_descr->gdbarch);
+  gdb_byte *regbuf;
+  size_t size;
+
+  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
+  gdb_assert (!m_readonly_p);
+
+  regbuf = register_buffer (regnum);
+  size = m_descr->sizeof_register[regnum];
+
+  store_signed_integer (regbuf, size, byte_order, val);
+  m_register_status[regnum] = REG_VALID;
+}
+
 /* Collect register REGNUM from REGCACHE and store its contents in BUF.  */

 void
@@ -1251,6 +1271,23 @@  regcache::raw_collect (int regnum, void *buf) const
   memcpy (buf, regbuf, size);
 }

+/* Collect register REGNUM from REGCACHE and extract its contents into a signed
+   LONGEST.  */
+
+LONGEST
+regcache::raw_collect_signed (int regnum) const
+{
+  enum bfd_endian byte_order = gdbarch_byte_order (m_descr->gdbarch);
+  const gdb_byte *regbuf;
+  size_t size;
+
+  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
+
+  regbuf = register_buffer (regnum);
+  size = m_descr->sizeof_register[regnum];
+  return extract_signed_integer (regbuf, size, byte_order);
+}
+
 void
 regcache::raw_copy (int regnum, struct regcache *src_regcache)
 {