[2/11] Add IA64_MAX_REGISTER_SIZE

Message ID 90F5717F-8685-4C74-B2E4-7317AF228034@arm.com
State New, archived
Headers

Commit Message

Alan Hayward April 11, 2017, 12:47 p.m. UTC
  > On 5 Apr 2017, at 11:00, Yao Qi <qiyaoltc@gmail.com> wrote:
> 
> Alan Hayward <Alan.Hayward@arm.com> writes:
> 
> Hi Alan,
> We have to define such macro if we have no other ways to remove
> MAX_REGISTER_SIZE.  AFAIK, there are some ways to remove many usages of
> MAX_REGISTER_SIZE.
> 
>> @@ -1516,7 +1516,7 @@ examine_prologue (CORE_ADDR pc, CORE_ADDR lim_pc,
>> 	  else if (qp == 0 && rN == 2
>> 	        && ((rM == fp_reg && fp_reg != 0) || rM == 12))
>> 	    {
>> -	      gdb_byte buf[MAX_REGISTER_SIZE];
>> +	      gdb_byte buf[IA64_MAX_REGISTER_SIZE];
>> 	      CORE_ADDR saved_sp = 0;
>> 	      /* adds r2, spilloffset, rFramePointer
>> 	           or
> 
> "buf" is used in the code below,
> 
> 		  get_frame_register (this_frame, sp_regnum, buf);
> 		  saved_sp = extract_unsigned_integer (buf, 8, byte_order);
> 
> why don't we use get_frame_register_unsigned, so the "buf" can be
> removed completely.
> 
>  saved_sp = get_frame_register_unsigned (this_frame, sp_regnum);
> 
>> @@ -2289,7 +2289,7 @@ static struct value *
>> ia64_sigtramp_frame_prev_register (struct frame_info *this_frame,
>> 				   void **this_cache, int regnum)
>> {
>> -  gdb_byte buf[MAX_REGISTER_SIZE];
>> +  gdb_byte buf[IA64_MAX_REGISTER_SIZE];
>> 
> 
> "buf" is used in the code below,
> 
> 	  read_memory (addr, buf, register_size (gdbarch, IA64_IP_REGNUM));
> 	  pc = extract_unsigned_integer (buf, 8, byte_order);
> 
> so it is for IP register.  Its size is 8-byte, so we can move "buf"
> here,
> 
>          gdb_byte buf[8];
> 
> 	  read_memory (addr, buf, register_size (gdbarch, IA64_IP_REGNUM));
> 	  pc = extract_unsigned_integer (buf, sizeof (buf), byte_order);
> 
> 
>>   struct gdbarch *gdbarch = get_frame_arch (this_frame);
>>   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>> @@ -2495,7 +2495,7 @@ ia64_access_reg (unw_addr_space_t as, unw_regnum_t uw_regnum, unw_word_t *val,
>>   struct gdbarch *gdbarch = get_frame_arch (this_frame);
>>   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>>   long new_sof, old_sof;
>> -  gdb_byte buf[MAX_REGISTER_SIZE];
>> +  gdb_byte buf[IA64_MAX_REGISTER_SIZE];
>> 
> 
> Use get_frame_register_{,un}signed, so we can remove "buf" completely.
> 
>>   /* We never call any libunwind routines that need to write registers.  */
>>   gdb_assert (!write);
>> @@ -2575,7 +2575,7 @@ ia64_access_rse_reg (unw_addr_space_t as, unw_regnum_t uw_regnum,
>>   struct gdbarch *gdbarch = get_regcache_arch (regcache);
>>   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>>   long new_sof, old_sof;
>> -  gdb_byte buf[MAX_REGISTER_SIZE];
>> +  gdb_byte buf[IA64_MAX_REGISTER_SIZE];
>> 
> 
> "buf" is used
> 
> 	regcache_cooked_read (regcache, IA64_IP_REGNUM, buf);
> 	ip = extract_unsigned_integer (buf, 8, byte_order);
> 
> so we can use regcache_cooked_read_unsigned,
> 
> 	regcache_cooked_read_unsigned (regcache, IA64_IP_REGNUM, &ip);
> 
>>   /* We never call any libunwind routines that need to write registers.  */
>>   gdb_assert (!write);
>> @@ -2982,7 +2982,7 @@ ia64_libunwind_frame_prev_register (struct frame_info *this_frame,
>> 	{
>> 	  int rrb_pr = 0;
>> 	  ULONGEST cfm;
>> -	  gdb_byte buf[MAX_REGISTER_SIZE];
>> +	  gdb_byte buf[IA64_MAX_REGISTER_SIZE];
>> 
> 
> Use get_frame_register_unsigned.
> 
>> 	  /* Fetch predicate register rename base from current frame
>> 	     marker for this frame.  */
> 
> The only leftover of MAX_REGISTER_SIZE is about floating type
> conversion, in ia64_register_to_value, ia64_push_dummy_call, etc.  Then,
> we can define an macro for the size of floating types, and replace
> MAX_REGISTER_SIZE with it.
> 
> -- 
> Yao (齐尧)

Patch updated to match all the changes suggested above.

Tested on a --enable-targets=all build using make check with board files
unix and native-gdbserver.

I do not have an IA64 machine to test on.
If there is any doubt of the validity of the changes then I'd suggest
applying the previous version of the patch (which just replaced a define).

Ok to commit?

Alan.

2017-04-11  Alan Hayward  <alan.hayward@arm.com>

	* ia64-tdep.c (IA64_MAX_FP_REGISTER_SIZE) Add.
	(ia64_register_to_value): Use IA64_MAX_FP_REGISTER_SIZE.
	(ia64_value_to_register): Likewise.
	(examine_prologue): Use get_frame_register_unsigned.
	(ia64_sigtramp_frame_prev_register): Likewise.
	(ia64_access_reg): Likewise.
	(ia64_access_rse_reg): Likewise.
	(ia64_libunwind_frame_prev_register): Likewise.
	(ia64_extract_return_value): Use IA64_MAX_FP_REGISTER_SIZE.
	(ia64_store_return_value): Likewise.
	(ia64_push_dummy_call): Likewise.
  

Comments

Yao Qi April 11, 2017, 5:17 p.m. UTC | #1
Alan Hayward <Alan.Hayward@arm.com> writes:

> @@ -2308,8 +2305,11 @@ ia64_sigtramp_frame_prev_register (struct frame_info *this_frame,
>
>        if (addr != 0)
>  	{
> -	  read_memory (addr, buf, register_size (gdbarch, IA64_IP_REGNUM));
> -	  pc = extract_unsigned_integer (buf, 8, byte_order);
> +	  gdb_byte buf[8];
> +	  struct gdbarch *gdbarch = get_frame_arch (this_frame);
> +	  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +	  read_memory (addr, buf, sizeof (buf));
> +	  pc = extract_unsigned_integer (buf, sizeof (buf), byte_order);

I just realize that we can use read_memory_unsigned_integer.

>  	}
>        pc &= ~0xf;
>        return frame_unwind_got_constant (this_frame, regnum, pc);

> @@ -2570,12 +2563,11 @@ ia64_access_rse_reg (unw_addr_space_t as, unw_regnum_t uw_regnum,
>  		     unw_word_t *val, int write, void *arg)
>  {
>    int regnum = ia64_uw2gdb_regnum (uw_regnum);
> -  unw_word_t bsp, sof, sol, cfm, psr, ip;
> +  ULONGEST bsp, sof, cfm, psr, ip;
>    struct regcache *regcache = (struct regcache *) arg;
>    struct gdbarch *gdbarch = get_regcache_arch (regcache);
>    enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>    long new_sof, old_sof;
> -  gdb_byte buf[MAX_REGISTER_SIZE];
>
>    /* We never call any libunwind routines that need to write registers.  */
>    gdb_assert (!write);
> @@ -2585,10 +2577,8 @@ ia64_access_rse_reg (unw_addr_space_t as, unw_regnum_t uw_regnum,
>        case UNW_REG_IP:
>  	/* Libunwind expects to see the pc value which means the slot number
>  	   from the psr must be merged with the ip word address.  */
> -	regcache_cooked_read (regcache, IA64_IP_REGNUM, buf);
> -	ip = extract_unsigned_integer (buf, 8, byte_order);
> -	regcache_cooked_read (regcache, IA64_PSR_REGNUM, buf);
> -	psr = extract_unsigned_integer (buf, 8, byte_order);
> +	ip = get_frame_register_unsigned (this_frame, IA64_IP_REGNUM);

In my last review, I suggested,

regcache_cooked_read_unsigned (regcache, IA64_IP_REGNUM, &ip);

but you still use get_frame_register_unsigned.  There is no variable
"this_frame" at all.

> +	psr = get_frame_register_unsigned (this_frame, IA64_PSR_REGNUM);
>  	*val = ip | ((psr >> 41) & 0x3);
>  	break;
>
  

Patch

diff --git a/gdb/ia64-tdep.c b/gdb/ia64-tdep.c
index 22e158866bbbf0d9457737ac973027521e2c1655..a0d33fea63dc0738bed2f73ba893d61bac770242 100644
--- a/gdb/ia64-tdep.c
+++ b/gdb/ia64-tdep.c
@@ -125,6 +125,9 @@  static CORE_ADDR ia64_find_global_pointer (struct gdbarch *gdbarch,

 #define NUM_IA64_RAW_REGS 462

+/* Big enough to hold the size of the largest FP register in bytes.  */
+#define IA64_MAX_FP_REGISTER_SIZE 16
+
 static int sp_regnum = IA64_GR12_REGNUM;

 /* NOTE: we treat the register stack registers r32-r127 as
@@ -1227,7 +1230,7 @@  ia64_register_to_value (struct frame_info *frame, int regnum,
 			int *optimizedp, int *unavailablep)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
-  gdb_byte in[MAX_REGISTER_SIZE];
+  gdb_byte in[IA64_MAX_FP_REGISTER_SIZE];

   /* Convert to TYPE.  */
   if (!get_frame_register_bytes (frame, regnum, 0,
@@ -1245,7 +1248,7 @@  ia64_value_to_register (struct frame_info *frame, int regnum,
                          struct type *valtype, const gdb_byte *in)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
-  gdb_byte out[MAX_REGISTER_SIZE];
+  gdb_byte out[IA64_MAX_FP_REGISTER_SIZE];
   convert_typed_floating (in, valtype, out, ia64_ext_type (gdbarch));
   put_frame_register (frame, regnum, out);
 }
@@ -1516,8 +1519,7 @@  examine_prologue (CORE_ADDR pc, CORE_ADDR lim_pc,
 	  else if (qp == 0 && rN == 2
 	        && ((rM == fp_reg && fp_reg != 0) || rM == 12))
 	    {
-	      gdb_byte buf[MAX_REGISTER_SIZE];
-	      CORE_ADDR saved_sp = 0;
+	      ULONGEST saved_sp = 0;
 	      /* adds r2, spilloffset, rFramePointer
 	           or
 		 adds r2, spilloffset, r12
@@ -1533,9 +1535,8 @@  examine_prologue (CORE_ADDR pc, CORE_ADDR lim_pc,
 	      if (this_frame)
 		{
 		  struct gdbarch *gdbarch = get_frame_arch (this_frame);
-		  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-		  get_frame_register (this_frame, sp_regnum, buf);
-		  saved_sp = extract_unsigned_integer (buf, 8, byte_order);
+		  saved_sp = get_frame_register_unsigned (this_frame,
+							  sp_regnum);
 		}
 	      spill_addr  = saved_sp
 	                  + (rM == 12 ? 0 : mem_stack_frame_size)
@@ -2289,10 +2290,6 @@  static struct value *
 ia64_sigtramp_frame_prev_register (struct frame_info *this_frame,
 				   void **this_cache, int regnum)
 {
-  gdb_byte buf[MAX_REGISTER_SIZE];
-
-  struct gdbarch *gdbarch = get_frame_arch (this_frame);
-  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   struct ia64_frame_cache *cache =
     ia64_sigtramp_frame_cache (this_frame, this_cache);

@@ -2308,8 +2305,11 @@  ia64_sigtramp_frame_prev_register (struct frame_info *this_frame,

       if (addr != 0)
 	{
-	  read_memory (addr, buf, register_size (gdbarch, IA64_IP_REGNUM));
-	  pc = extract_unsigned_integer (buf, 8, byte_order);
+	  gdb_byte buf[8];
+	  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+	  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+	  read_memory (addr, buf, sizeof (buf));
+	  pc = extract_unsigned_integer (buf, sizeof (buf), byte_order);
 	}
       pc &= ~0xf;
       return frame_unwind_got_constant (this_frame, regnum, pc);
@@ -2490,12 +2490,11 @@  ia64_access_reg (unw_addr_space_t as, unw_regnum_t uw_regnum, unw_word_t *val,
 		 int write, void *arg)
 {
   int regnum = ia64_uw2gdb_regnum (uw_regnum);
-  unw_word_t bsp, sof, sol, cfm, psr, ip;
+  ULONGEST bsp, sof, cfm, psr, ip;
   struct frame_info *this_frame = (struct frame_info *) arg;
   struct gdbarch *gdbarch = get_frame_arch (this_frame);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   long new_sof, old_sof;
-  gdb_byte buf[MAX_REGISTER_SIZE];

   /* We never call any libunwind routines that need to write registers.  */
   gdb_assert (!write);
@@ -2505,10 +2504,8 @@  ia64_access_reg (unw_addr_space_t as, unw_regnum_t uw_regnum, unw_word_t *val,
       case UNW_REG_IP:
 	/* Libunwind expects to see the pc value which means the slot number
 	   from the psr must be merged with the ip word address.  */
-	get_frame_register (this_frame, IA64_IP_REGNUM, buf);
-	ip = extract_unsigned_integer (buf, 8, byte_order);
-	get_frame_register (this_frame, IA64_PSR_REGNUM, buf);
-	psr = extract_unsigned_integer (buf, 8, byte_order);
+	ip = get_frame_register_unsigned (this_frame, IA64_IP_REGNUM);
+	psr = get_frame_register_unsigned (this_frame, IA64_PSR_REGNUM);
 	*val = ip | ((psr >> 41) & 0x3);
 	break;

@@ -2517,10 +2514,8 @@  ia64_access_reg (unw_addr_space_t as, unw_regnum_t uw_regnum, unw_word_t *val,
 	   register frame so we must account for the fact that
 	   ptrace() will return a value for bsp that points *after*
 	   the current register frame.  */
-	get_frame_register (this_frame, IA64_BSP_REGNUM, buf);
-	bsp = extract_unsigned_integer (buf, 8, byte_order);
-	get_frame_register (this_frame, IA64_CFM_REGNUM, buf);
-	cfm = extract_unsigned_integer (buf, 8, byte_order);
+	bsp = get_frame_register_unsigned (this_frame, IA64_BSP_REGNUM);
+	cfm = get_frame_register_unsigned (this_frame, IA64_CFM_REGNUM);
 	sof = gdbarch_tdep (gdbarch)->size_of_register_frame (this_frame, cfm);
 	*val = ia64_rse_skip_regs (bsp, -sof);
 	break;
@@ -2528,14 +2523,12 @@  ia64_access_reg (unw_addr_space_t as, unw_regnum_t uw_regnum, unw_word_t *val,
       case UNW_IA64_AR_BSPSTORE:
 	/* Libunwind wants bspstore to be after the current register frame.
 	   This is what ptrace() and gdb treats as the regular bsp value.  */
-	get_frame_register (this_frame, IA64_BSP_REGNUM, buf);
-	*val = extract_unsigned_integer (buf, 8, byte_order);
+	*val = get_frame_register_unsigned (this_frame, IA64_BSP_REGNUM);
 	break;

       default:
 	/* For all other registers, just unwind the value directly.  */
-	get_frame_register (this_frame, regnum, buf);
-	*val = extract_unsigned_integer (buf, 8, byte_order);
+	*val = get_frame_register_unsigned (this_frame, regnum);
 	break;
     }

@@ -2570,12 +2563,11 @@  ia64_access_rse_reg (unw_addr_space_t as, unw_regnum_t uw_regnum,
 		     unw_word_t *val, int write, void *arg)
 {
   int regnum = ia64_uw2gdb_regnum (uw_regnum);
-  unw_word_t bsp, sof, sol, cfm, psr, ip;
+  ULONGEST bsp, sof, cfm, psr, ip;
   struct regcache *regcache = (struct regcache *) arg;
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   long new_sof, old_sof;
-  gdb_byte buf[MAX_REGISTER_SIZE];

   /* We never call any libunwind routines that need to write registers.  */
   gdb_assert (!write);
@@ -2585,10 +2577,8 @@  ia64_access_rse_reg (unw_addr_space_t as, unw_regnum_t uw_regnum,
       case UNW_REG_IP:
 	/* Libunwind expects to see the pc value which means the slot number
 	   from the psr must be merged with the ip word address.  */
-	regcache_cooked_read (regcache, IA64_IP_REGNUM, buf);
-	ip = extract_unsigned_integer (buf, 8, byte_order);
-	regcache_cooked_read (regcache, IA64_PSR_REGNUM, buf);
-	psr = extract_unsigned_integer (buf, 8, byte_order);
+	ip = get_frame_register_unsigned (this_frame, IA64_IP_REGNUM);
+	psr = get_frame_register_unsigned (this_frame, IA64_PSR_REGNUM);
 	*val = ip | ((psr >> 41) & 0x3);
 	break;

@@ -2597,10 +2587,8 @@  ia64_access_rse_reg (unw_addr_space_t as, unw_regnum_t uw_regnum,
 	   register frame so we must account for the fact that
 	   ptrace() will return a value for bsp that points *after*
 	   the current register frame.  */
-	regcache_cooked_read (regcache, IA64_BSP_REGNUM, buf);
-	bsp = extract_unsigned_integer (buf, 8, byte_order);
-	regcache_cooked_read (regcache, IA64_CFM_REGNUM, buf);
-	cfm = extract_unsigned_integer (buf, 8, byte_order);
+	bsp = get_frame_register_unsigned (this_frame, IA64_BSP_REGNUM);
+	cfm = get_frame_register_unsigned (this_frame, IA64_CFM_REGNUM);
 	sof = (cfm & 0x7f);
 	*val = ia64_rse_skip_regs (bsp, -sof);
 	break;
@@ -2608,14 +2596,12 @@  ia64_access_rse_reg (unw_addr_space_t as, unw_regnum_t uw_regnum,
       case UNW_IA64_AR_BSPSTORE:
 	/* Libunwind wants bspstore to be after the current register frame.
 	   This is what ptrace() and gdb treats as the regular bsp value.  */
-	regcache_cooked_read (regcache, IA64_BSP_REGNUM, buf);
-	*val = extract_unsigned_integer (buf, 8, byte_order);
+	*val = get_frame_register_unsigned (this_frame, IA64_BSP_REGNUM);
 	break;

       default:
         /* For all other registers, just unwind the value directly.  */
-	regcache_cooked_read (regcache, regnum, buf);
-	*val = extract_unsigned_integer (buf, 8, byte_order);
+	*val = get_frame_register_unsigned (this_frame, regnum);
 	break;
     }

@@ -2982,12 +2968,10 @@  ia64_libunwind_frame_prev_register (struct frame_info *this_frame,
 	{
 	  int rrb_pr = 0;
 	  ULONGEST cfm;
-	  gdb_byte buf[MAX_REGISTER_SIZE];

 	  /* Fetch predicate register rename base from current frame
 	     marker for this frame.  */
-	  get_frame_register (this_frame, IA64_CFM_REGNUM, buf);
-	  cfm = extract_unsigned_integer (buf, 8, byte_order);
+	  cfm = get_frame_register_unsigned (this_frame, IA64_CFM_REGNUM);
 	  rrb_pr = (cfm >> 32) & 0x3f;

 	  /* Adjust the register number to account for register rotation.  */
@@ -3229,7 +3213,7 @@  ia64_extract_return_value (struct type *type, struct regcache *regcache,
   float_elt_type = is_float_or_hfa_type (type);
   if (float_elt_type != NULL)
     {
-      gdb_byte from[MAX_REGISTER_SIZE];
+      gdb_byte from[IA64_MAX_FP_REGISTER_SIZE];
       int offset = 0;
       int regnum = IA64_FR8_REGNUM;
       int n = TYPE_LENGTH (type) / TYPE_LENGTH (float_elt_type);
@@ -3294,7 +3278,7 @@  ia64_store_return_value (struct type *type, struct regcache *regcache,
   float_elt_type = is_float_or_hfa_type (type);
   if (float_elt_type != NULL)
     {
-      gdb_byte to[MAX_REGISTER_SIZE];
+      gdb_byte to[IA64_MAX_FP_REGISTER_SIZE];
       int offset = 0;
       int regnum = IA64_FR8_REGNUM;
       int n = TYPE_LENGTH (type) / TYPE_LENGTH (float_elt_type);
@@ -3856,7 +3840,7 @@  ia64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 	  len = TYPE_LENGTH (type);
 	  while (len > 0 && floatreg < IA64_FR16_REGNUM)
 	    {
-	      gdb_byte to[MAX_REGISTER_SIZE];
+	      gdb_byte to[IA64_MAX_FP_REGISTER_SIZE];
 	      convert_typed_floating (value_contents (arg) + argoffset,
 				      float_elt_type, to,
 				      ia64_ext_type (gdbarch));