Re: [RFA][2/5] New port: Cell BE SPU (valops.c fix)

Ulrich Weigand
2006-11-24T15:51:19Z

Daniel Jacobowitz wrote:
> On Thu, Nov 23, 2006 at 08:59:08PM +0100, Mark Kettenis wrote:
> > I actually think the problem is that you're thinking that s.x lives in
> > a register where it is actually s itself that lives in that register.
> > So VALUE_TO_REGISTER should be called for the struct itself, not its
> > char member.
>
> This is at least the second time recently I've seen someone want to go
> from value to parent value... I'm a little worried about consistency
> between the two (e.g. do you update value_contents of one? both?) but
> it may be that we need to link them to do this right. Or, just not use
> lval_register for struct members / array subscripting. Which may be
> the same thing in the end?

The following patch attemps to solve this problem by adding a new field
VALUE_REGTYPE to the value structure. It is non-NULL if and only if the
value is an lvalue refering to a (subfield of a) register that uses
specical conversion functions.

VALUE_REGTYPE is set in value_from_register if CONVERT_REGISTER_P
return true; it is set to the type used for this access. It is copied
across value_copy, value_primitive_field, and value_subscripted_rvalue;
note that always the original type remains unchanged.

This allows value_assign to use that original type of the full
register contents to perform a read/modify/write cycle on a register
that needs special conversion functions.

The patch fixes both the bitfield test cases, and the explicit access
to a register using its builtin_type_vec128 type. Tested with no
regressions on spu-elf.

Is this a reasonable approach?

Bye,
Ulrich

ChangeLog:

* value.h (VALUE_REGTYPE): New macro.
(deprecated_value_regtype_hack): Add prototype.
* value.c (struct value): New field regtype.
(deprecated_value_regtype_hack): New function.
(value_copy): Copy VALUE_REGTYPE.
(value_primitive_field): Likewise.
* valarith.c (value_subscripted_rvalue): Copy VALUE_REGTYPE.
* findvar.c (value_from_register): Set VALUE_REGTYPE for values
from register with special conversion functions.
* valops.c (value_assign): When assigning to a register with
special conversion functions, do a read/modify/write cycle
using type VALUE_REGTYPE.

