[PATCH 0/2] Fixes for some s390 fails with store.exp

[PATCH 0/2] Fixes for some s390 fails with store.exp

[Andreas Arnez]

Since vector register support was added to GDB, store.exp shows some
FAILs on a z13.  These are due to two different problems, each addressed
by a patch in this series.

Andreas Arnez (2):
  S390: Take value from sub-register if applicable
  Involve gdbarch in taking DWARF register pieces

 gdb/dwarf2loc.c       | 31 ++++++++++++++-----------------
 gdb/findvar.c         | 40 +++++++++++++++++++---------------------
 gdb/gdbarch.c         | 28 ++++++++++++++--------------
 gdb/gdbarch.h         | 15 +++++++--------
 gdb/gdbarch.sh        | 10 +++++-----
 gdb/s390-linux-tdep.c | 27 ++++++++++++++-------------
 gdb/spu-tdep.c        | 20 ++++++--------------
 gdb/value.h           |  6 ++----
 8 files changed, 81 insertions(+), 96 deletions(-)

-- 
2.5.0

[PATCH 1/2] S390: Take value from sub-register if applicable

[Andreas Arnez]

On an s390 target with vector registers store.exp emits various FAILs
while trying to access a variable located in an unwound floating-point
register.  In those cases the variable's DWARF location happens to be
DW_OP_reg24, which refers to FP register f8 or to vector register v8,
depending on whether the target has vector registers.  But while f8 is
call-saved and thus can be unwound by GDB, v8 is not.  Thus an uplevel
variable with such a location can neither be read nor written.

A similar situation occurs on an s390 (32-bit) target with high GPRs.
For that case we already have logic in s390_unwind_pseudo_register which
unwinds a full GPR by casting its lower half to the larger type.
However, that cast does not result in an lvalue, so an uplevel variable
based on such a location is readable, but not writable.  Again, this
results in various FAILs from store.exp.

Both issues are addressed with a change to the s390 implementation of
the gdbarch method 'value_from_register': If the given type is small
enough to fit in the appropriate sub-register, then take the value from
that sub-register instead of from the full register.

gdb/ChangeLog:

	* s390-linux-tdep.c (s390_value_from_register): Use sub-register
	if appropriate.
---
 gdb/s390-linux-tdep.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index fc57592..8b4efb1 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -518,12 +518,18 @@ s390_value_from_register (struct gdbarch *gdbarch, struct type *type,
 			  int regnum, struct frame_id frame_id)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
-  struct value *value = default_value_from_register (gdbarch, type,
-						     regnum, frame_id);
-  check_typedef (type);
+  unsigned int len = TYPE_LENGTH (check_typedef (type));
+  struct value *value;
 
