This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Add support for Tilera TILE-Gx processor (part 1/3: gdb)


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

Attachment: patch_1
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]