Re: [PATCH v2 11/13] TILEGX Linux: Fill 'collect_regset' in regset structure.

[Andreas Arnez <arnez at linux dot vnet dot ibm dot com>]

On Tue, Jul 15 2014, Ulrich Weigand wrote:

> Andreas Arnez wrote:
>
>> gdb/
>> 	* tilegx-linux-tdep.c (tilegx_linux_supply_regset): Delete
>> 	function.  Move logic to...
>> 	(tilegx_linux_regmap): ... this new register map.
>> 	(TILEGX_GREGS_SIZE): New enum value.
>> 	(tilegx_linux_regset): Refer to register map, replace supply
>> 	method by regcache_supply_regset, and add collect method.
>
>> +static const struct regcache_map_entry tilegx_linux_regmap[] =
>> +  {
>> +    { TILEGX_NUM_EASY_REGS, 0 },
>
> Maybe TILEGX_R0_REGNUM instead of 0?

Maybe.  Or, thinking about this again, I'd more clearly indicate the
group of "easy" registers with a new enum value

    TILEGX_FIRST_EASY_REGNUM = TILEGX_R0_REGNUM

OK?

>
>> +    { 1, TILEGX_PC_REGNUM },
>
> This seems to get the register size wrong in 32-bit mode; note
> that TILEGX_PC_REGNUM has type builtin_func_ptr, which may be
> 4 bytes in size ...

Hm, I didn't realize that 32-bit mode is actually supported.  Note that
tilegx_register_type() unconditionally assigns a 64-bit type to *all
other* registers:

  if (regnum == TILEGX_PC_REGNUM)
    return builtin_type (gdbarch)->builtin_func_ptr;
  else
    return builtin_type (gdbarch)->builtin_uint64;

Also, the original logic in tilegx_linux_supply_regset() maps an 8-byte
slot to each register.  In 32-bit mode, this behavior doesn't seem to
match the Linux kernel code, which -- if I interpret it correctly --
assigns the same type 'pt_reg_t' to all registers in pt_regs, and:

  pt_reg_t == uint_reg_t == __uint_reg_t == unsigned long

Thus, each register should be in a 4-byte slot, and the register type
for everything but the PC should be builtin_uint32, right?

Has GDB ever been used for 32-bit TILE-Gx core files?  Or am I getting
something wrong here?

>
>> +    { 1, TILEGX_FAULTNUM_REGNUM },
>> +    { 0 }
>> +  };
>>  
>> -      if (regnum == gri || regnum == -1)
>> -	regcache_raw_supply (regcache, gri, ptr);
>> -    }
>> -}
>> +enum
>> +{
>> +  TILEGX_GREGS_SIZE = (TILEGX_NUM_EASY_REGS + 2) * tilegx_reg_size
>> +};
>
> This doesn't appear to be used here.

Removed.