Re: MIPS: Handle the DSP registers

[Daniel Jacobowitz <drow at false dot org>]

On Mon, Dec 10, 2007 at 03:51:55PM +0000, Maciej W. Rozycki wrote:
> Hello,
> 
>  Here is code to support DSP registers as defined by the DSP ASE to the 
> MIPS Architecture.  This piece of code handles Linux and provides for 
> adding such support to embedded targets.  The ASE is optional (as are all 
> ASEs) and therefore some steps have been taken to handle this correctly.
> 
>  In particular, ptrace() calls may return a failure when called on a 
> system that does not support the ASE.  In such a case the registers are 
> marked as unavailable in the cache which has the effect of them being 
> hardwired to zero (I have a patch in the queue that would mark them 
> specially in the output; I will submit it later).

We have an even better way to handle this, nowadays.  Take a look at
arm-linux-nat.c:arm_linux_read_description.  This lets the registers
be completely hidden when they are not available.

> +/* Names of Linux registers.  */
> +static const char *mips_linux_reg_names[NUM_MIPS_PROCESSOR_REGS] = {
> +  "sr", "lo", "hi", "bad", "cause", "pc",
> +  "f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7",
> +  "f8", "f9", "f10", "f11", "f12", "f13", "f14", "f15",
> +  "f16", "f17", "f18", "f19", "f20", "f21", "f22", "f23",
> +  "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31",
> +  "fsr", "fir", "hi1", "lo1", "hi2", "lo2", "hi3", "lo3",
> +  "dspctl",
> +};

If you're using a target description wherever you support these
registers, then you shouldn't need another copy of them here.

> @@ -5270,11 +5341,11 @@
>  
>  
>        valid_p &= tdesc_numbered_register (feature, tdesc_data,
> -					  MIPS_EMBED_LO_REGNUM, "lo");
> +					  mips_regnum.lo, "lo");

I went to a bit of trouble to be able to use constants here, please
don't go back the other direction.  When all the raw registers use
constant numbering, it's much easier to manage the GDB backend.

> +      if (mips_regnum.dspacc >= 0)
> +	{
> +	  feature = tdesc_find_feature (info.target_desc,
> +					"org.gnu.gdb.mips.dsp");

Please add this to the manual section describing MIPS target features.

> Index: binutils-quilt/src/gdb/inf-ptrace.c
> ===================================================================
> --- binutils-quilt.orig/src/gdb/inf-ptrace.c	2007-12-07 15:08:21.000000000 +0000
> +++ binutils-quilt/src/gdb/inf-ptrace.c	2007-12-07 15:13:02.000000000 +0000

Using a target description that accurately describes the target should
eliminate the need for these.

> Index: binutils-quilt/src/gdb/features/mips-linux.xml
> ===================================================================
> --- binutils-quilt.orig/src/gdb/features/mips-linux.xml	2007-12-07 15:08:21.000000000 +0000
> +++ binutils-quilt/src/gdb/features/mips-linux.xml	2007-12-07 15:13:02.000000000 +0000
> @@ -11,6 +11,7 @@
>    <xi:include href="mips-cpu.xml"/>
>    <xi:include href="mips-cp0.xml"/>
>    <xi:include href="mips-fpu.xml"/>
> +  <xi:include href="mips-dsp.xml"/>

And a corrolary of that is that we'll have two mips-linux
descriptions, with and without the DSP registers.

> Index: binutils-quilt/src/gdb/testsuite/gdb.xml/tdesc-regs.exp
> ===================================================================
> --- binutils-quilt.orig/src/gdb/testsuite/gdb.xml/tdesc-regs.exp	2007-12-07 17:01:40.000000000 +0000
> +++ binutils-quilt/src/gdb/testsuite/gdb.xml/tdesc-regs.exp	2007-12-07 17:02:17.000000000 +0000
> @@ -33,7 +33,7 @@
>          set core-regs {arm-core.xml}
>      }
>      "mips*-*-*" {
> -	set core-regs {mips-cpu.xml mips-cp0.xml mips-fpu.xml}
> +	set core-regs {mips-cpu.xml mips-cp0.xml mips-fpu.xml mips-dsp.xml}
>      }
>      "powerpc*-*-*" {
>  	set regdir "rs6000/"

No need for this, since a MIPS target without the DSP registers is
still valid.

-- 
Daniel Jacobowitz
CodeSourcery