Re: [PATCH 3/11] Add MIPS_MAX_REGISTER_SIZE

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

> On 4 Apr 2017, at 16:59, John Baldwin <jhb@freebsd.org> wrote:
> 
> On Tuesday, April 04, 2017 10:12:39 AM Alan Hayward wrote:
>> Max size set to 64bits, which I determined using the files regformats/mips*.dat
>> 
>> Tested on a --enable-targets=all build using make check with board files
>> unix and native-gdbserver.
>> 
>> I do not have a MIPS machine to test on.
>> 
>> Ok to commit?
> 
> I don't know how much we (GDB) care, but keep in mind that this does mean that
> these constants have to be updated as architectures change.  In my case I'm
> working on a research CPU that extends MIPS with some 128 and 256-bit registers.
> This means that in my GDB patches for this processor I will need to bump this
> constant explicitly.  That's probably not the end of the world, but this approach
> of per-arch constants vs a global constant does make that sort of thing slightly
> more complex to handle.

Understood.
The problem I’m trying to solve here is that the SVE extension for Aarch64 will
have a maximum register size of 2048 bits, which could increase in the future too.
We didn’t want to have to make MAX_REGISTER_SIZE that big!

> 
>> Alan.
>> 
>> 2017-04-04  Alan Hayward  <alan.hayward@arm.com>
>> 
>> 	* mips-fbsd-tdep.c (mips_fbsd_supply_reg): Use MIPS_MAX_REGISTER_SIZE.
>> 	(mips_fbsd_collect_reg): Likewise.
>> 	* mips-linux-tdep.c (supply_32bit_reg): Likewise.
>> 	(mips_supply_gregset): Likewise.
>> 	(mips_supply_fpregset): Likewise.
>> 	(mips64_supply_gregset): Likewise.
>> 	(mips64_fill_gregset): Likewise.
>> 	(mips64_fill_fpregset): Likewise.
>> 	* mips-tdep.c (mips_eabi_push_dummy_call): Likewise.
>> 	(mips_o32_return_value): Likewise.
>> 	(print_gp_register_row): Likewise.
>> 	* mips-tdep.h (MIPS_MAX_REGISTER_SIZE): Add
>> 
>> 
>> 
>> diff --git a/gdb/mips-fbsd-tdep.c b/gdb/mips-fbsd-tdep.c
>> index 00fae0ec60ddc9e645d3236efe29f2f9e9ceab5c..cb696f7318a9da176fee2693e484ecf48346712c 100644
>> --- a/gdb/mips-fbsd-tdep.c
>> +++ b/gdb/mips-fbsd-tdep.c
>> @@ -63,7 +63,7 @@ mips_fbsd_supply_reg (struct regcache *regcache, int regnum, const void *addr,
>>   else
>>     {
>>       enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>> -      gdb_byte buf[MAX_REGISTER_SIZE];
>> +      gdb_byte buf[MIPS_MAX_REGISTER_SIZE];
>>       LONGEST val;
>> 
>>       val = extract_signed_integer ((const gdb_byte *) addr, len, byte_order);
>> @@ -90,7 +90,7 @@ mips_fbsd_collect_reg (const struct regcache *regcache, int regnum, void *addr,
>>   else
>>     {
>>       enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>> -      gdb_byte buf[MAX_REGISTER_SIZE];
>> +      gdb_byte buf[MIPS_MAX_REGISTER_SIZE];
>>       LONGEST val;
>> 
>>       regcache_raw_collect (regcache, regnum, buf);
> 
> This part is ok with me, but I can't approve changes so you will need
> additional review from an approver.
> 

Ok, thanks for looking.