Re: [PATCH 3/11] Add MIPS_MAX_REGISTER_SIZE

Alan Hayward
2017-04-05T10:27:14Z

> 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.