diff mbox

[01/12] regcache: Add functions suitable for regset_supply/collect.

Message ID 1401122208-2481-2-git-send-email-arnez@linux.vnet.ibm.com
State New
Headers show

Commit Message

Andreas Arnez May 26, 2014, 4:36 p.m. UTC
These functions are intended to suit all targets that don't require too
special logic in their regset supply/collect methods.  Having such
generic functions helps reducing target-specific complexity.

gdb/
	* regcache.c: Include "regset.h".
	(regcache_supply_regset, regcache_collect_regset): New functions.
	* regcache.h (struct regcache_map_entry): New structure.
	(REGCACHE_MAP_SKIP_BYTES): New enum value.
	(regcache_supply_regset, regcache_collect_regset): New prototypes.
---
 gdb/regcache.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/regcache.h | 38 +++++++++++++++++++++++++++++++++
 2 files changed, 104 insertions(+)

Comments

Yao Qi May 27, 2014, 2:46 a.m. UTC | #1
On 05/27/2014 12:36 AM, Andreas Arnez wrote:
> +/* Mapping between register numbers and offsets in a buffer, for use
> +   in the '*regset' functions below.  The first entry in a map refers
> +   to offset 0, and each entry increases the offset by its size.

Except that 'regno' is REGCACHE_MAP_SKIP_BYTES.  When 'regno' is
REGCACHE_MAP_SKIP_BYTES, 'count' is the increased offset by bytes, IIUC.

> +/* Transfer a set of registers (as described by REGSET) between
> +   REGCACHE and BUF.  If REGNUM == -1, transfer all registers
> +   belonging to the regset, otherwise just the register numbered
> +   REGNUM.  The REGSET's 'descr' field must point to an array of
> +   'struct regcache_map_entry'.

IWBN to update the comments to 'descr' field, and go a step further,
rename field 'descr'.

> +
> +   These functions are suitable for the 'regset_supply' and
> +   'regset_collect' fields in a regset structure.
> +  */

"*/" should go to the previous line.
Andreas Arnez May 27, 2014, 11:53 a.m. UTC | #2
On Tue, May 27 2014, Yao Qi wrote:

> On 05/27/2014 12:36 AM, Andreas Arnez wrote:
>> +/* Mapping between register numbers and offsets in a buffer, for use
>> +   in the '*regset' functions below.  The first entry in a map refers
>> +   to offset 0, and each entry increases the offset by its size.
>
> Except that 'regno' is REGCACHE_MAP_SKIP_BYTES.  When 'regno' is
> REGCACHE_MAP_SKIP_BYTES, 'count' is the increased offset by bytes, IIUC.

Correct.  Maybe it's better to rephrase the whole comment like this:

/* Mapping between register numbers and offsets in a buffer, for use
   in the '*regset' functions below.  In an array of
   'regcache_map_entry' each element is interpreted like follows:

   - If 'regno' is a register number: Map register 'regno' to the
     current offset (starting with 0) and increase the current offset
     by the register's size.  Repeat this with consecutive register
     numbers up to 'regno+count-1'.

   - If 'regno' has the special value REGCACHE_MAP_SKIP_BYTES: Add
     'count' to the current offset.

   - If count=0: End of the map.  */

>
>> +/* Transfer a set of registers (as described by REGSET) between
>> +   REGCACHE and BUF.  If REGNUM == -1, transfer all registers
>> +   belonging to the regset, otherwise just the register numbered
>> +   REGNUM.  The REGSET's 'descr' field must point to an array of
>> +   'struct regcache_map_entry'.
>
> IWBN to update the comments to 'descr' field, and go a step further,
> rename field 'descr'.

With the new name being something like 'map' or 'regmap', I guess?  If
that's what you mean, I tend to agree, and I could provide a separate
patch for that.

>> +
>> +   These functions are suitable for the 'regset_supply' and
>> +   'regset_collect' fields in a regset structure.
>> +  */
>
> "*/" should go to the previous line.

Done.
Yao Qi May 27, 2014, 12:20 p.m. UTC | #3
On 05/27/2014 07:53 PM, Andreas Arnez wrote:
> Correct.  Maybe it's better to rephrase the whole comment like this:
> 

