diff mbox

[v2,01/10] Aarch64 SVE pseudo register support

Message ID 20180606151629.36602-2-alan.hayward@arm.com
State New, archived
Headers

Commit Message

Alan Hayward June 6, 2018, 3:16 p.m. UTC 
  Add the functionality for reading/writing pseudo registers.

On SVE the V registers are pseudo registers. This is supported
by adding AARCH64_SVE_V0_REGNUM.

2018-06-06  Alan Hayward  <alan.hayward@arm.com>

	* aarch64-tdep.c (AARCH64_SVE_V0_REGNUM): Add define.
	(aarch64_vnv_type): Add function.
	(aarch64_pseudo_register_name): Add V regs for SVE.
	(aarch64_pseudo_register_type): Likewise.
	(aarch64_pseudo_register_reggroup_p): Likewise.
	(aarch64_pseudo_read_value_2): Use V0 offset for SVE
	(aarch64_pseudo_read_value): Add V regs for SVE.
	(aarch64_pseudo_write_2): Use V0 offset for SVE
	(aarch64_pseudo_write): Add V regs for SVE.
	* aarch64-tdep.h (struct gdbarch_tdep): Add vnv_type.
---
 gdb/aarch64-tdep.c | 131 ++++++++++++++++++++++++++++++++++++++++++++---------
 gdb/aarch64-tdep.h |   1 +
 2 files changed, 111 insertions(+), 21 deletions(-)

Comments

Simon Marchi June 6, 2018, 10:17 p.m. UTC | #1 
Hi Alan,

I have some more nits/suggestions, feel free to cherry-pick the ones you
want and push the result.

On 2018-06-06 11:16 AM, Alan Hayward wrote:
> @@ -2243,6 +2297,9 @@ aarch64_pseudo_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
>      return group == all_reggroup || group == vector_reggroup;
>    else if (regnum >= AARCH64_B0_REGNUM && regnum < AARCH64_B0_REGNUM + 32)
>      return group == all_reggroup || group == vector_reggroup;
> +  else if (tdep->has_sve () && regnum >= AARCH64_SVE_V0_REGNUM
> +	   && regnum < AARCH64_SVE_V0_REGNUM + 32)

Here you use the magical "32" number but in aarch64_pseudo_register_name you used
AARCH64_V_REGS_NUM to refer (I think) to the same number.  Would it be good to use
AARCH64_V_REGS_NUM everywhere?  It might be good to extract that condition in a
function:

static bool
is_sve_regnum (int regnum)
{
  return (regnum >= AARCH64_SVE_V0_REGNUM
	  && regnum < AARCH64_SVE_V0_REGNUM + AARCH64_V_REGS_NUM);
}

and use it throughout.

>  /* Helper for aarch64_pseudo_write.  */
>  
>  static void
> -aarch64_pseudo_write_1 (struct regcache *regcache, int regnum_offset,
> -			int regsize, const gdb_byte *buf)
> +aarch64_pseudo_write_1 (struct gdbarch *gdbarch, struct regcache *regcache,
> +			int regnum_offset, int regsize, const gdb_byte *buf)

You could use the gdbarch from regcache.