Index: gdb/findvar.c
===================================================================
RCS file: /cvs/src/src/gdb/findvar.c,v
retrieving revision 1.96
diff -u -p -r1.96 findvar.c
--- gdb/findvar.c 22 Nov 2006 13:44:45 -0000 1.96
+++ gdb/findvar.c 24 Nov 2006 14:25:51 -0000
@@ -648,6 +648,7 @@ value_from_register (struct type *type,
VALUE_LVAL (v) = lval_register;
VALUE_FRAME_ID (v) = get_frame_id (frame);
VALUE_REGNUM (v) = regnum;
+ VALUE_REGTYPE (v) = type;
}
else
{
Index: gdb/valarith.c
===================================================================
RCS file: /cvs/src/src/gdb/valarith.c,v
retrieving revision 1.45
diff -u -p -r1.45 valarith.c
--- gdb/valarith.c 24 Jan 2006 21:21:12 -0000 1.45
+++ gdb/valarith.c 24 Nov 2006 14:25:53 -0000
@@ -280,6 +280,7 @@ value_subscripted_rvalue (struct value *
VALUE_LVAL (v) = VALUE_LVAL (array);
VALUE_ADDRESS (v) = VALUE_ADDRESS (array);
VALUE_REGNUM (v) = VALUE_REGNUM (array);
+ VALUE_REGTYPE (v) = VALUE_REGTYPE (array);
VALUE_FRAME_ID (v) = VALUE_FRAME_ID (array);
set_value_offset (v, value_offset (array) + elt_offs);
return v;
Index: gdb/valops.c
===================================================================
RCS file: /cvs/src/src/gdb/valops.c,v
retrieving revision 1.165
diff -u -p -r1.165 valops.c
--- gdb/valops.c 9 Oct 2006 19:28:14 -0000 1.165
+++ gdb/valops.c 24 Nov 2006 14:25:53 -0000
@@ -643,12 +643,31 @@ value_assign (struct value *toval, struc
error (_("Value being assigned to is no longer active."));

if (VALUE_LVAL (toval) == lval_register
- && CONVERT_REGISTER_P (VALUE_REGNUM (toval), type))
+ && VALUE_REGTYPE (toval) != NULL
+ && CONVERT_REGISTER_P (value_reg, VALUE_REGTYPE (toval)))
{
- /* If TOVAL is a special machine register requiring
+ /* TOVAL is a special machine register requiring
conversion of program values to a special raw format. */
- VALUE_TO_REGISTER (frame, VALUE_REGNUM (toval),
- type, value_contents (fromval));
+
+ /* Determine type of full register contents. Note that TOVAL
+ may refer to a subfield of the full register. */
+ struct type *regtype = VALUE_REGTYPE (toval);
+ gdb_byte *buffer = alloca (TYPE_LENGTH (regtype));
+
+ /* Copy in old register contents. */
+ REGISTER_TO_VALUE (frame, value_reg, regtype, buffer);
+
+ /* Modify what needs to be modified. */
+ if (value_bitsize (toval))
+ modify_field (buffer + value_offset (toval),
+ value_as_long (fromval),
+ value_bitpos (toval), value_bitsize (toval));
+ else
+ memcpy (buffer + value_offset (toval),
+ value_contents (fromval), TYPE_LENGTH (type));
+
+ /* Write back the new register contents. */
+ VALUE_TO_REGISTER (frame, value_reg, regtype, buffer);
}
else
{
Index: gdb/value.c
===================================================================
RCS file: /cvs/src/src/gdb/value.c,v
retrieving revision 1.36
diff -u -p -r1.36 value.c
--- gdb/value.c 31 Mar 2006 10:36:18 -0000 1.36
+++ gdb/value.c 24 Nov 2006 14:25:53 -0000
@@ -135,6 +135,9 @@ struct value
list. */
struct value *next;

+ /* Full type of register contents if the value is from a register. */
+ struct type *regtype;
+
/* Register number if the value is from a register. */
short regnum;

@@ -444,6 +447,12 @@ deprecated_value_regnum_hack (struct val
return &value->regnum;
}

+struct type **
+deprecated_value_regtype_hack (struct value *value)
+{
+ return &value->regtype;
+}
+
int
deprecated_value_modifiable (struct value *value)
{
@@ -557,6 +566,7 @@ value_copy (struct value *arg)
val->bitsize = arg->bitsize;
VALUE_FRAME_ID (val) = VALUE_FRAME_ID (arg);
VALUE_REGNUM (val) = VALUE_REGNUM (arg);
+ VALUE_REGTYPE (val) = VALUE_REGTYPE (arg);
val->lazy = arg->lazy;
val->optimized_out = arg->optimized_out;
val->embedded_offset = value_embedded_offset (arg);
@@ -1347,6 +1357,7 @@ value_primitive_field (struct value *arg
VALUE_LVAL (v) = lval_internalvar_component;
VALUE_ADDRESS (v) = VALUE_ADDRESS (arg1);
VALUE_REGNUM (v) = VALUE_REGNUM (arg1);
+ VALUE_REGTYPE (v) = VALUE_REGTYPE (arg1);
VALUE_FRAME_ID (v) = VALUE_FRAME_ID (arg1);
/* VALUE_OFFSET (v) = VALUE_OFFSET (arg1) + offset
+ TYPE_FIELD_BITPOS (arg_type, fieldno) / 8; */
Index: gdb/value.h
===================================================================
RCS file: /cvs/src/src/gdb/value.h,v
retrieving revision 1.93
diff -u -p -r1.93 value.h
--- gdb/value.h 22 Nov 2006 13:44:45 -0000 1.93
+++ gdb/value.h 24 Nov 2006 14:25:53 -0000
@@ -220,6 +220,10 @@ extern struct frame_id *deprecated_value
extern short *deprecated_value_regnum_hack (struct value *);
#define VALUE_REGNUM(val) (*deprecated_value_regnum_hack (val))

+/* Full type of register contents if the value is from a register. */
+extern struct type **deprecated_value_regtype_hack (struct value *);
+#define VALUE_REGTYPE(val) (*deprecated_value_regtype_hack (val))
+
/* Convert a REF to the object referenced. */

extern struct value *coerce_ref (struct value *value);

--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com