This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 4/4] GDB: New target s12z
- From: John Darrington <john at darrington dot wattle dot id dot au>
- To: Tom Tromey <tom at tromey dot com>
- Cc: John Darrington <john at darrington dot wattle dot id dot au>, gdb-patches at sourceware dot org
- Date: Sat, 8 Sep 2018 06:46:05 +0200
- Subject: Re: [PATCH 4/4] GDB: New target s12z
- References: <20180829141845.26378-1-john@darrington.wattle.id.au> <20180829141845.26378-5-john@darrington.wattle.id.au> <874lf1vuca.fsf@tromey.com>
On Fri, Sep 07, 2018 at 04:03:17PM -0600, Tom Tromey wrote:
>>>>> "John" == John Darrington <john@darrington.wattle.id.au> writes:
Thank you for the patch. It seems generally good. I have a few nits,
but nothing serious.
John> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
John> index 118c3c8062..f448d1ee19 100644
John> --- a/gdb/Makefile.in
John> +++ b/gdb/Makefile.in
John> @@ -758,6 +758,7 @@ ALL_TARGET_OBS = \
John> rs6000-lynx178-tdep.o \
John> rs6000-tdep.o \
John> rx-tdep.o \
John> + s12z-tdep.o \
This change should be mentioned in the ChangeLog.
John> +
John> +#define N_PHYSICAL_REGISTERS (S12Z_N_REGISTERS - 2)
John> +
John> +static const int reg_perm[N_PHYSICAL_REGISTERS] =
John> + {
New functions, comments, and macros should have an explanatory comment.
Sometimes this can be just a reference to some generic thing, like
implementations of gdbarch methods can refer to gdbarch.h. There are a
number of cases of this in the file.
John> + /* registers is declared in opcodes/s12z.h */
GNU comment style is to start with a capital letter and end with a
period and two spaces.
John> + if (0 != prologue_end)
John> + {
John> + struct symtab_and_line prologue_sal = find_pc_line (start_pc, 0);
John> + struct compunit_symtab *compunit
John> + = SYMTAB_COMPUNIT (prologue_sal.symtab);
John> + const char *debug_format = COMPUNIT_DEBUGFORMAT (compunit);
John> +
John> + if ((NULL != debug_format)
John> + && (strlen ("dwarf") <= strlen (debug_format))
John> + && (0 == strncasecmp ("dwarf", debug_format, strlen ("dwarf"))))
John> + return (prologue_end > pc) ? prologue_end : pc;
John> + }
Is this stuff useful?
I suspect not. In fact, I'm very dubious about the whole
skip_prologue_using_sal function. It seems to not work very well.
John> + /* JPB: 28-Apr-11. This is a temporary patch, to get round GDB
John> + crashing right at the beginning. Build the frame ID as best we
John> + can. */
John> + trad_frame_set_id (info, frame_id_build (this_sp, this_pc));
Could you explain this more?
I'm afraid I can't. This was code I took over from the or1k target.