>  {
> -  gdb_byte reg_buf[V_REGISTER_SIZE];
>    unsigned v_regnum = AARCH64_V0_REGNUM + regnum_offset;
>  
> +  /* Enough space for a full vector register.  */
> +  gdb_byte reg_buf[register_size (gdbarch, AARCH64_V0_REGNUM)];
> +  gdb_assert (AARCH64_V0_REGNUM == AARCH64_SVE_Z0_REGNUM);

This is checking a static assertion.  You could use static_assert instead, which
produces a compilation error if false.  You can either leave it here or put it
next to the aarch64_regnum definition.  I have seen the same gdb_assert somewhere
else too, with a static_assert you only need one.

Thanks,

Simon
Alan Hayward June 7, 2018, 9:34 a.m. UTC | #2 
Thanks for the review. Updated as below and pushed.


> On 6 Jun 2018, at 23:17, Simon Marchi <simon.marchi@ericsson.com> wrote:

> 

> Hi Alan,

> 

> I have some more nits/suggestions, feel free to cherry-pick the ones you

> want and push the result.

> 

> On 2018-06-06 11:16 AM, Alan Hayward wrote:

>> @@ -2243,6 +2297,9 @@ aarch64_pseudo_register_reggroup_p (struct gdbarch *gdbarch, int regnum,

>>     return group == all_reggroup || group == vector_reggroup;

>>   else if (regnum >= AARCH64_B0_REGNUM && regnum < AARCH64_B0_REGNUM + 32)

>>     return group == all_reggroup || group == vector_reggroup;

>> +  else if (tdep->has_sve () && regnum >= AARCH64_SVE_V0_REGNUM

>> +	   && regnum < AARCH64_SVE_V0_REGNUM + 32)

> 

> Here you use the magical "32" number but in aarch64_pseudo_register_name you used

> AARCH64_V_REGS_NUM to refer (I think) to the same number.  Would it be good to use

> AARCH64_V_REGS_NUM everywhere?  It might be good to extract that condition in a

> function:

> 

> static bool

> is_sve_regnum (int regnum)

> {

>  return (regnum >= AARCH64_SVE_V0_REGNUM

> 	  && regnum < AARCH64_SVE_V0_REGNUM + AARCH64_V_REGS_NUM);

> }

> 

> and use it throughout.


Updated to use AARCH64_V_REGS_NUM.

I originally used 32 because I was matching the style of the code above which
uses 32. Agreed in this case makes more sense to use AARCH64_V_REGS_NUM.

Didn’t add the func, as here we’re checking against AARCH64_SVE_V0_REGNUM
and not AARCH64_V0_REGNUM.

> 

>> /* Helper for aarch64_pseudo_write.  */

>> 

>> static void

>> -aarch64_pseudo_write_1 (struct regcache *regcache, int regnum_offset,

>> -			int regsize, const gdb_byte *buf)

>> +aarch64_pseudo_write_1 (struct gdbarch *gdbarch, struct regcache *regcache,

>> +			int regnum_offset, int regsize, const gdb_byte *buf)

> 

> You could use the gdbarch from regcache.


Not changed.

Make sense, but regcache is already being passed into aarch64_pseudo_write (which
the function type is defined in gdbarch). Easier to just pass it straight down
given the compiler would have already have put gdbarch in a register.

> 

>> {

>> -  gdb_byte reg_buf[V_REGISTER_SIZE];

>>   unsigned v_regnum = AARCH64_V0_REGNUM + regnum_offset;

>> 

>> +  /* Enough space for a full vector register.  */

>> +  gdb_byte reg_buf[register_size (gdbarch, AARCH64_V0_REGNUM)];

>> +  gdb_assert (AARCH64_V0_REGNUM == AARCH64_SVE_Z0_REGNUM);

> 

> This is checking a static assertion.  You could use static_assert instead, which

> produces a compilation error if false.  You can either leave it here or put it

> next to the aarch64_regnum definition.  I have seen the same gdb_assert somewhere

> else too, with a static_assert you only need one.

> 


Updated to gdb_static_assert. Left both of them in the code at the same place (there’s
one in the read func and one in the write func).

I wanted something explicit in this function because these are (I think) the only
two functions that rely on the defines being the same.


Thanks,
Alan.
diff mbox

Patch

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index c6b1e2a9a3..2fe6a4006c 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -69,6 +69,7 @@ 
 #define AARCH64_S0_REGNUM (AARCH64_D0_REGNUM + 32)
 #define AARCH64_H0_REGNUM (AARCH64_S0_REGNUM + 32)
 #define AARCH64_B0_REGNUM (AARCH64_H0_REGNUM + 32)
+#define AARCH64_SVE_V0_REGNUM (AARCH64_B0_REGNUM + 32)
 
 /* All possible aarch64 target descriptors.  */
 struct target_desc *tdesc_aarch64_list[AARCH64_MAX_SVE_VQ + 1];
@@ -1766,6 +1767,30 @@  aarch64_vnb_type (struct gdbarch *gdbarch)
   return tdep->vnb_type;
 }
 
+/* Return the type for an AdvSISD V register.  */
+
+static struct type *
+aarch64_vnv_type (struct gdbarch *gdbarch)
+{
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  if (tdep->vnv_type == NULL)
+    {
+      struct type *t = arch_composite_type (gdbarch, "__gdb_builtin_type_vnv",
+					    TYPE_CODE_UNION);
+
+      append_composite_type_field (t, "d", aarch64_vnd_type (gdbarch));
+      append_composite_type_field (t, "s", aarch64_vns_type (gdbarch));
+      append_composite_type_field (t, "h", aarch64_vnh_type (gdbarch));
+      append_composite_type_field (t, "b", aarch64_vnb_type (gdbarch));
+      append_composite_type_field (t, "q", aarch64_vnq_type (gdbarch));
+
+      tdep->vnv_type = t;
+    }
+
+  return tdep->vnv_type;
+}
+
 /* Implement the "dwarf2_reg_to_regnum" gdbarch method.  */
 
 static int
@@ -2114,6 +2139,8 @@  aarch64_gen_return_address (struct gdbarch *gdbarch,
 static const char *
 aarch64_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
   static const char *const q_name[] =
     {
       "q0", "q1", "q2", "q3",
@@ -2191,6 +2218,25 @@  aarch64_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
   if (regnum >= AARCH64_B0_REGNUM && regnum < AARCH64_B0_REGNUM + 32)
     return b_name[regnum - AARCH64_B0_REGNUM];
 
+  if (tdep->has_sve ())
+    {
+      static const char *const sve_v_name[] =
+	{
+	  "v0", "v1", "v2", "v3",
+	  "v4", "v5", "v6", "v7",
+	  "v8", "v9", "v10", "v11",
+	  "v12", "v13", "v14", "v15",
+	  "v16", "v17", "v18", "v19",
+	  "v20", "v21", "v22", "v23",
+	  "v24", "v25", "v26", "v27",
+	  "v28", "v29", "v30", "v31",
+	};
+
+      if (regnum >= AARCH64_SVE_V0_REGNUM
+	  && regnum < AARCH64_SVE_V0_REGNUM + AARCH64_V_REGS_NUM)
+	return sve_v_name[regnum - AARCH64_SVE_V0_REGNUM];
+    }
+
   internal_error (__FILE__, __LINE__,
 		  _("aarch64_pseudo_register_name: bad register number %d"),
 		  regnum);
@@ -2201,6 +2247,8 @@  aarch64_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
 static struct type *
 aarch64_pseudo_register_type (struct gdbarch *gdbarch, int regnum)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
   regnum -= gdbarch_num_regs (gdbarch);
 
   if (regnum >= AARCH64_Q0_REGNUM && regnum < AARCH64_Q0_REGNUM + 32)
@@ -2218,6 +2266,10 @@  aarch64_pseudo_register_type (struct gdbarch *gdbarch, int regnum)
   if (regnum >= AARCH64_B0_REGNUM && regnum < AARCH64_B0_REGNUM + 32)
     return aarch64_vnb_type (gdbarch);
 
+  if (tdep->has_sve () && regnum >= AARCH64_SVE_V0_REGNUM
+      && regnum < AARCH64_SVE_V0_REGNUM + 32)
+    return aarch64_vnv_type (gdbarch);
+
   internal_error (__FILE__, __LINE__,
 		  _("aarch64_pseudo_register_type: bad register number %d"),
 		  regnum);
@@ -2229,6 +2281,8 @@  static int
 aarch64_pseudo_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
 				    struct reggroup *group)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
   regnum -= gdbarch_num_regs (gdbarch);
 
   if (regnum >= AARCH64_Q0_REGNUM && regnum < AARCH64_Q0_REGNUM + 32)
@@ -2243,6 +2297,9 @@  aarch64_pseudo_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
     return group == all_reggroup || group == vector_reggroup;
   else if (regnum >= AARCH64_B0_REGNUM && regnum < AARCH64_B0_REGNUM + 32)
     return group == all_reggroup || group == vector_reggroup;
+  else if (tdep->has_sve () && regnum >= AARCH64_SVE_V0_REGNUM
+	   && regnum < AARCH64_SVE_V0_REGNUM + 32)
+    return group == all_reggroup || group == vector_reggroup;
 
   return group == all_reggroup;
 }
@@ -2250,17 +2307,22 @@  aarch64_pseudo_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
 /* Helper for aarch64_pseudo_read_value.  */
 
 static struct value *
-aarch64_pseudo_read_value_1 (readable_regcache *regcache, int regnum_offset,
+aarch64_pseudo_read_value_1 (struct gdbarch *gdbarch,
+			     readable_regcache *regcache, int regnum_offset,
 			     int regsize, struct value *result_value)
 {
-  gdb_byte reg_buf[V_REGISTER_SIZE];
   unsigned v_regnum = AARCH64_V0_REGNUM + regnum_offset;
 
+  /* Enough space for a full vector register.  */
+  gdb_byte reg_buf[register_size (gdbarch, AARCH64_V0_REGNUM)];
+  gdb_assert (AARCH64_V0_REGNUM == AARCH64_SVE_Z0_REGNUM);
+
   if (regcache->raw_read (v_regnum, reg_buf) != REG_VALID)
     mark_value_bytes_unavailable (result_value, 0,
 				  TYPE_LENGTH (value_type (result_value)));
   else
     memcpy (value_contents_raw (result_value), reg_buf, regsize);
+
   return result_value;
  }
 
@@ -2270,6 +2332,7 @@  static struct value *
 aarch64_pseudo_read_value (struct gdbarch *gdbarch, readable_regcache *regcache,
 			   int regnum)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   struct value *result_value = allocate_value (register_type (gdbarch, regnum));
 
   VALUE_LVAL (result_value) = lval_register;
@@ -2278,42 +2341,56 @@  aarch64_pseudo_read_value (struct gdbarch *gdbarch, readable_regcache *regcache,
   regnum -= gdbarch_num_regs (gdbarch);
 
   if (regnum >= AARCH64_Q0_REGNUM && regnum < AARCH64_Q0_REGNUM + 32)
-    return aarch64_pseudo_read_value_1 (regcache, regnum - AARCH64_Q0_REGNUM,
+    return aarch64_pseudo_read_value_1 (gdbarch, regcache,
+					regnum - AARCH64_Q0_REGNUM,
 					Q_REGISTER_SIZE, result_value);
 
   if (regnum >= AARCH64_D0_REGNUM && regnum < AARCH64_D0_REGNUM + 32)
-    return aarch64_pseudo_read_value_1 (regcache, regnum - AARCH64_D0_REGNUM,
+    return aarch64_pseudo_read_value_1 (gdbarch, regcache,
+					regnum - AARCH64_D0_REGNUM,
 					D_REGISTER_SIZE, result_value);
 
   if (regnum >= AARCH64_S0_REGNUM && regnum < AARCH64_S0_REGNUM + 32)
-    return aarch64_pseudo_read_value_1 (regcache, regnum - AARCH64_S0_REGNUM,
+    return aarch64_pseudo_read_value_1 (gdbarch, regcache,
+					regnum - AARCH64_S0_REGNUM,
 					S_REGISTER_SIZE, result_value);
 
   if (regnum >= AARCH64_H0_REGNUM && regnum < AARCH64_H0_REGNUM + 32)
-    return aarch64_pseudo_read_value_1 (regcache, regnum - AARCH64_H0_REGNUM,
+    return aarch64_pseudo_read_value_1 (gdbarch, regcache,
+					regnum - AARCH64_H0_REGNUM,
 					H_REGISTER_SIZE, result_value);
 
   if (regnum >= AARCH64_B0_REGNUM && regnum < AARCH64_B0_REGNUM + 32)
-    return aarch64_pseudo_read_value_1 (regcache, regnum - AARCH64_B0_REGNUM,
+    return aarch64_pseudo_read_value_1 (gdbarch, regcache,
+					regnum - AARCH64_B0_REGNUM,
 					B_REGISTER_SIZE, result_value);
 
+  if (tdep->has_sve () && regnum >= AARCH64_SVE_V0_REGNUM
+      && regnum < AARCH64_SVE_V0_REGNUM + 32)
+    return aarch64_pseudo_read_value_1 (gdbarch, regcache,
+					regnum - AARCH64_SVE_V0_REGNUM,
+					V_REGISTER_SIZE, result_value);
+
   gdb_assert_not_reached ("regnum out of bound");
 }
 
 /* Helper for aarch64_pseudo_write.  */
 
 static void
-aarch64_pseudo_write_1 (struct regcache *regcache, int regnum_offset,
-			int regsize, const gdb_byte *buf)
+aarch64_pseudo_write_1 (struct gdbarch *gdbarch, struct regcache *regcache,
+			int regnum_offset, int regsize, const gdb_byte *buf)
 {
-  gdb_byte reg_buf[V_REGISTER_SIZE];
   unsigned v_regnum = AARCH64_V0_REGNUM + regnum_offset;
 
+  /* Enough space for a full vector register.  */
+  gdb_byte reg_buf[register_size (gdbarch, AARCH64_V0_REGNUM)];
+  gdb_assert (AARCH64_V0_REGNUM == AARCH64_SVE_Z0_REGNUM);
+
   /* Ensure the register buffer is zero, we want gdb writes of the
      various 'scalar' pseudo registers to behavior like architectural
      writes, register width bytes are written the remainder are set to
      zero.  */
-  memset (reg_buf, 0, sizeof (reg_buf));
+  memset (reg_buf, 0, register_size (gdbarch, AARCH64_V0_REGNUM));
 
   memcpy (reg_buf, buf, regsize);
   regcache->raw_write (v_regnum, reg_buf);
@@ -2325,27 +2402,39 @@  static void
 aarch64_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
 		      int regnum, const gdb_byte *buf)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   regnum -= gdbarch_num_regs (gdbarch);
 
   if (regnum >= AARCH64_Q0_REGNUM && regnum < AARCH64_Q0_REGNUM + 32)
-    return aarch64_pseudo_write_1 (regcache, regnum - AARCH64_Q0_REGNUM,
-				   Q_REGISTER_SIZE, buf);
+    return aarch64_pseudo_write_1 (gdbarch, regcache,
+				   regnum - AARCH64_Q0_REGNUM, Q_REGISTER_SIZE,
+				   buf);
 
   if (regnum >= AARCH64_D0_REGNUM && regnum < AARCH64_D0_REGNUM + 32)
-    return aarch64_pseudo_write_1 (regcache, regnum - AARCH64_D0_REGNUM,
-				   D_REGISTER_SIZE, buf);
+    return aarch64_pseudo_write_1 (gdbarch, regcache,
+				   regnum - AARCH64_D0_REGNUM, D_REGISTER_SIZE,
+				   buf);
 
   if (regnum >= AARCH64_S0_REGNUM && regnum < AARCH64_S0_REGNUM + 32)
-    return aarch64_pseudo_write_1 (regcache, regnum - AARCH64_S0_REGNUM,
-				   S_REGISTER_SIZE, buf);
+    return aarch64_pseudo_write_1 (gdbarch, regcache,
+				   regnum - AARCH64_S0_REGNUM, S_REGISTER_SIZE,
+				   buf);
 
   if (regnum >= AARCH64_H0_REGNUM && regnum < AARCH64_H0_REGNUM + 32)
-    return aarch64_pseudo_write_1 (regcache, regnum - AARCH64_H0_REGNUM,
-				   H_REGISTER_SIZE, buf);
+    return aarch64_pseudo_write_1 (gdbarch, regcache,
+				   regnum - AARCH64_H0_REGNUM, H_REGISTER_SIZE,
+				   buf);
 
   if (regnum >= AARCH64_B0_REGNUM && regnum < AARCH64_B0_REGNUM + 32)
-    return aarch64_pseudo_write_1 (regcache, regnum - AARCH64_B0_REGNUM,
-				   B_REGISTER_SIZE, buf);
+    return aarch64_pseudo_write_1 (gdbarch, regcache,
+				   regnum - AARCH64_B0_REGNUM, B_REGISTER_SIZE,
+				   buf);
+
+  if (tdep->has_sve () && regnum >= AARCH64_SVE_V0_REGNUM
+      && regnum < AARCH64_SVE_V0_REGNUM + 32)
+    return aarch64_pseudo_write_1 (gdbarch, regcache,
+				   regnum - AARCH64_SVE_V0_REGNUM,
+				   V_REGISTER_SIZE, buf);
 
   gdb_assert_not_reached ("regnum out of bound");
 }
diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h
index 598a0aafa2..5a319551e6 100644
--- a/gdb/aarch64-tdep.h
+++ b/gdb/aarch64-tdep.h
@@ -70,6 +70,7 @@  struct gdbarch_tdep
   struct type *vns_type;
   struct type *vnh_type;
   struct type *vnb_type;
+  struct type *vnv_type;
 
   /* syscall record.  */
   int (*aarch64_syscall_record) (struct regcache *regcache, unsigned long svc_number);