This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] MIPS/Linux: DSP ASE support
- From: "Maciej W. Rozycki" <macro at codesourcery dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: <gdb-patches at sourceware dot org>, Chris Dearman <chris at mips dot com>, Joseph Myers <joseph at codesourcery dot com>
- Date: Thu, 1 Mar 2012 15:49:19 +0000
- Subject: Re: [PATCH] MIPS/Linux: DSP ASE support
- References: <alpine.DEB.1.10.1111221438190.4191@tp.orcam.me.uk> <4F4D3A7B.1030702@redhat.com>
Hi Pedro,
> > (mips64_linux_regsets_fetch_registers): Likewise.
> > (mips64_linux_regsets_store_registers): Likewise.
> > (mips64_linux_fetch_registers): Update call to
> > mips64_linux_regsets_fetch_registers.
> > (mips64_linux_store_registers): Update call to
> > mips64_linux_regsets_store_registers.
>
> Note tabs/spaces.
Thanks, I've noticed that too (and I fix up ChangeLog entries routinely
before committing as I frequently copy and paste them around which does
not necessarily preserve tabs).
> > * linux-low.h (linux_target_ops): Add regset_bitmap member.
> > * linux-low.c (use_linux_regsets): New macro.
> > (regsets_fetch_inferior_registers): Likewise.
> > (regsets_store_inferior_registers): Likewise.
>
> See <http://www.gnu.org/prep/standards/standards.html#Conditional-Changes>
> for what the standard says about conditional changes. In this case,
> you'd write something like:
>
> * linux-low.c (use_linux_regsets): New macro.
> [!HAVE_LINUX_REGSETS] (regsets_fetch_inferior_registers): Likewise.
> [!HAVE_LINUX_REGSETS] (regsets_store_inferior_registers): Likewise.
Good point, I'll remember that for future cases like this.
> > * gdb.xml/tdesc-regs.exp: Add "mips-dsp.xml" to the list of MIPS
> > core registers.
>
> Looks okay to me, but a change to the manual to document to new standard
> xml feature (org.gnu.gdb.mips.dsp) is missing.
I wasn't aware XML features are listed in the manual. Here's the
respective update. Any other concerns, or is the change otherwise OK?
While at it it's interesting to note that `fcsr' and `fir' (from the FPU
register set) are treated as 64-bit registers on 64-bit platforms, even
though architecturally these registers are always 32-bit. They are a part
of the coprocessor control register set and accessed with CFC1/CTC1
instructions that only ever transfer 32 bits of data (sign-extending them
on reads from bit #31, as usually, or truncating on writes on 64-bit
processors); there are no 64-bit counterparts like with MFC1/MTC1 vs
DMFC1/DMTC1. I wonder if that is an oversight or was a deliberate choice
for some reason, and what the impact could be if this was corrected.
Maciej
2012-01-01 Maciej W. Rozycki <macro@codesourcery.com>
doc/
* gdb.texinfo (MIPS Features): Add org.gnu.gdb.mips.dsp.
Index: gdb-fsf-trunk-quilt/gdb/doc/gdb.texinfo
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/doc/gdb.texinfo 2012-02-28 18:26:10.000000000 +0000
+++ gdb-fsf-trunk-quilt/gdb/doc/gdb.texinfo 2012-02-29 19:21:23.645605064 +0000
@@ -38624,6 +38624,11 @@ it may be optional in a future version o
contain registers @samp{f0} through @samp{f31}, @samp{fcsr}, and
@samp{fir}. They may be 32-bit or 64-bit depending on the target.
+The @samp{org.gnu.gdb.mips.dsp} feature is optional. It should
+contain registers @samp{hi1} through @samp{hi3}, @samp{lo1} through
+@samp{lo3}, and @samp{dspctl}. The @samp{dspctl} register should
+be 32-bit and the rest may be 32-bit or 64-bit depending on the target.
+
The @samp{org.gnu.gdb.mips.linux} feature is optional. It should
contain a single register, @samp{restart}, which is used by the
Linux kernel to control restartable syscalls.