[PATCH] Add support for Tilera TILE-Gx processor (part 1/3: gdb)
Jeff Kenton
jkenton@tilera.com
Tue May 29 15:24:00 GMT 2012
On 05/25/2012 11:17 AM, Pedro Alves wrote:
> On 05/24/2012 08:26 PM, Jeff Kenton wrote:
>
>> On 05/23/2012 01:33 PM, Pedro Alves wrote:
>>> Hi Jeff,
>>>
>>> On 05/21/2012 03:23 PM, Jeff Kenton wrote:
>>>
>>>
>> - Comments applied.
>> - SIGTRAMP frame handling improved.
>> - Patch attached.
>
> For the future, it's easier for the reviewer if you reply to the issues
> and questions raised. For example, in patch 2, I'm left wondering if the
> ps_get_thread_area addition fixed any testsuite failures, indicating it is
> correct. I'm also left wondering what does "improved" mean
> (probably fixed something in the testsuite?).
ps_get_thread_area didn't affect the testsuite results.
SIGTRAMP_FRAME improvements eliminated 65 FAILs in the sigstep tests (48
still FAIL -- I'm working on those).
>
>> diff -r -u -N /home/packages/gdb-7.4x/gdb/tilegx-linux-tdep.c ./gdb/tilegx-linux-tdep.c
>> --- /home/packages/gdb-7.4x/gdb/tilegx-linux-tdep.c 1969-12-31 19:00:00.000000000 -0500
>> +++ ./gdb/tilegx-linux-tdep.c 2012-05-24 13:43:30.089276000 -0400
>> +/* Supply raw registers from REGCACHE to REGS. */
>> +
>> +/* TILE-Gx has 56 general purpose registers,
>> + plus 8 special general purpose registers (network and ZERO),
>> + plus 1 magic register (PC == 64).
>> + We only need the following 2 enums here. */
>> +
>> +enum {
>> + E_NUM_EASY_REGS = 56,
>> + E_PC_REGNUM = 64
>> +};
>> +
>> +enum { tilegx_reg_size = 8 };
>
>
> I didn't pay close attention to what these two enums were before. The standard thing
> to do instead of defining tilegx_reg_size twice, and hacking up that short
> enum for the register numbers, is to move the register numbers enum from
> tilegx-tdep.c to and tilegx-tdep.h file, and include that where necessary,
> so we only have one place with the register numbers.
> 'E_' isn't a great prefix (does it mean 'enum'?), so please replace it with
> something like TILEGX_PC_REGNUM, etc. along with the move.
Done. Good suggestion.
>
>> +/* OS specific initialization of gdbarch. */
>> +
>> +static void
>> +tilegx_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>> +{
>> + int arch_size = 64;
>> +
>> + /* Handle arch_size == 32 or 64. Default to 64. */
>> + if (info.abfd)
>> + arch_size = bfd_get_arch_size (info.abfd);
>> +
>> + if (arch_size != 32)
>> + arch_size = 64;
>
> Why is this 'if' necessary? Does bfd_get_arch_size ever return anything
> not 32 or 64? In any case, you can just use gdbarch_addr_bit here,
> as tilegx_gdbarch_init has already done the similar bfd_get_arch_size bit.
Done.
>
>
>
>> +/* This is the implementation of gdbarch method breakpoint_from_pc. */
>> +
>> +static const unsigned char *
>> +tilegx_breakpoint_from_pc (struct gdbarch *gdbarch,
>> + CORE_ADDR *pcptr, int *lenptr)
>> +{
>> + /* 64-bit pattern for a { bpt ; nop } bundle. */
>> + static unsigned char breakpoint[] =
>
> static const.
Done.
>
>> + { 0x00, 0x50, 0x48, 0x51, 0xae, 0x44, 0x6a, 0x28 };
>> +
>> + *lenptr = sizeof (breakpoint);
>> + return breakpoint;
>> +}
>> +
> I think we're getting close. If Joel or anyone else has further comments,
> the next iteration should be about ready to go in.
>
Updated patch attached.
Thanks.
--jeff kenton
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: patch_1
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20120529/cac6e4f9/attachment.ksh>
More information about the Gdb-patches
mailing list