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

Andreas Arnez
2014-07-16T13:01:01Z

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.