-  if ((regnum >= S390_F0_REGNUM && regnum <= S390_F15_REGNUM
-       && TYPE_LENGTH (type) < 8)
+  /* If the value fits in a sub-register, use that instead.  */
+  if (regnum_is_vxr_full (tdep, regnum) && len <= 8)
+    regnum = regnum - tdep->v0_full_regnum + S390_F0_REGNUM;
+  else if (regnum_is_gpr_full (tdep, regnum) && len <= 4)
+    regnum = regnum - tdep->gpr_full_regnum + S390_R0_REGNUM;
+
+  value = default_value_from_register (gdbarch, type, regnum, frame_id);
+
+  if ((regnum >= S390_F0_REGNUM && regnum <= S390_F15_REGNUM && len < 8)
       || regnum_is_vxr_full (tdep, regnum)
       || (regnum >= S390_V16_REGNUM && regnum <= S390_V31_REGNUM))
     set_value_offset (value, 0);
-- 
2.5.0

[PATCH 2/2] Involve gdbarch in taking DWARF register pieces

[Andreas Arnez]

On s390, when a 'long double' variable is located in a pair of FP
registers, its DWARF location looks something like this:

     DW_OP_reg24 (f8); DW_OP_piece: 8; DW_OP_reg25 (f10); DW_OP_piece: 8

If the target has a vector facility, each of its FP registers is
embedded left-aligned in a vector register with the same identical DWARF
register number.  But GDB always takes a register piece from the "least
significant" end, which is exactly opposite from where the FP register
is embedded.  This mismatch causes a regression with store.exp.

This change fixes this issue by taking s390 vector register pieces from
the left end instead.  Since this requires a suitable gdbarch interface,
the gdbarch method 'value_from_register' is replaced by a new method
'register_part' with similar functionality, but a slightly different
interface.  This method is then invoked in read_pieced_value and
write_pieced_value.

gdb/ChangeLog:

	* gdbarch.sh (value_from_register): Remove.
	(register_part): New.
	* gdbarch.c: Regenerate.
	* gdbarch.h: Likewise.
	* dwarf2loc.c (read_pieced_value): Get arch-specific placement of
	a register piece from the new gdbarch method register_part.
	(write_pieced_value): Likewise.
	* value.h (default_value_from_register): Remove.
	(default_register_part): Declare.
	* findvar.c (default_value_from_register): Remove.
	(default_register_part): New.
	(value_from_register): Call gdbarch_register_part instead of
	gdbarch_value_from_register.
	(address_from_register): Likewise.
	* s390-linux-tdep.c (s390_value_from_register): Remove.
	(s390_register_part): New.
	(s390_gdbarch_init): Set new gdbarch method register_part instead
	of value_from_register.
	* spu-tdep.c (spu_value_from_register): Remove.
	(spu_register_part): New.
	(spu_gdbarch_init): Set new gdbarch method register_part instead
	of value_from_register.
---
 gdb/dwarf2loc.c       | 31 ++++++++++++++-----------------
 gdb/findvar.c         | 40 +++++++++++++++++++---------------------
 gdb/gdbarch.c         | 28 ++++++++++++++--------------
 gdb/gdbarch.h         | 15 +++++++--------
 gdb/gdbarch.sh        | 10 +++++-----
 gdb/s390-linux-tdep.c | 31 +++++++++++++------------------
 gdb/spu-tdep.c        | 20 ++++++--------------
 gdb/value.h           |  6 ++----
 8 files changed, 80 insertions(+), 101 deletions(-)

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index ba6ed42..270518c 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1715,6 +1715,7 @@ read_pieced_value (struct value *v)
   for (i = 0; i < c->n_pieces && offset < type_len; i++)
     {
       struct dwarf_expr_piece *p = &c->pieces[i];
+      size_t piece_bytes = (p->size + 7) / 8;
       size_t this_size, this_size_bits;
       long dest_offset_bits, source_offset_bits, source_offset;
       const gdb_byte *intermediate_buffer;
@@ -1759,17 +1760,13 @@ read_pieced_value (struct value *v)
 	    struct gdbarch *arch = get_frame_arch (frame);
 	    int gdb_regnum = dwarf_reg_to_regnum_or_error (arch, p->v.regno);
 	    int optim, unavail;
-	    int reg_offset = source_offset;
+	    int reg_offset = gdbarch_register_part (arch, piece_bytes,
+						    &gdb_regnum);
 
-	    if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG
-		&& this_size < register_size (arch, gdb_regnum))
-	      {
-		/* Big-endian, and we want less than full size.  */
-		reg_offset = register_size (arch, gdb_regnum) - this_size;
-		/* We want the lower-order THIS_SIZE_BITS of the bytes
-		   we extract from the register.  */
-		source_offset_bits += 8 * this_size - this_size_bits;
-	      }
+	    source_offset_bits += 8 * reg_offset;
+	    if (bits_big_endian)
+	      source_offset_bits += 8 * piece_bytes - this_size_bits;
+	    reg_offset = source_offset_bits / 8;
 
 	    if (!get_frame_register_bytes (frame, gdb_regnum, reg_offset,
 					   this_size, buffer,
@@ -1890,6 +1887,7 @@ write_pieced_value (struct value *to, struct value *from)
   for (i = 0; i < c->n_pieces && offset < type_len; i++)
     {
       struct dwarf_expr_piece *p = &c->pieces[i];
+      size_t piece_bytes = (p->size + 7) / 8;
       size_t this_size_bits, this_size;
       long dest_offset_bits, source_offset_bits, dest_offset, source_offset;
       int need_bitwise;
@@ -1941,14 +1939,13 @@ write_pieced_value (struct value *to, struct value *from)
 	  {
 	    struct gdbarch *arch = get_frame_arch (frame);
 	    int gdb_regnum = dwarf_reg_to_regnum_or_error (arch, p->v.regno);
-	    int reg_offset = dest_offset;
+	    int reg_offset = gdbarch_register_part (arch, piece_bytes,
+						    &gdb_regnum);
 
-	    if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG
-		&& this_size <= register_size (arch, gdb_regnum))
-	      {
-		/* Big-endian, and we want less than full size.  */
-		reg_offset = register_size (arch, gdb_regnum) - this_size;
-	      }
+	    source_offset_bits += 8 * reg_offset;
+	    if (bits_big_endian)
+	      source_offset_bits += 8 * piece_bytes - this_size_bits;
+	    reg_offset = source_offset_bits / 8;
 
 	    if (need_bitwise)
 	      {
diff --git a/gdb/findvar.c b/gdb/findvar.c
index a39d897..fca18a1 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -797,29 +797,19 @@ read_var_value (struct symbol *var, const struct block *var_block,
 
 /* Install default attributes for register values.  */
 
-struct value *
-default_value_from_register (struct gdbarch *gdbarch, struct type *type,
-                             int regnum, struct frame_id frame_id)
+int
+default_register_part (struct gdbarch *gdbarch, int len, int *regnum)
 {
-  int len = TYPE_LENGTH (type);
-  struct value *value = allocate_value (type);
-
-  VALUE_LVAL (value) = lval_register;
-  VALUE_FRAME_ID (value) = frame_id;
-  VALUE_REGNUM (value) = regnum;
-
   /* Any structure stored in more than one register will always be
      an integral number of registers.  Otherwise, you need to do
      some fiddling with the last register copied here for little
      endian machines.  */
   if (gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG
-      && len < register_size (gdbarch, regnum))
+      && len < register_size (gdbarch, *regnum))
     /* Big-endian, and we want less than full size.  */
-    set_value_offset (value, register_size (gdbarch, regnum) - len);
-  else
-    set_value_offset (value, 0);
+    return register_size (gdbarch, *regnum) - len;
 
-  return value;
+  return 0;
 }
 
 /* VALUE must be an lval_register value.  If regnum is the value's
@@ -877,6 +867,10 @@ value_from_register (struct type *type, int regnum, struct frame_info *frame)
   struct type *type1 = check_typedef (type);
   struct value *v;
 
+  v = allocate_value (type);
+  VALUE_LVAL (v) = lval_register;
+  VALUE_FRAME_ID (v) = get_frame_id (frame);
+
   if (gdbarch_convert_register_p (gdbarch, regnum, type1))
     {
       int optim, unavail, ok;
@@ -888,9 +882,6 @@ value_from_register (struct type *type, int regnum, struct frame_info *frame)
          the corresponding [integer] type (see Alpha).  The assumption
          is that gdbarch_register_to_value populates the entire value
          including the location.  */
-      v = allocate_value (type);
-      VALUE_LVAL (v) = lval_register;
-      VALUE_FRAME_ID (v) = get_frame_id (frame);
       VALUE_REGNUM (v) = regnum;
       ok = gdbarch_register_to_value (gdbarch, frame, regnum, type1,
 				      value_contents_raw (v), &optim,
@@ -907,8 +898,10 @@ value_from_register (struct type *type, int regnum, struct frame_info *frame)
   else
     {
       /* Construct the value.  */
-      v = gdbarch_value_from_register (gdbarch, type,
-				       regnum, get_frame_id (frame));
+      int offset = gdbarch_register_part (gdbarch, TYPE_LENGTH (type),
+					  &regnum);
+      VALUE_REGNUM (v) = regnum;
+      set_value_offset (v, offset);
 
       /* Get the data.  */
       read_frame_register_value (v, frame);
@@ -927,6 +920,7 @@ address_from_register (int regnum, struct frame_info *frame)
   struct type *type = builtin_type (gdbarch)->builtin_data_ptr;
   struct value *value;
   CORE_ADDR result;
+  int offset;
   int regnum_max_excl = (gdbarch_num_regs (gdbarch)
 			 + gdbarch_num_pseudo_regs (gdbarch));
 
@@ -962,7 +956,11 @@ address_from_register (int regnum, struct frame_info *frame)
       return unpack_long (type, buf);
     }
 
-  value = gdbarch_value_from_register (gdbarch, type, regnum, null_frame_id);
+  offset = gdbarch_register_part (gdbarch, TYPE_LENGTH (type), &regnum);
+  value = allocate_value (type);
+  VALUE_LVAL (value) = lval_register;
+  VALUE_REGNUM (value) = regnum;
+  set_value_offset (value, offset);
   read_frame_register_value (value, frame);
 
   if (value_optimized_out (value))
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index bd0b48c..fc7a723 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -216,7 +216,7 @@ struct gdbarch
   gdbarch_convert_register_p_ftype *convert_register_p;
   gdbarch_register_to_value_ftype *register_to_value;
   gdbarch_value_to_register_ftype *value_to_register;
-  gdbarch_value_from_register_ftype *value_from_register;
+  gdbarch_register_part_ftype *register_part;
   gdbarch_pointer_to_address_ftype *pointer_to_address;
   gdbarch_address_to_pointer_ftype *address_to_pointer;
   gdbarch_integer_to_address_ftype *integer_to_address;
@@ -393,7 +393,7 @@ gdbarch_alloc (const struct gdbarch_info *info,
   gdbarch->cannot_fetch_register = cannot_register_not;
   gdbarch->cannot_store_register = cannot_register_not;
   gdbarch->convert_register_p = generic_convert_register_p;
-  gdbarch->value_from_register = default_value_from_register;
+  gdbarch->register_part = default_register_part;
   gdbarch->pointer_to_address = unsigned_pointer_to_address;
   gdbarch->address_to_pointer = unsigned_address_to_pointer;
   gdbarch->return_in_first_hidden_param_p = default_return_in_first_hidden_param_p;
@@ -560,7 +560,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of cannot_store_register, invalid_p == 0 */
   /* Skip verify of get_longjmp_target, has predicate.  */
   /* Skip verify of convert_register_p, invalid_p == 0 */
-  /* Skip verify of value_from_register, invalid_p == 0 */
+  /* Skip verify of register_part, invalid_p == 0 */
   /* Skip verify of pointer_to_address, invalid_p == 0 */
   /* Skip verify of address_to_pointer, invalid_p == 0 */
   /* Skip verify of integer_to_address, has predicate.  */
@@ -1242,6 +1242,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: register_name = <%s>\n",
                       host_address_to_string (gdbarch->register_name));
   fprintf_unfiltered (file,
+                      "gdbarch_dump: register_part = <%s>\n",
+                      host_address_to_string (gdbarch->register_part));
+  fprintf_unfiltered (file,
                       "gdbarch_dump: register_reggroup_p = <%s>\n",
                       host_address_to_string (gdbarch->register_reggroup_p));
   fprintf_unfiltered (file,
@@ -1398,9 +1401,6 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: unwind_sp = <%s>\n",
                       host_address_to_string (gdbarch->unwind_sp));
   fprintf_unfiltered (file,
-                      "gdbarch_dump: value_from_register = <%s>\n",
-                      host_address_to_string (gdbarch->value_from_register));
-  fprintf_unfiltered (file,
                       "gdbarch_dump: value_to_register = <%s>\n",
                       host_address_to_string (gdbarch->value_to_register));
   fprintf_unfiltered (file,
@@ -2514,21 +2514,21 @@ set_gdbarch_value_to_register (struct gdbarch *gdbarch,
   gdbarch->value_to_register = value_to_register;
 }
 
-struct value *
-gdbarch_value_from_register (struct gdbarch *gdbarch, struct type *type, int regnum, struct frame_id frame_id)
+int
+gdbarch_register_part (struct gdbarch *gdbarch, int len, int *regnum)
 {
   gdb_assert (gdbarch != NULL);
-  gdb_assert (gdbarch->value_from_register != NULL);
+  gdb_assert (gdbarch->register_part != NULL);
   if (gdbarch_debug >= 2)
-    fprintf_unfiltered (gdb_stdlog, "gdbarch_value_from_register called\n");
-  return gdbarch->value_from_register (gdbarch, type, regnum, frame_id);
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_register_part called\n");
+  return gdbarch->register_part (gdbarch, len, regnum);
 }
 
 void
-set_gdbarch_value_from_register (struct gdbarch *gdbarch,
-                                 gdbarch_value_from_register_ftype value_from_register)
+set_gdbarch_register_part (struct gdbarch *gdbarch,
+                           gdbarch_register_part_ftype register_part)
 {
-  gdbarch->value_from_register = value_from_register;
+  gdbarch->register_part = register_part;
 }
 
 CORE_ADDR
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 252fc4b..ac93c96 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -450,14 +450,13 @@ typedef void (gdbarch_value_to_register_ftype) (struct frame_info *frame, int re
 extern void gdbarch_value_to_register (struct gdbarch *gdbarch, struct frame_info *frame, int regnum, struct type *type, const gdb_byte *buf);
 extern void set_gdbarch_value_to_register (struct gdbarch *gdbarch, gdbarch_value_to_register_ftype *value_to_register);
 
-/* Construct a value representing the contents of register REGNUM in
-   frame FRAME_ID, interpreted as type TYPE.  The routine needs to
-   allocate and return a struct value with all value attributes
-   (but not the value contents) filled in. */
-
-typedef struct value * (gdbarch_value_from_register_ftype) (struct gdbarch *gdbarch, struct type *type, int regnum, struct frame_id frame_id);
-extern struct value * gdbarch_value_from_register (struct gdbarch *gdbarch, struct type *type, int regnum, struct frame_id frame_id);
-extern void set_gdbarch_value_from_register (struct gdbarch *gdbarch, gdbarch_value_from_register_ftype *value_from_register);
+/* Locate a part of size LEN within register *REGNUM, possibly overwriting
+   *REGNUM.  Return the offset of the part within the (possibly adjusted)
+   register. */
+
+typedef int (gdbarch_register_part_ftype) (struct gdbarch *gdbarch, int len, int *regnum);
+extern int gdbarch_register_part (struct gdbarch *gdbarch, int len, int *regnum);
+extern void set_gdbarch_register_part (struct gdbarch *gdbarch, gdbarch_register_part_ftype *register_part);
 
 typedef CORE_ADDR (gdbarch_pointer_to_address_ftype) (struct gdbarch *gdbarch, struct type *type, const gdb_byte *buf);
 extern CORE_ADDR gdbarch_pointer_to_address (struct gdbarch *gdbarch, struct type *type, const gdb_byte *buf);
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 37f59b7..96bff43 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -506,11 +506,11 @@ v:int:believe_pcc_promotion:::::::
 m:int:convert_register_p:int regnum, struct type *type:regnum, type:0:generic_convert_register_p::0
 f:int:register_to_value:struct frame_info *frame, int regnum, struct type *type, gdb_byte *buf, int *optimizedp, int *unavailablep:frame, regnum, type, buf, optimizedp, unavailablep:0
 f:void:value_to_register:struct frame_info *frame, int regnum, struct type *type, const gdb_byte *buf:frame, regnum, type, buf:0
-# Construct a value representing the contents of register REGNUM in
-# frame FRAME_ID, interpreted as type TYPE.  The routine needs to
-# allocate and return a struct value with all value attributes
-# (but not the value contents) filled in.
-m:struct value *:value_from_register:struct type *type, int regnum, struct frame_id frame_id:type, regnum, frame_id::default_value_from_register::0
+
+# Locate a part of size LEN within register *REGNUM, possibly overwriting
+# *REGNUM.  Return the offset of the part within the (possibly adjusted)
+# register.
+m:int:register_part:int len, int *regnum:len, regnum::default_register_part::0
 #
 m:CORE_ADDR:pointer_to_address:struct type *type, const gdb_byte *buf:type, buf::unsigned_pointer_to_address::0
 m:void:address_to_pointer:struct type *type, gdb_byte *buf, CORE_ADDR addr:type, buf, addr::unsigned_address_to_pointer::0
diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index 8b4efb1..bfc2a8e 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -513,28 +513,23 @@ s390_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
    registers, even though we are otherwise a big-endian platform.  The
    same applies to a 'float' value within a vector.  */
 
-static struct value *
-s390_value_from_register (struct gdbarch *gdbarch, struct type *type,
-			  int regnum, struct frame_id frame_id)
+static int
+s390_register_part (struct gdbarch *gdbarch, int len, int *regnum)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
-  unsigned int len = TYPE_LENGTH (check_typedef (type));
-  struct value *value;
 
   /* If the value fits in a sub-register, use that instead.  */
-  if (regnum_is_vxr_full (tdep, regnum) && len <= 8)
-    regnum = regnum - tdep->v0_full_regnum + S390_F0_REGNUM;
-  else if (regnum_is_gpr_full (tdep, regnum) && len <= 4)
-    regnum = regnum - tdep->gpr_full_regnum + S390_R0_REGNUM;
-
-  value = default_value_from_register (gdbarch, type, regnum, frame_id);
-
-  if ((regnum >= S390_F0_REGNUM && regnum <= S390_F15_REGNUM && len < 8)
-      || regnum_is_vxr_full (tdep, regnum)
-      || (regnum >= S390_V16_REGNUM && regnum <= S390_V31_REGNUM))
-    set_value_offset (value, 0);
+  if (regnum_is_vxr_full (tdep, *regnum) && len <= 8)
+    *regnum = *regnum - tdep->v0_full_regnum + S390_F0_REGNUM;
+  else if (regnum_is_gpr_full (tdep, *regnum) && len <= 4)
+    *regnum = *regnum - tdep->gpr_full_regnum + S390_R0_REGNUM;
+
+  if ((*regnum >= S390_F0_REGNUM && *regnum <= S390_F15_REGNUM && len < 8)
+      || regnum_is_vxr_full (tdep, *regnum)
+      || (*regnum >= S390_V16_REGNUM && *regnum <= S390_V31_REGNUM))
+    return 0;
 
-  return value;
+  return default_register_part (gdbarch, len, regnum);
 }
 
 /* Register groups.  */
@@ -8005,7 +8000,7 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_fp0_regnum (gdbarch, S390_F0_REGNUM);
   set_gdbarch_stab_reg_to_regnum (gdbarch, s390_dwarf_reg_to_regnum);
   set_gdbarch_dwarf2_reg_to_regnum (gdbarch, s390_dwarf_reg_to_regnum);
-  set_gdbarch_value_from_register (gdbarch, s390_value_from_register);
+  set_gdbarch_register_part (gdbarch, s390_register_part);
   set_gdbarch_core_read_description (gdbarch, s390_core_read_description);
   set_gdbarch_iterate_over_regset_sections (gdbarch,
 					    s390_iterate_over_regset_sections);
diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index 8dad5c3..ef0923a 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -357,21 +357,13 @@ spu_ax_pseudo_register_push_stack (struct gdbarch *gdbarch,
 
 /* Value conversion -- access scalar values at the preferred slot.  */
 
-static struct value *
-spu_value_from_register (struct gdbarch *gdbarch, struct type *type,
-			 int regnum, struct frame_id frame_id)
+static int
+spu_register_part (struct gdbarch *gdbarch, int len, int *regnum)
 {
-  struct value *value = default_value_from_register (gdbarch, type,
-						     regnum, frame_id);
-  int len = TYPE_LENGTH (type);
-
-  if (regnum < SPU_NUM_GPRS && len < 16)
-    {
-      int preferred_slot = len < 4 ? 4 - len : 0;
-      set_value_offset (value, preferred_slot);
-    }
+  if (*regnum < SPU_NUM_GPRS && len < 16)
+    return len < 4 ? 4 - len : 0;
 
-  return value;
+  return default_register_part (gdbarch, len, regnum);
 }
 
 /* Register groups.  */
@@ -2738,7 +2730,7 @@ spu_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_register_type (gdbarch, spu_register_type);
   set_gdbarch_pseudo_register_read (gdbarch, spu_pseudo_register_read);
   set_gdbarch_pseudo_register_write (gdbarch, spu_pseudo_register_write);
-  set_gdbarch_value_from_register (gdbarch, spu_value_from_register);
+  set_gdbarch_register_part (gdbarch, spu_register_part);
   set_gdbarch_register_reggroup_p (gdbarch, spu_register_reggroup_p);
   set_gdbarch_dwarf2_reg_to_regnum (gdbarch, spu_dwarf_reg_to_regnum);
   set_gdbarch_ax_pseudo_register_collect
diff --git a/gdb/value.h b/gdb/value.h
index f8ec854..79addae 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -647,10 +647,8 @@ extern struct value *value_from_contents_and_address (struct type *,
 						      CORE_ADDR);
 extern struct value *value_from_contents (struct type *, const gdb_byte *);
 
-extern struct value *default_value_from_register (struct gdbarch *gdbarch,
-						  struct type *type,
-						  int regnum,
-						  struct frame_id frame_id);
+extern int default_register_part (struct gdbarch *gdbarch,
+				  int len, int *regnum);
 
 extern void read_frame_register_value (struct value *value,
 				       struct frame_info *frame);
-- 
2.5.0

Re: [PATCH 1/2] S390: Take value from sub-register if applicable

[Ulrich Weigand]

Andreas Arnez wrote:

> 	* s390-linux-tdep.c (s390_value_from_register): Use sub-register
> 	if appropriate.

Makes sense to me.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH 2/2] Involve gdbarch in taking DWARF register pieces

[Ulrich Weigand]

Andreas Arnez wrote:

> On s390, when a 'long double' variable is located in a pair of FP
> registers, its DWARF location looks something like this:
> 
>      DW_OP_reg24 (f8); DW_OP_piece: 8; DW_OP_reg25 (f10); DW_OP_piece: 8
> 
> If the target has a vector facility, each of its FP registers is
> embedded left-aligned in a vector register with the same identical DWARF
> register number.  But GDB always takes a register piece from the "least
> significant" end, which is exactly opposite from where the FP register
> is embedded.  This mismatch causes a regression with store.exp.
> 
> This change fixes this issue by taking s390 vector register pieces from
> the left end instead.  Since this requires a suitable gdbarch interface,
> the gdbarch method 'value_from_register' is replaced by a new method
> 'register_part' with similar functionality, but a slightly different
> interface.  This method is then invoked in read_pieced_value and
> write_pieced_value.

I agree it makes sense to use the same gdbarch-provided information
in both places here.

> gdb/ChangeLog:
> 
> 	* gdbarch.sh (value_from_register): Remove.
> 	(register_part): New.
> 	* gdbarch.c: Regenerate.
> 	* gdbarch.h: Likewise.
> 	* dwarf2loc.c (read_pieced_value): Get arch-specific placement of
> 	a register piece from the new gdbarch method register_part.
> 	(write_pieced_value): Likewise.
> 	* value.h (default_value_from_register): Remove.
> 	(default_register_part): Declare.
> 	* findvar.c (default_value_from_register): Remove.
> 	(default_register_part): New.
> 	(value_from_register): Call gdbarch_register_part instead of
> 	gdbarch_value_from_register.
> 	(address_from_register): Likewise.
> 	* s390-linux-tdep.c (s390_value_from_register): Remove.
> 	(s390_register_part): New.
> 	(s390_gdbarch_init): Set new gdbarch method register_part instead
> 	of value_from_register.
> 	* spu-tdep.c (spu_value_from_register): Remove.
> 	(spu_register_part): New.
> 	(spu_gdbarch_init): Set new gdbarch method register_part instead
> 	of value_from_register.

This is OK.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH 2/2] Involve gdbarch in taking DWARF register pieces

[Pedro Alves]

Could we have some comment somewhere on what a "part" is?

This comment isn't very enlightening, IMHO:

+# Locate a part of size LEN within register *REGNUM, possibly overwriting
+# *REGNUM.  Return the offset of the part within the (possibly adjusted)
+# register.
+m:int:register_part:int len, int *regnum:len, regnum::default_register_part::0

Reading this in isolation I have no idea what it's for.  I skimmed the patch
and didn't find any.  Sorry if it's there and I missed it.

Thanks,
Pedro Alves

Re: [PATCH 2/2] Involve gdbarch in taking DWARF register pieces

[Andreas Arnez]

On Fri, Apr 15 2016, Pedro Alves wrote:

> Could we have some comment somewhere on what a "part" is?
>
> This comment isn't very enlightening, IMHO:
>
> +# Locate a part of size LEN within register *REGNUM, possibly overwriting
> +# *REGNUM.  Return the offset of the part within the (possibly adjusted)
> +# register.
> +m:int:register_part:int len, int *regnum:len, regnum::default_register_part::0
>
> Reading this in isolation I have no idea what it's for.  I skimmed the patch
> and didn't find any.  Sorry if it's there and I missed it.

Hm, I used the word "part" to imply a slightly more general meaning than
the DWARF term "piece".  But that may actually be counterproductive...
So how about renaming the method to "register_piece" and then adjusting
the comment like this:

  Determine the placement of a DWARF piece (DW_OP_piece) of size LEN
  within register *REGNUM, possibly overwriting *REGNUM.  Return the
  offset of the piece within the (possibly adjusted) register.  This
  method also applies when interpreting a register as a LEN-sized type.

Does this help?

--
Andreas

Re: [PATCH 2/2] Involve gdbarch in taking DWARF register pieces

[Pedro Alves]

On 04/18/2016 12:53 PM, Andreas Arnez wrote:
> On Fri, Apr 15 2016, Pedro Alves wrote:
> 
>> Could we have some comment somewhere on what a "part" is?
>>
>> This comment isn't very enlightening, IMHO:
>>
>> +# Locate a part of size LEN within register *REGNUM, possibly overwriting
>> +# *REGNUM.  Return the offset of the part within the (possibly adjusted)
>> +# register.
>> +m:int:register_part:int len, int *regnum:len, regnum::default_register_part::0
>>
>> Reading this in isolation I have no idea what it's for.  I skimmed the patch
>> and didn't find any.  Sorry if it's there and I missed it.
> 
> Hm, I used the word "part" to imply a slightly more general meaning than
> the DWARF term "piece".  But that may actually be counterproductive...
> So how about renaming the method to "register_piece" and then adjusting
> the comment like this:
> 
>   Determine the placement of a DWARF piece (DW_OP_piece) of size LEN
>   within register *REGNUM, possibly overwriting *REGNUM.  Return the
>   offset of the piece within the (possibly adjusted) register.  This
>   method also applies when interpreting a register as a LEN-sized type.
> 
> Does this help?

Yes, it does.  Thanks.

I'd suggest even calling it "dwarf_register_piece_placement" for
caller clarity?

-- 
Pedro Alves

Re: [PATCH 2/2] Involve gdbarch in taking DWARF register pieces

[Andreas Arnez]

On Mon, Apr 18 2016, Pedro Alves wrote:

> On 04/18/2016 12:53 PM, Andreas Arnez wrote:
>> On Fri, Apr 15 2016, Pedro Alves wrote:
>> 
>>> Could we have some comment somewhere on what a "part" is?
>>>
>>> This comment isn't very enlightening, IMHO:
>>>
>>> +# Locate a part of size LEN within register *REGNUM, possibly overwriting
>>> +# *REGNUM.  Return the offset of the part within the (possibly adjusted)
>>> +# register.
>>> +m:int:register_part:int len, int *regnum:len, regnum::default_register_part::0
>>>
>>> Reading this in isolation I have no idea what it's for.  I skimmed the patch
>>> and didn't find any.  Sorry if it's there and I missed it.
>> 
>> Hm, I used the word "part" to imply a slightly more general meaning than
>> the DWARF term "piece".  But that may actually be counterproductive...
>> So how about renaming the method to "register_piece" and then adjusting
>> the comment like this:
>> 
>>   Determine the placement of a DWARF piece (DW_OP_piece) of size LEN
>>   within register *REGNUM, possibly overwriting *REGNUM.  Return the
>>   offset of the piece within the (possibly adjusted) register.  This
>>   method also applies when interpreting a register as a LEN-sized type.
>> 
>> Does this help?
>
> Yes, it does.  Thanks.

OK.  Now, reading this again, it seems to me that I should better add
the following disclaimer at the end:

  ... This method also applies when interpreting a register as a
  LEN-sized type, except when convert_register_p indicates that a
  special conversion is required instead.

Still OK?

> I'd suggest even calling it "dwarf_register_piece_placement" for
> caller clarity?

Sure, but I find a name like 'gdbarch_dwarf_register_piece_placement' a
bit too unwieldy when trying to stick to an 80 char line size limit.
Maybe 'gdbarch_register_piece_placement'?

--
Andreas

Re: [PATCH 2/2] Involve gdbarch in taking DWARF register pieces

[Pedro Alves]

On 04/18/2016 04:02 PM, Andreas Arnez wrote:
> On Mon, Apr 18 2016, Pedro Alves wrote:

> OK.  Now, reading this again, it seems to me that I should better add
> the following disclaimer at the end:
> 
>   ... This method also applies when interpreting a register as a
>   LEN-sized type, except when convert_register_p indicates that a
>   special conversion is required instead.
> 
> Still OK?

Hmm, isn't "type" the other side of the same coin though?

How about this:

Determine the physical placement of a type of size LEN within register
*REGNUM, possibly overwriting *REGNUM.  (E.g., some ABIs lay vector
types in registers pairs, and thus this method writes the correct pair
element to *REGNUM).  Returns the byte offset of the data within the
(possibly adjusted) register.  This method is used when determining
the placement of a DWARF piece (DW_OP_piece), or when interpreting a
register as a LEN-sized type, except when convert_register_p indicates
that a special conversion is required instead.

>> I'd suggest even calling it "dwarf_register_piece_placement" for
>> caller clarity?
> 
> Sure, but I find a name like 'gdbarch_dwarf_register_piece_placement' a
> bit too unwieldy when trying to stick to an 80 char line size limit.
> Maybe 'gdbarch_register_piece_placement'?

Deal.  :-)

Thanks,
Pedro Alves

Re: [PATCH 2/2] Involve gdbarch in taking DWARF register pieces

[Doug Evans]

On Mon, Apr 18, 2016 at 8:55 AM, Pedro Alves <palves@redhat.com> wrote:
> On 04/18/2016 04:02 PM, Andreas Arnez wrote:
>> On Mon, Apr 18 2016, Pedro Alves wrote:
>
>> OK.  Now, reading this again, it seems to me that I should better add
>> the following disclaimer at the end:
>>
>>   ... This method also applies when interpreting a register as a
>>   LEN-sized type, except when convert_register_p indicates that a
>>   special conversion is required instead.
>>
>> Still OK?

This is the part that I found lacking when I read the initial patch.

> Hmm, isn't "type" the other side of the same coin though?
>
> How about this:
>
> Determine the physical placement of a type of size LEN within register
> *REGNUM, possibly overwriting *REGNUM.  (E.g., some ABIs lay vector
> types in registers pairs, and thus this method writes the correct pair
> element to *REGNUM).  Returns the byte offset of the data within the
> (possibly adjusted) register.  This method is used when determining
> the placement of a DWARF piece (DW_OP_piece), or when interpreting a
> register as a LEN-sized type, except when convert_register_p indicates
> that a special conversion is required instead.

SGTM

>>> I'd suggest even calling it "dwarf_register_piece_placement" for
>>> caller clarity?
>>
>> Sure, but I find a name like 'gdbarch_dwarf_register_piece_placement' a
>> bit too unwieldy when trying to stick to an 80 char line size limit.
>> Maybe 'gdbarch_register_piece_placement'?
>
> Deal.  :-)
>
> Thanks,
> Pedro Alves
>

Re: [PATCH 2/2] Involve gdbarch in taking DWARF register pieces

[Andreas Arnez]

On Mon, Apr 18 2016, Pedro Alves wrote:

> How about this:
>
> Determine the physical placement of a type of size LEN within register
> *REGNUM, possibly overwriting *REGNUM.  (E.g., some ABIs lay vector
> types in registers pairs, and thus this method writes the correct pair
> element to *REGNUM).  Returns the byte offset of the data within the
> (possibly adjusted) register.  This method is used when determining
> the placement of a DWARF piece (DW_OP_piece), or when interpreting a
> register as a LEN-sized type, except when convert_register_p indicates
> that a special conversion is required instead.

Thanks, I like that wording.  However, I still see a slight potential
for misunderstanding:

* When using the definition "physical placement of a type of size LEN",
  the reader may be led to assume that LEN always specifies the length
  of the resulting type, not the length of an individual DWARF piece.

* I'm not sure about the example in parentheses.  It seems to imply that
  the input register may differ from the first resulting pair element.
  Is that based on a real scenario, or is it hypothetical?

* The disclaimer for convert_register_p may now be misinterpreted to
  apply to DWARF pieces as well.

Here's another attempt:

Determine the physical placement of a piece of size LEN within register
*REGNUM, possibly overwriting *REGNUM.  (E.g., some ABIs have unwindable
sub-registers embedded in non-unwindable full registers, and this method
diverts from the full register to the sub-register if possible.)
Returns the byte offset of the data within the (possibly adjusted)
register.  This method is used for determining the placement of a
LEN-sized DWARF piece (DW_OP_piece) and when interpreting a register as
a LEN-sized type (unless convert_register_p indicates that the type
needs a special conversion).

>
>>> I'd suggest even calling it "dwarf_register_piece_placement" for
>>> caller clarity?
>> 
>> Sure, but I find a name like 'gdbarch_dwarf_register_piece_placement' a
>> bit too unwieldy when trying to stick to an 80 char line size limit.
>> Maybe 'gdbarch_register_piece_placement'?
>
> Deal.  :-)

:-)

Updated patch below.

-- >8 --
Subject: [PATCH v2] Involve gdbarch in taking DWARF register pieces

On s390, when a 'long double' variable is located in a pair of FP
registers, its DWARF location looks something like this:

     DW_OP_reg24 (f8); DW_OP_piece: 8; DW_OP_reg25 (f10); DW_OP_piece: 8

If the target has a vector facility, each of its FP registers is
embedded left-aligned in a vector register with the same identical DWARF
register number.  But GDB always takes a register piece from the "least
significant" end, which is exactly opposite from where the FP register
is embedded.  This mismatch causes a regression with store.exp.

This change fixes this issue by taking s390 vector register pieces from
the left end instead.  Since this requires a suitable gdbarch interface,
the gdbarch method 'value_from_register' is replaced by a new method
'register_piece_placement' with similar functionality, but a slightly
different interface.  This method is then invoked in read_pieced_value and
write_pieced_value.

gdb/ChangeLog:

	* gdbarch.sh (value_from_register): Remove.
	(register_piece_placement): New.
	* gdbarch.c: Regenerate.
	* gdbarch.h: Likewise.
	* dwarf2loc.c (read_pieced_value): Get arch-specific placement of
	a register piece from gdbarch_register_piece_placement.
	(write_pieced_value): Likewise.
	* value.h (default_value_from_register): Remove.
	(default_register_piece_placement): Declare.
	* findvar.c (default_value_from_register): Remove.
	(default_register_piece_placement): New.
	(value_from_register): Call gdbarch_register_piece_placement
	instead of gdbarch_value_from_register.
	(address_from_register): Likewise.
	* s390-linux-tdep.c (s390_value_from_register): Remove.
	(s390_register_piece_placement): New.
	(s390_gdbarch_init): Set new gdbarch method
	register_piece_placement instead of value_from_register.
	* spu-tdep.c (spu_value_from_register): Remove.
	(spu_register_piece_placement): New.
	(spu_gdbarch_init): Set new gdbarch method
	register_piece_placement instead of value_from_register.
---
 gdb/dwarf2loc.c       | 35 +++++++++++++++++------------------
 gdb/findvar.c         | 42 +++++++++++++++++++++---------------------
 gdb/gdbarch.c         | 28 ++++++++++++++--------------
 gdb/gdbarch.h         | 21 +++++++++++++--------
 gdb/gdbarch.sh        | 17 +++++++++++------
 gdb/s390-linux-tdep.c | 32 ++++++++++++++------------------
 gdb/spu-tdep.c        | 21 +++++++--------------
 gdb/value.h           |  6 ++----
 8 files changed, 99 insertions(+), 103 deletions(-)

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index ba6ed42..d258a41 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1715,6 +1715,7 @@ read_pieced_value (struct value *v)
   for (i = 0; i < c->n_pieces && offset < type_len; i++)
     {
       struct dwarf_expr_piece *p = &c->pieces[i];
+      size_t piece_bytes = (p->size + 7) / 8;
       size_t this_size, this_size_bits;
       long dest_offset_bits, source_offset_bits, source_offset;
       const gdb_byte *intermediate_buffer;
@@ -1759,17 +1760,14 @@ read_pieced_value (struct value *v)
 	    struct gdbarch *arch = get_frame_arch (frame);
 	    int gdb_regnum = dwarf_reg_to_regnum_or_error (arch, p->v.regno);
 	    int optim, unavail;
-	    int reg_offset = source_offset;
+	    int reg_offset = gdbarch_register_piece_placement (arch,
+							       piece_bytes,
+							       &gdb_regnum);
 
-	    if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG
-		&& this_size < register_size (arch, gdb_regnum))
-	      {
-		/* Big-endian, and we want less than full size.  */
-		reg_offset = register_size (arch, gdb_regnum) - this_size;
-		/* We want the lower-order THIS_SIZE_BITS of the bytes
-		   we extract from the register.  */
-		source_offset_bits += 8 * this_size - this_size_bits;
-	      }
+	    source_offset_bits += 8 * reg_offset;
+	    if (bits_big_endian)
+	      source_offset_bits += 8 * piece_bytes - this_size_bits;
+	    reg_offset = source_offset_bits / 8;
 
 	    if (!get_frame_register_bytes (frame, gdb_regnum, reg_offset,
 					   this_size, buffer,
@@ -1890,6 +1888,7 @@ write_pieced_value (struct value *to, struct value *from)
   for (i = 0; i < c->n_pieces && offset < type_len; i++)
     {
       struct dwarf_expr_piece *p = &c->pieces[i];
+      size_t piece_bytes = (p->size + 7) / 8;
       size_t this_size_bits, this_size;
       long dest_offset_bits, source_offset_bits, dest_offset, source_offset;
       int need_bitwise;
@@ -1941,14 +1940,14 @@ write_pieced_value (struct value *to, struct value *from)
 	  {
 	    struct gdbarch *arch = get_frame_arch (frame);
 	    int gdb_regnum = dwarf_reg_to_regnum_or_error (arch, p->v.regno);
-	    int reg_offset = dest_offset;
-
-	    if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG
-		&& this_size <= register_size (arch, gdb_regnum))
-	      {
-		/* Big-endian, and we want less than full size.  */
-		reg_offset = register_size (arch, gdb_regnum) - this_size;
-	      }
+	    int reg_offset = gdbarch_register_piece_placement (arch,
+							       piece_bytes,
+							       &gdb_regnum);
+
+	    source_offset_bits += 8 * reg_offset;
+	    if (bits_big_endian)
+	      source_offset_bits += 8 * piece_bytes - this_size_bits;
+	    reg_offset = source_offset_bits / 8;
 
 	    if (need_bitwise)
 	      {
diff --git a/gdb/findvar.c b/gdb/findvar.c
index a39d897..0a305f5 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -797,29 +797,19 @@ read_var_value (struct symbol *var, const struct block *var_block,
 
 /* Install default attributes for register values.  */
 
-struct value *
-default_value_from_register (struct gdbarch *gdbarch, struct type *type,
-                             int regnum, struct frame_id frame_id)
+int
+default_register_piece_placement (struct gdbarch *gdbarch, int len, int *regnum)
 {
-  int len = TYPE_LENGTH (type);
-  struct value *value = allocate_value (type);
-
-  VALUE_LVAL (value) = lval_register;
-  VALUE_FRAME_ID (value) = frame_id;
-  VALUE_REGNUM (value) = regnum;
-
   /* Any structure stored in more than one register will always be
      an integral number of registers.  Otherwise, you need to do
      some fiddling with the last register copied here for little
      endian machines.  */
   if (gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG
-      && len < register_size (gdbarch, regnum))
+      && len < register_size (gdbarch, *regnum))
     /* Big-endian, and we want less than full size.  */
-    set_value_offset (value, register_size (gdbarch, regnum) - len);
-  else
-    set_value_offset (value, 0);
+    return register_size (gdbarch, *regnum) - len;
 
-  return value;
+  return 0;
 }
 
 /* VALUE must be an lval_register value.  If regnum is the value's
@@ -877,6 +867,10 @@ value_from_register (struct type *type, int regnum, struct frame_info *frame)
   struct type *type1 = check_typedef (type);
   struct value *v;
 
+  v = allocate_value (type);
+  VALUE_LVAL (v) = lval_register;
+  VALUE_FRAME_ID (v) = get_frame_id (frame);
+
   if (gdbarch_convert_register_p (gdbarch, regnum, type1))
     {
       int optim, unavail, ok;
@@ -888,9 +882,6 @@ value_from_register (struct type *type, int regnum, struct frame_info *frame)
          the corresponding [integer] type (see Alpha).  The assumption
          is that gdbarch_register_to_value populates the entire value
          including the location.  */
-      v = allocate_value (type);
-      VALUE_LVAL (v) = lval_register;
-      VALUE_FRAME_ID (v) = get_frame_id (frame);
       VALUE_REGNUM (v) = regnum;
       ok = gdbarch_register_to_value (gdbarch, frame, regnum, type1,
 				      value_contents_raw (v), &optim,
@@ -907,8 +898,11 @@ value_from_register (struct type *type, int regnum, struct frame_info *frame)
   else
     {
       /* Construct the value.  */
-      v = gdbarch_value_from_register (gdbarch, type,
-				       regnum, get_frame_id (frame));
+      int offset = gdbarch_register_piece_placement (gdbarch,
+						     TYPE_LENGTH (type),
+						     &regnum);
+      VALUE_REGNUM (v) = regnum;
+      set_value_offset (v, offset);
 
       /* Get the data.  */
       read_frame_register_value (v, frame);
@@ -927,6 +921,7 @@ address_from_register (int regnum, struct frame_info *frame)
   struct type *type = builtin_type (gdbarch)->builtin_data_ptr;
   struct value *value;
   CORE_ADDR result;
+  int offset;
   int regnum_max_excl = (gdbarch_num_regs (gdbarch)
 			 + gdbarch_num_pseudo_regs (gdbarch));
 
@@ -962,7 +957,12 @@ address_from_register (int regnum, struct frame_info *frame)
       return unpack_long (type, buf);
     }
 
-  value = gdbarch_value_from_register (gdbarch, type, regnum, null_frame_id);
+  offset = gdbarch_register_piece_placement (gdbarch, TYPE_LENGTH (type),
+					     &regnum);
+  value = allocate_value (type);
+  VALUE_LVAL (value) = lval_register;
+  VALUE_REGNUM (value) = regnum;
+  set_value_offset (value, offset);
   read_frame_register_value (value, frame);
 
   if (value_optimized_out (value))
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index bd0b48c..156373c 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -216,7 +216,7 @@ struct gdbarch
   gdbarch_convert_register_p_ftype *convert_register_p;
   gdbarch_register_to_value_ftype *register_to_value;
   gdbarch_value_to_register_ftype *value_to_register;
-  gdbarch_value_from_register_ftype *value_from_register;
+  gdbarch_register_piece_placement_ftype *register_piece_placement;
   gdbarch_pointer_to_address_ftype *pointer_to_address;
   gdbarch_address_to_pointer_ftype *address_to_pointer;
   gdbarch_integer_to_address_ftype *integer_to_address;
@@ -393,7 +393,7 @@ gdbarch_alloc (const struct gdbarch_info *info,
   gdbarch->cannot_fetch_register = cannot_register_not;
   gdbarch->cannot_store_register = cannot_register_not;
   gdbarch->convert_register_p = generic_convert_register_p;
-  gdbarch->value_from_register = default_value_from_register;
+  gdbarch->register_piece_placement = default_register_piece_placement;
   gdbarch->pointer_to_address = unsigned_pointer_to_address;
   gdbarch->address_to_pointer = unsigned_address_to_pointer;
   gdbarch->return_in_first_hidden_param_p = default_return_in_first_hidden_param_p;
@@ -560,7 +560,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of cannot_store_register, invalid_p == 0 */
   /* Skip verify of get_longjmp_target, has predicate.  */
   /* Skip verify of convert_register_p, invalid_p == 0 */
-  /* Skip verify of value_from_register, invalid_p == 0 */
+  /* Skip verify of register_piece_placement, invalid_p == 0 */
   /* Skip verify of pointer_to_address, invalid_p == 0 */
   /* Skip verify of address_to_pointer, invalid_p == 0 */
   /* Skip verify of integer_to_address, has predicate.  */
@@ -1242,6 +1242,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: register_name = <%s>\n",
                       host_address_to_string (gdbarch->register_name));
   fprintf_unfiltered (file,
+                      "gdbarch_dump: register_piece_placement = <%s>\n",
+                      host_address_to_string (gdbarch->register_piece_placement));
+  fprintf_unfiltered (file,
                       "gdbarch_dump: register_reggroup_p = <%s>\n",
                       host_address_to_string (gdbarch->register_reggroup_p));
   fprintf_unfiltered (file,
@@ -1398,9 +1401,6 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: unwind_sp = <%s>\n",
                       host_address_to_string (gdbarch->unwind_sp));
   fprintf_unfiltered (file,
-                      "gdbarch_dump: value_from_register = <%s>\n",
-                      host_address_to_string (gdbarch->value_from_register));
-  fprintf_unfiltered (file,
                       "gdbarch_dump: value_to_register = <%s>\n",
                       host_address_to_string (gdbarch->value_to_register));
   fprintf_unfiltered (file,
@@ -2514,21 +2514,21 @@ set_gdbarch_value_to_register (struct gdbarch *gdbarch,
   gdbarch->value_to_register = value_to_register;
 }
 
-struct value *
-gdbarch_value_from_register (struct gdbarch *gdbarch, struct type *type, int regnum, struct frame_id frame_id)
+int
+gdbarch_register_piece_placement (struct gdbarch *gdbarch, int len, int *regnum)
 {
   gdb_assert (gdbarch != NULL);
-  gdb_assert (gdbarch->value_from_register != NULL);
+  gdb_assert (gdbarch->register_piece_placement != NULL);
   if (gdbarch_debug >= 2)
-    fprintf_unfiltered (gdb_stdlog, "gdbarch_value_from_register called\n");
-  return gdbarch->value_from_register (gdbarch, type, regnum, frame_id);
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_register_piece_placement called\n");
+  return gdbarch->register_piece_placement (gdbarch, len, regnum);
 }
 
 void
-set_gdbarch_value_from_register (struct gdbarch *gdbarch,
-                                 gdbarch_value_from_register_ftype value_from_register)
+set_gdbarch_register_piece_placement (struct gdbarch *gdbarch,
+                                      gdbarch_register_piece_placement_ftype register_piece_placement)
 {
-  gdbarch->value_from_register = value_from_register;
+  gdbarch->register_piece_placement = register_piece_placement;
 }
 
 CORE_ADDR
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 252fc4b..b867635 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -450,14 +450,19 @@ typedef void (gdbarch_value_to_register_ftype) (struct frame_info *frame, int re
 extern void gdbarch_value_to_register (struct gdbarch *gdbarch, struct frame_info *frame, int regnum, struct type *type, const gdb_byte *buf);
 extern void set_gdbarch_value_to_register (struct gdbarch *gdbarch, gdbarch_value_to_register_ftype *value_to_register);
 
-/* Construct a value representing the contents of register REGNUM in
-   frame FRAME_ID, interpreted as type TYPE.  The routine needs to
-   allocate and return a struct value with all value attributes
-   (but not the value contents) filled in. */
-
-typedef struct value * (gdbarch_value_from_register_ftype) (struct gdbarch *gdbarch, struct type *type, int regnum, struct frame_id frame_id);
-extern struct value * gdbarch_value_from_register (struct gdbarch *gdbarch, struct type *type, int regnum, struct frame_id frame_id);
-extern void set_gdbarch_value_from_register (struct gdbarch *gdbarch, gdbarch_value_from_register_ftype *value_from_register);
+/* Determine the physical placement of a piece of size LEN within register
+   *REGNUM, possibly overwriting *REGNUM.  (E.g., some ABIs have unwindable
+   sub-registers embedded in non-unwindable full registers, and this method
+   diverts from the full register to the sub-register if possible.)
+   Returns the byte offset of the data within the (possibly adjusted)
+   register.  This method is used for determining the placement of a
+   LEN-sized DWARF piece (DW_OP_piece) and when interpreting a register as
+   a LEN-sized type (unless convert_register_p indicates that the type
+   needs a special conversion). */
+
+typedef int (gdbarch_register_piece_placement_ftype) (struct gdbarch *gdbarch, int len, int *regnum);
+extern int gdbarch_register_piece_placement (struct gdbarch *gdbarch, int len, int *regnum);
+extern void set_gdbarch_register_piece_placement (struct gdbarch *gdbarch, gdbarch_register_piece_placement_ftype *register_piece_placement);
 
 typedef CORE_ADDR (gdbarch_pointer_to_address_ftype) (struct gdbarch *gdbarch, struct type *type, const gdb_byte *buf);
 extern CORE_ADDR gdbarch_pointer_to_address (struct gdbarch *gdbarch, struct type *type, const gdb_byte *buf);
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 37f59b7..8f02a1a 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -24,7 +24,6 @@
 LANG=C ; export LANG
 LC_ALL=C ; export LC_ALL
 
-
 compare_new ()
 {
     file=$1
@@ -506,11 +505,17 @@ v:int:believe_pcc_promotion:::::::
 m:int:convert_register_p:int regnum, struct type *type:regnum, type:0:generic_convert_register_p::0
 f:int:register_to_value:struct frame_info *frame, int regnum, struct type *type, gdb_byte *buf, int *optimizedp, int *unavailablep:frame, regnum, type, buf, optimizedp, unavailablep:0
 f:void:value_to_register:struct frame_info *frame, int regnum, struct type *type, const gdb_byte *buf:frame, regnum, type, buf:0
-# Construct a value representing the contents of register REGNUM in
-# frame FRAME_ID, interpreted as type TYPE.  The routine needs to
-# allocate and return a struct value with all value attributes
-# (but not the value contents) filled in.
-m:struct value *:value_from_register:struct type *type, int regnum, struct frame_id frame_id:type, regnum, frame_id::default_value_from_register::0
+
+# Determine the physical placement of a piece of size LEN within register
+# *REGNUM, possibly overwriting *REGNUM.  (E.g., some ABIs have unwindable
+# sub-registers embedded in non-unwindable full registers, and this method
+# diverts from the full register to the sub-register if possible.)
+# Returns the byte offset of the data within the (possibly adjusted)
+# register.  This method is used for determining the placement of a
+# LEN-sized DWARF piece (DW_OP_piece) and when interpreting a register as
+# a LEN-sized type (unless convert_register_p indicates that the type
+# needs a special conversion).
+m:int:register_piece_placement:int len, int *regnum:len, regnum::default_register_piece_placement::0
 #
 m:CORE_ADDR:pointer_to_address:struct type *type, const gdb_byte *buf:type, buf::unsigned_pointer_to_address::0
 m:void:address_to_pointer:struct type *type, gdb_byte *buf, CORE_ADDR addr:type, buf, addr::unsigned_address_to_pointer::0
diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index 8b4efb1..cb2baf1 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -513,28 +513,24 @@ s390_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
    registers, even though we are otherwise a big-endian platform.  The
    same applies to a 'float' value within a vector.  */
 
-static struct value *
-s390_value_from_register (struct gdbarch *gdbarch, struct type *type,
-			  int regnum, struct frame_id frame_id)
+static int
+s390_register_piece_placement (struct gdbarch *gdbarch, int len,
+			       int *regnum)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
-  unsigned int len = TYPE_LENGTH (check_typedef (type));
-  struct value *value;
 
   /* If the value fits in a sub-register, use that instead.  */
-  if (regnum_is_vxr_full (tdep, regnum) && len <= 8)
-    regnum = regnum - tdep->v0_full_regnum + S390_F0_REGNUM;
-  else if (regnum_is_gpr_full (tdep, regnum) && len <= 4)
-    regnum = regnum - tdep->gpr_full_regnum + S390_R0_REGNUM;
-
-  value = default_value_from_register (gdbarch, type, regnum, frame_id);
-
-  if ((regnum >= S390_F0_REGNUM && regnum <= S390_F15_REGNUM && len < 8)
-      || regnum_is_vxr_full (tdep, regnum)
-      || (regnum >= S390_V16_REGNUM && regnum <= S390_V31_REGNUM))
-    set_value_offset (value, 0);
+  if (regnum_is_vxr_full (tdep, *regnum) && len <= 8)
+    *regnum = *regnum - tdep->v0_full_regnum + S390_F0_REGNUM;
+  else if (regnum_is_gpr_full (tdep, *regnum) && len <= 4)
+    *regnum = *regnum - tdep->gpr_full_regnum + S390_R0_REGNUM;
+
+  if ((*regnum >= S390_F0_REGNUM && *regnum <= S390_F15_REGNUM && len < 8)
+      || regnum_is_vxr_full (tdep, *regnum)
+      || (*regnum >= S390_V16_REGNUM && *regnum <= S390_V31_REGNUM))
+    return 0;
 
-  return value;
+  return default_register_piece_placement (gdbarch, len, regnum);
 }
 
 /* Register groups.  */
@@ -8005,7 +8001,7 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_fp0_regnum (gdbarch, S390_F0_REGNUM);
   set_gdbarch_stab_reg_to_regnum (gdbarch, s390_dwarf_reg_to_regnum);
   set_gdbarch_dwarf2_reg_to_regnum (gdbarch, s390_dwarf_reg_to_regnum);
-  set_gdbarch_value_from_register (gdbarch, s390_value_from_register);
+  set_gdbarch_register_piece_placement (gdbarch, s390_register_piece_placement);
   set_gdbarch_core_read_description (gdbarch, s390_core_read_description);
   set_gdbarch_iterate_over_regset_sections (gdbarch,
 					    s390_iterate_over_regset_sections);
diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index 8dad5c3..15ca5ac 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -357,21 +357,14 @@ spu_ax_pseudo_register_push_stack (struct gdbarch *gdbarch,
 
 /* Value conversion -- access scalar values at the preferred slot.  */
 
-static struct value *
-spu_value_from_register (struct gdbarch *gdbarch, struct type *type,
-			 int regnum, struct frame_id frame_id)
+static int
+spu_register_piece_placement (struct gdbarch *gdbarch, int len,
+			      int *regnum)
 {
-  struct value *value = default_value_from_register (gdbarch, type,
-						     regnum, frame_id);
-  int len = TYPE_LENGTH (type);
-
-  if (regnum < SPU_NUM_GPRS && len < 16)
-    {
-      int preferred_slot = len < 4 ? 4 - len : 0;
-      set_value_offset (value, preferred_slot);
-    }
+  if (*regnum < SPU_NUM_GPRS && len < 16)
+    return len < 4 ? 4 - len : 0;
 
-  return value;
+  return default_register_piece_placement (gdbarch, len, regnum);
 }
 
 /* Register groups.  */
@@ -2738,7 +2731,7 @@ spu_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_register_type (gdbarch, spu_register_type);
   set_gdbarch_pseudo_register_read (gdbarch, spu_pseudo_register_read);
   set_gdbarch_pseudo_register_write (gdbarch, spu_pseudo_register_write);
-  set_gdbarch_value_from_register (gdbarch, spu_value_from_register);
+  set_gdbarch_register_piece_placement (gdbarch, spu_register_piece_placement);
   set_gdbarch_register_reggroup_p (gdbarch, spu_register_reggroup_p);
   set_gdbarch_dwarf2_reg_to_regnum (gdbarch, spu_dwarf_reg_to_regnum);
   set_gdbarch_ax_pseudo_register_collect
diff --git a/gdb/value.h b/gdb/value.h
index f8ec854..c7fc28e 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -647,10 +647,8 @@ extern struct value *value_from_contents_and_address (struct type *,
 						      CORE_ADDR);
 extern struct value *value_from_contents (struct type *, const gdb_byte *);
 
-extern struct value *default_value_from_register (struct gdbarch *gdbarch,
-						  struct type *type,
-						  int regnum,
-						  struct frame_id frame_id);
+extern int default_register_piece_placement (struct gdbarch *gdbarch,
+					     int len, int *regnum);
 
 extern void read_frame_register_value (struct value *value,
 				       struct frame_info *frame);
-- 
2.5.0

[PING][PATCH 2/2] Involve gdbarch in taking DWARF register pieces

[Andreas Arnez]

Ping:

  https://sourceware.org/ml/gdb-patches/2016-04/msg00437.html

IIRC, there was some uncertainty about the clarity/meaning of the new
gdbarch method's description below.  Or has this cleared up by now?

On Tue, Apr 19 2016, Andreas Arnez wrote:

> Here's another attempt:
>
> Determine the physical placement of a piece of size LEN within register
> *REGNUM, possibly overwriting *REGNUM.  (E.g., some ABIs have unwindable
> sub-registers embedded in non-unwindable full registers, and this method
> diverts from the full register to the sub-register if possible.)
> Returns the byte offset of the data within the (possibly adjusted)
> register.  This method is used for determining the placement of a
> LEN-sized DWARF piece (DW_OP_piece) and when interpreting a register as
> a LEN-sized type (unless convert_register_p indicates that the type
> needs a special conversion).

[...]

> gdb/ChangeLog:
>
> 	* gdbarch.sh (value_from_register): Remove.
> 	(register_piece_placement): New.
> 	* gdbarch.c: Regenerate.
> 	* gdbarch.h: Likewise.
> 	* dwarf2loc.c (read_pieced_value): Get arch-specific placement of
> 	a register piece from gdbarch_register_piece_placement.
> 	(write_pieced_value): Likewise.
> 	* value.h (default_value_from_register): Remove.
> 	(default_register_piece_placement): Declare.
> 	* findvar.c (default_value_from_register): Remove.
> 	(default_register_piece_placement): New.
> 	(value_from_register): Call gdbarch_register_piece_placement
> 	instead of gdbarch_value_from_register.
> 	(address_from_register): Likewise.
> 	* s390-linux-tdep.c (s390_value_from_register): Remove.
> 	(s390_register_piece_placement): New.
> 	(s390_gdbarch_init): Set new gdbarch method
> 	register_piece_placement instead of value_from_register.
> 	* spu-tdep.c (spu_value_from_register): Remove.
> 	(spu_register_piece_placement): New.
> 	(spu_gdbarch_init): Set new gdbarch method
> 	register_piece_placement instead of value_from_register.

Re: [PING][PATCH 2/2] Involve gdbarch in taking DWARF register pieces

[Pedro Alves]

On 04/28/2016 02:24 PM, Andreas Arnez wrote:
> Ping:
> 
>   https://sourceware.org/ml/gdb-patches/2016-04/msg00437.html
> 
> IIRC, there was some uncertainty about the clarity/meaning of the new
> gdbarch method's description below.  Or has this cleared up by now?

Sorry, I've spent the last hour trying to wrap my head around this,
but I'm still confused.  :-/  I'm sorry to be blocking this.

> 
> On Tue, Apr 19 2016, Andreas Arnez wrote:
> 
>> Here's another attempt:
>>
>> Determine the physical placement of a piece of size LEN within register
>> *REGNUM, possibly overwriting *REGNUM.  (E.g., some ABIs have unwindable
>> sub-registers embedded in non-unwindable full registers, and this method
>> diverts from the full register to the sub-register if possible.)

I couldn't find any reference to "sub-register" in the codebase.
I'd assume it's something like "eax" being a sub part of "rax"
on x86-64.  But I'm not certain that's the case here?  On a machine with
vector registers, is a FP register really a chunk of the vector
register, or is it a real separate physical register?

My main confusion revolves I think, around how these points
are addressed:

 - FP registers and vector registers have the same identical
   DWARF register number.

 - If the object stored is <= 8 bytes, we should find it in
   the FP register; otherwise get it from the vector register.

I'd naively think that the fix for something like that would be
to make dwarf_reg_to_regnum return the gdb FP register number instead 
of the vector number, when the type fits in a FP register, instead of
the need for an extra diversion step.  Ignoring the fact that we don't
currently pass the type/size to gdbarch_dwarf_reg_to_regnum.

It may be that the end result is the same, but it's all blurry to
me still.

Thanks,
Pedro Alves

Re: [PING][PATCH 2/2] Involve gdbarch in taking DWARF register pieces

[Andreas Arnez]

On Thu, Apr 28 2016, Pedro Alves wrote:

> I couldn't find any reference to "sub-register" in the codebase.
> I'd assume it's something like "eax" being a sub part of "rax"
> on x86-64.  But I'm not certain that's the case here?  On a machine with
> vector registers, is a FP register really a chunk of the vector
> register, or is it a real separate physical register?

It's exactly comparable with eax and rax.  Or consider the SSE registers
xmm0-xmm15, which are embedded in their double-wide AVX counterparts
ymm0-ymm15.  With z/Architecture, each 64-bit FP register is just a
"chunk" ("sub-register" / "part" / "slice" / ...) of a 128-bit vector
register.  The ASCII art in section 2.1 of this article illustrates
this:

  https://sourceware.org/ml/gdb/2016-01/msg00013.html

(BTW, I still didn't get much feedback on that article...)

And if there is a better (or wider used) term than "sub-register", I'll
be happy to change the wording.

> My main confusion revolves I think, around how these points
> are addressed:
>
>  - FP registers and vector registers have the same identical
>    DWARF register number.
>
>  - If the object stored is <= 8 bytes, we should find it in
>    the FP register; otherwise get it from the vector register.
>
> I'd naively think that the fix for something like that would be
> to make dwarf_reg_to_regnum return the gdb FP register number instead 
> of the vector number, when the type fits in a FP register, instead of
> the need for an extra diversion step.  Ignoring the fact that we don't
> currently pass the type/size to gdbarch_dwarf_reg_to_regnum.

Right, ignoring that fact ;-)

Also, IMHO the "actual" placement of an object within a register does
not conceptually depend on where the register number came from.  It
could come from DWARF, from some other debug format, from the user, or
from wherever.  Adjusting the placement only for objects with a DWARF
location seems wrong to me.

--
Andreas

Re: [PING][PATCH 2/2] Involve gdbarch in taking DWARF register pieces

[Andreas Arnez]

On Thu, Apr 28 2016, Andreas Arnez wrote:

> On Thu, Apr 28 2016, Pedro Alves wrote:
>
>> I'd naively think that the fix for something like that would be
>> to make dwarf_reg_to_regnum return the gdb FP register number instead 
>> of the vector number, when the type fits in a FP register, instead of
>> the need for an extra diversion step.  Ignoring the fact that we don't
>> currently pass the type/size to gdbarch_dwarf_reg_to_regnum.
>
> Right, ignoring that fact ;-)
>
> Also, IMHO the "actual" placement of an object within a register does
> not conceptually depend on where the register number came from.  It
> could come from DWARF, from some other debug format, from the user, or
> from wherever.  Adjusting the placement only for objects with a DWARF
> location seems wrong to me.

There's another aspect I should probably mention here.  The adjustment
of the register number in this gdbarch method is really just a
circumvention around the register cache's inability of representing
*partially* valid registers.  Otherwise unwinding would result in the
first 64 bits of such a vector register being "valid" and the others
"invalid", and then we could slice any value types from the valid bits.

In the long run maybe we want to extend the regcache in such a
direction, but I didn't see that in the scope of this fix.

--
Andreas

Re: [PING][PATCH 2/2] Involve gdbarch in taking DWARF register pieces

[Pedro Alves]

On 04/28/2016 05:51 PM, Andreas Arnez wrote:
> On Thu, Apr 28 2016, Pedro Alves wrote:
> 
>> I couldn't find any reference to "sub-register" in the codebase.
>> I'd assume it's something like "eax" being a sub part of "rax"
>> on x86-64.  But I'm not certain that's the case here?  On a machine with
>> vector registers, is a FP register really a chunk of the vector
>> register, or is it a real separate physical register?
> 
> It's exactly comparable with eax and rax.  Or consider the SSE registers
> xmm0-xmm15, which are embedded in their double-wide AVX counterparts
> ymm0-ymm15.  With z/Architecture, each 64-bit FP register is just a
> "chunk" ("sub-register" / "part" / "slice" / ...) of a 128-bit vector
> register.  The ASCII art in section 2.1 of this article illustrates
> this:
> 
>   https://sourceware.org/ml/gdb/2016-01/msg00013.html

Thanks, this helps a lot.

> 
> (BTW, I still didn't get much feedback on that article...)
> 
> And if there is a better (or wider used) term than "sub-register", I'll
> be happy to change the wording.

No, that's fine terminology.  I was just confused because I wasn't very
clear whether we're talking about completely different registers.
Thanks,
Pedro Alves

Re: [PING][PATCH 2/2] Involve gdbarch in taking DWARF register pieces

[Pedro Alves]

On 04/28/2016 07:16 PM, Andreas Arnez wrote:

>> Also, IMHO the "actual" placement of an object within a register does
>> not conceptually depend on where the register number came from.  It
>> could come from DWARF, from some other debug format, from the user, or
>> from wherever.  Adjusting the placement only for objects with a DWARF
>> location seems wrong to me.
> 
> There's another aspect I should probably mention here.  The adjustment
> of the register number in this gdbarch method is really just a
> circumvention around the register cache's inability of representing
> *partially* valid registers.  Otherwise unwinding would result in the
> first 64 bits of such a vector register being "valid" and the others
> "invalid", and then we could slice any value types from the valid bits.

Ah, when I saw your other email pointing at the ascii art, the thought
of partially valid registers crossed my mind too.

But why do you say we don't support this?

The register cache only cares about registers in the current frame
(IOW, the real current state of a thread's registers).  And in the
current frame, the whole vector register is always valid.  So I don't
think we need to worry about the register cache.

The unwinding machinery instead works with struct values, and those _do_
support being marked partially optimized out (not saved).  You do that by
calling mark_value_bits_optimized_out on the part of the value that
is not saved.

Maybe there's e.g., some value_optimized_out() checks on common code
that might need to be replaced with a more finer-grained "these bytes/bit
here are optimized-out" check, but what else would be necessary?

> In the long run maybe we want to extend the regcache in such a
> direction, but I didn't see that in the scope of this fix.

Thanks,
Pedro Alves

Re: [PING][PATCH 2/2] Involve gdbarch in taking DWARF register pieces

[Jan Kratochvil]

On Thu, 28 Apr 2016 15:24:08 +0200, Andreas Arnez wrote:
> IIRC, there was some uncertainty about the clarity/meaning of the new
> gdbarch method's description below.  Or has this cleared up by now?

Ping (to Andreas Arnez) as this thread remained unfinished.


Jan