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

["Ulrich Weigand" <uweigand@de.ibm.com>]

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