Re: [PATCH] Remove MAX_REGISTER_SIZE from regcache.c

[Alan Hayward <Alan.Hayward@arm.com>]

> On 5 Apr 2017, at 17:00, Yao Qi <qiyaoltc@gmail.com> wrote:

<snip Msp430 and nds32 changes>
Moved these out to a different patch

>> @@ -379,7 +388,7 @@ regcache_restore (struct regcache *dst,
>> 		  void *cooked_read_context)
>> {
>>   struct gdbarch *gdbarch = dst->descr->gdbarch;
>> -  gdb_byte buf[MAX_REGISTER_SIZE];
>> +  std::vector<gdb_byte> buf (max_register_size (gdbarch));
>>   int regnum;
>> 
>>   /* The dst had better not be read-only.  If it is, the `restore'
>> @@ -395,9 +404,9 @@ regcache_restore (struct regcache *dst,
>> 	{
>> 	  enum register_status status;
> 
> Can we move "buf" here? and initialize it with the register_size,
> 
>           std::vector<gdb_byte> buf (register_size (gdbarch, regnum));
> 
> then, we don't need max_register_size ().
> 

Problem with this is that we are then creating a brand new buffer for each
iteration of the loop, which is a little heavyweight.
We could create an empty buf outside the loop and re-size it each iteration,
but that's still going to cost.

We'll still need to keep max_register_size () if we want to add checks
that the FOO_MAX_REGISTER size defines are big enough.
(see the BFIN_MAX_REGISTER_SIZE email thread)



>> @@ -1480,17 +1488,19 @@ regcache_dump (struct regcache *regcache, struct ui_file *file,
>> 	    fprintf_unfiltered (file, "Cooked value");
>> 	  else
>> 	    {
>> -	      enum register_status status;
>> +	      struct value *value = regcache_cooked_read_value (regcache,
>> +								regnum);
>> 
>> -	      status = regcache_cooked_read (regcache, regnum, buf);
>> -	      if (status == REG_UNKNOWN)
>> -		fprintf_unfiltered (file, "<invalid>");
>> -	      else if (status == REG_UNAVAILABLE)
>> +	      if (value_optimized_out (value)
>> +		  || !value_entirely_available (value))
>> 		fprintf_unfiltered (file, "<unavailable>");
> 
> It is still not right to me.  With your changes to msp430 and nds32, we
> won't get REG_UNKNOWN for pseudo registers, but we may still get
> REG_UNKNOWN from raw registers (from regcache->register_status[]).  How
> is this?
> 
>  gdb_byte *buf = NULL;
>  enum register_status status;
>  struct value * value = NULL;
> 
>  if (regnum < regcache->descr->nr_raw_registers)
>    {
>      regcache_raw_update (regcache, regnum);
> 
>      status = regcache->register_status[regnum];
>      buf = register_buffer (regcache, regnum);
>    }
>  else
>   {
>      value = regcache_cooked_read_value (regcache, regnum);
> 
>      if (value_entirely_available (value))
>        {
>          status = REG_VALID;
>          buf = value_contents_all (value);
>        }
>      else
>        status = REG_REG_UNAVAILABLE;
>   }
> 
>   if (status == REG_UNKNOWN)
>      fprintf_unfiltered (file, "<invalid>");
>   else if (status == REG_UNAVAILABLE)
>      fprintf_unfiltered (file, "<unavailable>");
>   else
>       print_hex_chars (file, buf,
> 			 regcache->descr->sizeof_register[regnum],
> 			 gdbarch_byte_order (gdbarch));
> 
>   if (value != NULL)
>    {
>      release_value (value);
>      value_free (value);
>    }
> 

Yes, I’ll add those changes.