Re: [PATCH] MIPS: Handle the DSP registers for bare metal

[Pedro Alves <palves@redhat.com>]

On 12/18/2014 01:25 PM, Yao Qi wrote:

> 
> gdb:
> 
> 2014-12-18  Maciej W. Rozycki  <macro@codesourcery.com>
> 	    Chris Dearman  <chris@mips.com>
> 
> 	* mips-tdep.c (NUM_MIPS_PROCESSOR_REGS): Set from
> 	MIPS_LAST_EMBED_REGNUM.
> 	(mips_gdbarch_init): Add embedded DSP support.
> 	* mips-tdep.h (MIPS_EMBED_CP2_REGNUM): Offset to CP2 registers.
> 	(MIPS_EMBED_DSPACC_REGNUM): Offset to DSP accumulator registers.
> 	(MIPS_EMBED_DSPCTL_REGNUM): Offset to DSP control registers.
> 	(MIPS_LAST_EMBED_REGNUM): Update accordingly.
> 	(MIPS_EMBED_NUM_REGS): New value to make sure that an even
> 	number of registers is used.
> ---
>  gdb/mips-tdep.c | 15 ++++++++++-----
>  gdb/mips-tdep.h | 23 ++++++++++++++++++++---
>  2 files changed, 30 insertions(+), 8 deletions(-)
> 
> diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
> index 60f43ac..1fdb216 100644
> --- a/gdb/mips-tdep.c
> +++ b/gdb/mips-tdep.c
> @@ -546,7 +546,7 @@ static struct cmd_list_element *showmipscmdlist = NULL;
>     are listed in the following tables.  */
>  
>  enum
> -{ NUM_MIPS_PROCESSOR_REGS = (90 - 32) };
> +{ NUM_MIPS_PROCESSOR_REGS = (MIPS_LAST_EMBED_REGNUM + 1 - 32) };
>  
>  /* Generic MIPS.  */
>  
> @@ -8191,7 +8191,7 @@ mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>    struct gdbarch_tdep *tdep;
>    int elf_flags;
>    enum mips_abi mips_abi, found_abi, wanted_abi;
> -  int i, num_regs;
> +  int i, num_regs, dsp_space;
>    enum mips_fpu_type fpu_type;
>    struct tdesc_arch_data *tdesc_data = NULL;
>    int elf_fpu_type = Val_GNU_MIPS_ABI_FP_ANY;
> @@ -8214,6 +8214,7 @@ mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>        mips_regnum.fp_implementation_revision = 70;
>        mips_regnum.dspacc = dspacc = -1;
>        mips_regnum.dspctl = dspctl = -1;
> +      dsp_space = 0;
>        num_regs = 71;
>        reg_names = mips_irix_reg_names;
>      }
> @@ -8231,6 +8232,7 @@ mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>        mips_regnum.dspctl = -1;
>        dspacc = 72;
>        dspctl = 78;
> +      dsp_space = 0;
>        num_regs = 79;
>        reg_names = mips_linux_reg_names;
>      }
> @@ -8244,8 +8246,9 @@ mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>        mips_regnum.fp0 = MIPS_EMBED_FP0_REGNUM;
>        mips_regnum.fp_control_status = 70;
>        mips_regnum.fp_implementation_revision = 71;
> -      mips_regnum.dspacc = dspacc = -1;
> -      mips_regnum.dspctl = dspctl = -1;
> +      mips_regnum.dspacc = dspacc = MIPS_EMBED_DSPACC_REGNUM;
> +      mips_regnum.dspctl = dspctl = MIPS_EMBED_DSPCTL_REGNUM;
> +      dsp_space = 1;
>        num_regs = MIPS_LAST_EMBED_REGNUM + 1;
>        if (info.bfd_arch_info != NULL
>            && info.bfd_arch_info->mach == bfd_mach_mips3900)
> @@ -8357,16 +8360,18 @@ mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>  	  /* The DSP registers are optional; it's OK if they are absent.  */
>  	  if (feature != NULL)
>  	    {
> -	      i = 0;
> +	      i = dsp_space;
>  	      valid_p = 1;
>  	      valid_p &= tdesc_numbered_register (feature, tdesc_data,
>  						  dspacc + i++, "hi1");
>  	      valid_p &= tdesc_numbered_register (feature, tdesc_data,
>  						  dspacc + i++, "lo1");
> +	      i += dsp_space;
>  	      valid_p &= tdesc_numbered_register (feature, tdesc_data,
>  						  dspacc + i++, "hi2");
>  	      valid_p &= tdesc_numbered_register (feature, tdesc_data,
>  						  dspacc + i++, "lo2");
> +	      i += dsp_space;
>  	      valid_p &= tdesc_numbered_register (feature, tdesc_data,
>  						  dspacc + i++, "hi3");
>  	      valid_p &= tdesc_numbered_register (feature, tdesc_data,

Took me a bit to grok this, but this is adding slack for ACXn, right?
But it seems like nothing in GDB knows about those ACX registers.  I
guess I'm being dense, but why is this patch needed then?  They should still
be accessible to the user even without this change, right?  Assuming the
description is including them.

Thanks,
Pedro Alves