This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [commit] Take into account Xtensa TX
- From: Joel Brobecker <brobecker at adacore dot com>
- To: Maxim Grigoriev <maxim at tensilica dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Wed, 9 Mar 2011 07:46:08 +0400
- Subject: Re: [commit] Take into account Xtensa TX
- References: <4D76CDB9.8070902@tensilica.com>
> 2011-03-08 Maxim Grigoriev <maxim2405@gmail.com>
>
> * xtensa-tdep.c (TX_PS): New.
> (windowing_enabled): Update to count for Call0 ABI.
> (xtensa_hextochar): New.
> (xtensa_init_reggroups): Make algorithm generic.
> (xtensa_frame_cache): Use TX_PS on Tiny Xtensa.
Just a few minor comments...
> +/* On TX, hardware can be configured without Exception Option.
> + There is no PS register in this case. Inside XT-GDB, let us treat
> + it as a virtual read-only register always holding the same value. */
Not really an issue, but we don't need 2 spaces after commas, only
after periods.
> +static inline char xtensa_hextochar (int xdigit)
Formatting:
static inline char
xtensa_hextochar (int xdigit)
Also, we really would like to have every function documented.
While we can't go back and document everything that already
exists, we can try to make an effort for new functions...
> +{
> + static char hex[]="0123456789abcdef";
space around '='.
> + return hex[xdigit & 0x0f];
Why do you need the 0xf? Just a precaution? I would use a gdb_assert,
if you are about this. Note that instead of using a character array,
you can also return '0' + xdigit.
> + ps = (ps_regnum >= 0)
> + ? get_frame_register_unsigned (this_frame, ps_regnum) : TX_PS;
The GNU coding style recommends the use of parentheses in this case
(and the parens around ps_regnum >= 0 are superfluous. Thus:
ps = (ps_regnum >= 0
? get_frame_register_unsigned (this_frame, ps_regnum) : TX_PS)
> if (gdbarch_tdep (gdbarch)->call_abi != CallAbiCall0Only)
> {
> + ULONGEST val;
> ra = (bp_addr & 0x3fffffff) | 0x40000000;
> - regcache_raw_read (regcache, gdbarch_ps_regnum (gdbarch), buf);
Empty line between variable declarations and the first statement.
(this is a GDB convention)
--
Joel