Yes, that is much better.

> /* Mapping between register numbers and offsets in a buffer, for use
>    in the '*regset' functions below.  In an array of
>    'regcache_map_entry' each element is interpreted like follows:
> 
>    - If 'regno' is a register number: Map register 'regno' to the
>      current offset (starting with 0) and increase the current offset
>      by the register's size.  Repeat this with consecutive register
>      numbers up to 'regno+count-1'.
> 
>    - If 'regno' has the special value REGCACHE_MAP_SKIP_BYTES: Add
>      'count' to the current offset.

Nit: I'd say "If 'regno' is REGCACHE_MAP_SKIP_BYTES, 'count' is the
increased offset".  This is just my suggestion, which may be worse
than yours.

> 
>    - If count=0: End of the map.  */
> 
>> >
>>> >> +/* Transfer a set of registers (as described by REGSET) between
>>> >> +   REGCACHE and BUF.  If REGNUM == -1, transfer all registers
>>> >> +   belonging to the regset, otherwise just the register numbered
>>> >> +   REGNUM.  The REGSET's 'descr' field must point to an array of
>>> >> +   'struct regcache_map_entry'.
>> >
>> > IWBN to update the comments to 'descr' field, and go a step further,
>> > rename field 'descr'.
> With the new name being something like 'map' or 'regmap', I guess?  If
> that's what you mean, I tend to agree, and I could provide a separate
> patch for that.
> 

'regmap' sounds good to me.  I don't have other comments.
Andreas Arnez May 27, 2014, 2:21 p.m. UTC | #4
On Tue, May 27 2014, Yao Qi wrote:

> On 05/27/2014 07:53 PM, Andreas Arnez wrote:
>> Correct.  Maybe it's better to rephrase the whole comment like this:
>> 
>
> Yes, that is much better.

Good -- thanks for pointing this out.

>
>> /* Mapping between register numbers and offsets in a buffer, for use
>>    in the '*regset' functions below.  In an array of
>>    'regcache_map_entry' each element is interpreted like follows:
>> 
>>    - If 'regno' is a register number: Map register 'regno' to the
>>      current offset (starting with 0) and increase the current offset
>>      by the register's size.  Repeat this with consecutive register
>>      numbers up to 'regno+count-1'.
>> 
>>    - If 'regno' has the special value REGCACHE_MAP_SKIP_BYTES: Add
>>      'count' to the current offset.
>
> Nit: I'd say "If 'regno' is REGCACHE_MAP_SKIP_BYTES, 'count' is the
> increased offset".  This is just my suggestion, which may be worse
> than yours.

I agree with simplifying the phrase "has the special value" to "is".
For consistency I'd like to stick to the term "current offset" from the
previous bullet.  Also, I intentionally avoid saying "increase", because
the current implementation allows 'count' to be negative.  So I'll just
shorten to:

  If 'regno' is REGCACHE_MAP_SKIP_BYTES: Add 'count' to the current
  offset.

>
>> 
>>    - If count=0: End of the map.  */
>> 
>>> >
>>>> >> +/* Transfer a set of registers (as described by REGSET) between
>>>> >> +   REGCACHE and BUF.  If REGNUM == -1, transfer all registers
>>>> >> +   belonging to the regset, otherwise just the register numbered
>>>> >> +   REGNUM.  The REGSET's 'descr' field must point to an array of
>>>> >> +   'struct regcache_map_entry'.
>>> >
>>> > IWBN to update the comments to 'descr' field, and go a step further,
>>> > rename field 'descr'.
>> With the new name being something like 'map' or 'regmap', I guess?  If
>> that's what you mean, I tend to agree, and I could provide a separate
>> patch for that.
>> 
>
> 'regmap' sounds good to me.  I don't have other comments.

OK, I'll post a separate patch for that.
diff mbox

Patch

diff --git a/gdb/regcache.c b/gdb/regcache.c
index 8b588c6..b38031e 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -30,6 +30,7 @@ 
 #include "exceptions.h"
 #include "remote.h"
 #include "valprint.h"
+#include "regset.h"
 
 /*
  * DATA STRUCTURE
@@ -1031,6 +1032,71 @@  regcache_raw_collect (const struct regcache *regcache, int regnum, void *buf)
   memcpy (buf, regbuf, size);
 }
 
+/* Supply register REGNUM from BUF to REGCACHE, using the register map
+   in REGSET.  If REGNUM is -1, do this for all registers in
+   REGSET.  */
+
+void
+regcache_supply_regset (const struct regset *regset,
+			struct regcache *regcache,
+			int regnum, const void *buf, size_t size)
+{
+  const struct regcache_map_entry *map = regset->descr;
+  int offs = 0;
+  int i, count;
+
+  for (i = 0; (count = map[i].count) != 0; i++)
+    {
+      int regno = map[i].regno;
+
+      if (regno == REGCACHE_MAP_SKIP_BYTES)
+	offs += count;
+      else
+	for (; count--; regno++)
+	  {
+	    int new_offs = offs + regcache->descr->sizeof_register[regno];
+
+	    if (new_offs <= size && (regnum == -1 || regnum == regno))
+	      regcache_raw_supply (regcache, regno,
+				   buf ? (const gdb_byte *) buf + offs
+				   : NULL);
+	    offs = new_offs;
+	  }
+    }
+}
+
+/* Collect register REGNUM from REGCACHE to BUF, using the register
+   map in REGSET.  If REGNUM is -1, do this for all registers in
+   REGSET.  */
+
+void
+regcache_collect_regset (const struct regset *regset,
+			 const struct regcache *regcache,
+			 int regnum, void *buf, size_t size)
+{
+  const struct regcache_map_entry *map = regset->descr;
+  int offs = 0;
+  int i, count;
+
+  for (i = 0; (count = map[i].count) != 0; i++)
+    {
+      int regno = map[i].regno;
+
+      if (regno == REGCACHE_MAP_SKIP_BYTES)
+	offs += count;
+      else
+	for (; count--; regno++)
+	  {
+	    int new_offs = offs + regcache->descr->sizeof_register[regno];
+
+	    if (new_offs <= size && (regnum == -1 || regnum == regno))
+	      regcache_raw_collect (regcache, regno,
+				    (gdb_byte *) buf + offs);
+	    offs = new_offs;
+	  }
+    }
+}
+
 
 /* Special handling for register PC.  */
 
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 8423f57..cc2bda8 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -147,6 +147,44 @@  extern void regcache_raw_supply (struct regcache *regcache,
 extern void regcache_raw_collect (const struct regcache *regcache,
 				  int regnum, void *buf);
 
+/* Mapping between register numbers and offsets in a buffer, for use
+   in the '*regset' functions below.  The first entry in a map refers
+   to offset 0, and each entry increases the offset by its size.
+   Specifying count=N and regno=R is equivalent to N consecutive
+   entries with count=1 and with regno ranging from R to R+N-1.  The
+   end of the map is marked with a dummy entry having count=0.  */
+
+struct regcache_map_entry
+{
+  int count;
+  int regno;
+};
+
+/* Special value for the 'regno' field in the struct above.  */
+
+enum
+  {
+    REGCACHE_MAP_SKIP_BYTES = -1,
+  };
+
+/* Transfer a set of registers (as described by REGSET) between
+   REGCACHE and BUF.  If REGNUM == -1, transfer all registers
+   belonging to the regset, otherwise just the register numbered
+   REGNUM.  The REGSET's 'descr' field must point to an array of
+   'struct regcache_map_entry'.
+
+   These functions are suitable for the 'regset_supply' and
+   'regset_collect' fields in a regset structure.
+  */
+
+extern void regcache_supply_regset (const struct regset *regset,
+				    struct regcache *regcache,
+				    int regnum, const void *buf,
+				    size_t size);
+extern void regcache_collect_regset (const struct regset *regset,
+				     const struct regcache *regcache,
+				     int regnum, void *buf, size_t size);
+
 
 /* The type of a register.  This function is slightly more efficient
    then its gdbarch vector counterpart since it returns a precomputed