[PATCH] Bug 20936 - provide sparc and sparcv9 target description XML files

Yao Qi qiyaoltc@gmail.com
Mon Dec 12 12:54:00 GMT 2016


On 16-12-06 23:57:58, Ivo Raisr wrote:
> For some strange reason, test suite changes have not been included
> in my last patch. Please see the latest version of the patch and
> ChangeLog entry. Kind regards, I.
> 

Hi Ivo,
Your patch does two orthogonal things IMO,

 - Pseudo register support enhancement, patch #1
 - XML target description support and sparc*-tdep.c updates, patch #2,

Can you split them to two patches?

> @@ -327,6 +331,18 @@ static const char *sparc32_pseudo_regist
>  #define SPARC32_NUM_PSEUDO_REGS ARRAY_SIZE (sparc32_pseudo_register_names)
>  
>  /* Return the name of register REGNUM.  */
> +static const char *
> +sparc32_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
> +{
> +  regnum -= gdbarch_num_regs (gdbarch);
> +
> +  if (regnum < SPARC32_NUM_PSEUDO_REGS)
> +    return sparc32_pseudo_register_names[regnum];
> +
> +  internal_error (__FILE__, __LINE__,
> +                  _("sparc32_pseudo_register_name: bad register number %d"),
> +                  regnum);
> +}
>  
>  static const char *
>  sparc32_register_name (struct gdbarch *gdbarch, int regnum)
> @@ -334,10 +350,10 @@ sparc32_register_name (struct gdbarch *g
>    if (regnum >= 0 && regnum < SPARC32_NUM_REGS)
>      return sparc32_register_names[regnum];
>  
> -  if (regnum < SPARC32_NUM_REGS + SPARC32_NUM_PSEUDO_REGS)
> -    return sparc32_pseudo_register_names[regnum - SPARC32_NUM_REGS];
> +  if (regnum >= gdbarch_num_regs (gdbarch))
> +    return sparc32_pseudo_register_name (gdbarch, regnum);
>  
> -  return NULL;
> +  return tdesc_register_name (gdbarch, regnum);
>  }

I prefer to using register names provided by target description, does
the code below work for you?

  if (tdesc_has_registers (gdbarch_target_desc (gdbarch)))
    return tdesc_register_name (gdbarch, rawnum);
  else if (regnum >= 0 && regnum < gdbarch_num_regs (gdbarch))
    return sparc32_register_names[regnum];
  else
    return sparc32_pseudo_register_name (gdbarch, regnum);


>  static struct type *
>  sparc32_register_type (struct gdbarch *gdbarch, int regnum)
> @@ -406,9 +434,6 @@ sparc32_register_type (struct gdbarch *g
>    if (regnum >= SPARC_F0_REGNUM && regnum <= SPARC_F31_REGNUM)
>      return builtin_type (gdbarch)->builtin_float;
>  
> -  if (regnum >= SPARC32_D0_REGNUM && regnum <= SPARC32_D30_REGNUM)
> -    return builtin_type (gdbarch)->builtin_double;
> -
>    if (regnum == SPARC_SP_REGNUM || regnum == SPARC_FP_REGNUM)
>      return builtin_type (gdbarch)->builtin_data_ptr;
>  
> @@ -421,6 +446,9 @@ sparc32_register_type (struct gdbarch *g
>    if (regnum == SPARC32_FSR_REGNUM)
>      return sparc_fsr_type (gdbarch);
>  
> +  if (regnum >= gdbarch_num_regs (gdbarch))
> +    return sparc32_pseudo_register_type (gdbarch, regnum);
> +
>    return builtin_type (gdbarch)->builtin_int32;
>  }

This can be moved to patch #1.  In patch #2, we can prefer register
types from target description, at the start of sparc32_register_type,
we can add this,

  /* If the XML description has register information, use that to
     determine the register type.  */
  if (tdesc_has_registers (gdbarch_target_desc (gdbarch)))
    return tdesc_register_type (gdbarch, regno);

>  
> @@ -431,6 +459,7 @@ sparc32_pseudo_register_read (struct gdb
>  {
>    enum register_status status;
>  
> +  regnum -= gdbarch_num_regs (gdbarch);
>    gdb_assert (regnum >= SPARC32_D0_REGNUM && regnum <= SPARC32_D30_REGNUM);
>  
>    regnum = SPARC_F0_REGNUM + 2 * (regnum - SPARC32_D0_REGNUM);
> @@ -445,6 +474,7 @@ sparc32_pseudo_register_write (struct gd
>  			       struct regcache *regcache,
>  			       int regnum, const gdb_byte *buf)
>  {
> +  regnum -= gdbarch_num_regs (gdbarch);
>    gdb_assert (regnum >= SPARC32_D0_REGNUM && regnum <= SPARC32_D30_REGNUM);
>  
>    regnum = SPARC_F0_REGNUM + 2 * (regnum - SPARC32_D0_REGNUM);
> @@ -1660,11 +1690,36 @@ sparc_iterate_over_regset_sections (stru
>  }
>  


They can be moved to patch #1.

>  
> -  /* Pseudo registers.  */
> +/* Pseudo registers.  */
> +enum sparc32_pseudo_regnum
> +{
>    SPARC32_D0_REGNUM,		/* %d0 */

Explicitly set it to zero.

>    SPARC32_D30_REGNUM		/* %d30 */
>    = SPARC32_D0_REGNUM + 15
> diff -Npur a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
> --- a/gdb/sparc64-tdep.c	2016-02-09 19:19:39.000000000 +0000
> +++ b/gdb/sparc64-tdep.c	2016-12-06 13:53:05.174301647 +0000
> @@ -31,6 +31,7 @@
>  #include "objfiles.h"
>  #include "osabi.h"
>  #include "regcache.h"
> +#include "target-descriptions.h"
>  #include "target.h"
>  #include "value.h"
>  
> @@ -226,28 +227,29 @@ sparc64_fprs_type (struct gdbarch *gdbar
>  
>  
>  /* Register information.  */
> +#define SPARC64_FPU_REGISTERS                             \
> +  "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", \
> +  "f32", "f34", "f36", "f38", "f40", "f42", "f44", "f46", \
> +  "f48", "f50", "f52", "f54", "f56", "f58", "f60", "f62"
> +#define SPARC64_CP0_REGISTERS                                             \
> +  "pc", "npc",                                                            \
> +  /* FIXME: Give "state" a name until we start using register groups.  */ \
> +  "state",                                                                \
> +  "fsr",                                                                  \
> +  "fprs",                                                                 \
> +  "y"
> +
> +static const char *sparc64_fpu_register_names[] = { SPARC64_FPU_REGISTERS };
> +static const char *sparc64_cp0_register_names[] = { SPARC64_CP0_REGISTERS };
>  
>  static const char *sparc64_register_names[] =
>  {
> -  "g0", "g1", "g2", "g3", "g4", "g5", "g6", "g7",
> -  "o0", "o1", "o2", "o3", "o4", "o5", "sp", "o7",
> -  "l0", "l1", "l2", "l3", "l4", "l5", "l6", "l7",
> -  "i0", "i1", "i2", "i3", "i4", "i5", "fp", "i7",
> -
> -  "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",
> -  "f32", "f34", "f36", "f38", "f40", "f42", "f44", "f46",
> -  "f48", "f50", "f52", "f54", "f56", "f58", "f60", "f62",
> -
> -  "pc", "npc",
> -  
> -  /* FIXME: Give "state" a name until we start using register groups.  */
> -  "state",
> -  "fsr",
> -  "fprs",
> -  "y",
> +  SPARC_CORE_REGISTERS,
> +  SPARC64_FPU_REGISTERS,
> +  SPARC64_CP0_REGISTERS
>  };
>  
>  /* Total number of registers.  */
> @@ -273,6 +275,18 @@ static const char *sparc64_pseudo_regist
>  #define SPARC64_NUM_PSEUDO_REGS ARRAY_SIZE (sparc64_pseudo_register_names)
>  
>  /* Return the name of register REGNUM.  */
> +static const char *
> +sparc64_pseudo_register_name (struct gdbarch *gdbarch, int regnum)
> +{
> +  regnum -= gdbarch_num_regs (gdbarch);
> +
> +  if (regnum < SPARC64_NUM_PSEUDO_REGS)
> +    return sparc64_pseudo_register_names[regnum];
> +
> +  internal_error (__FILE__, __LINE__,
> +                  _("sparc64_pseudo_register_name: bad register number %d"),
> +                  regnum);
> +}
>  
>  static const char *
>  sparc64_register_name (struct gdbarch *gdbarch, int regnum)
> @@ -280,15 +294,36 @@ sparc64_register_name (struct gdbarch *g
>    if (regnum >= 0 && regnum < SPARC64_NUM_REGS)
>      return sparc64_register_names[regnum];
>  
> -  if (regnum >= SPARC64_NUM_REGS
> -      && regnum < SPARC64_NUM_REGS + SPARC64_NUM_PSEUDO_REGS)
> -    return sparc64_pseudo_register_names[regnum - SPARC64_NUM_REGS];
> +  if (regnum >= gdbarch_num_regs (gdbarch))
> +    return sparc64_pseudo_register_name (gdbarch, regnum);
>  

Change like this can be moved to patch #1.

>    return NULL;
>  }
>  
>  /* Return the GDB type object for the "standard" data type of data in
>     register REGNUM.  */
> +static struct type *
> +sparc64_pseudo_register_type (struct gdbarch *gdbarch, int regnum)
> +{
> +  regnum -= gdbarch_num_regs (gdbarch);
> +
> +  if (regnum == SPARC64_CWP_REGNUM)
> +    return builtin_type (gdbarch)->builtin_int64;
> +  if (regnum == SPARC64_PSTATE_REGNUM)
> +    return sparc64_pstate_type (gdbarch);
> +  if (regnum == SPARC64_ASI_REGNUM)
> +    return builtin_type (gdbarch)->builtin_int64;
> +  if (regnum == SPARC64_CCR_REGNUM)
> +    return sparc64_ccr_type (gdbarch);
> +  if (regnum >= SPARC64_D0_REGNUM && regnum <= SPARC64_D62_REGNUM)
> +    return builtin_type (gdbarch)->builtin_double;
> +  if (regnum >= SPARC64_Q0_REGNUM && regnum <= SPARC64_Q60_REGNUM)
> +    return builtin_type (gdbarch)->builtin_long_double;
> +
> +  internal_error (__FILE__, __LINE__,
> +                  _("sparc64_pseudo_register_type: bad register number %d"),
> +                  regnum);
> +}
>  
>  static struct type *
>  sparc64_register_type (struct gdbarch *gdbarch, int regnum)
> @@ -319,19 +354,8 @@ sparc64_register_type (struct gdbarch *g
>      return builtin_type (gdbarch)->builtin_int64;
>  
>    /* Pseudo registers.  */
> -
> -  if (regnum == SPARC64_CWP_REGNUM)
> -    return builtin_type (gdbarch)->builtin_int64;
> -  if (regnum == SPARC64_PSTATE_REGNUM)
> -    return sparc64_pstate_type (gdbarch);
> -  if (regnum == SPARC64_ASI_REGNUM)
> -    return builtin_type (gdbarch)->builtin_int64;
> -  if (regnum == SPARC64_CCR_REGNUM)
> -    return builtin_type (gdbarch)->builtin_int64;
> -  if (regnum >= SPARC64_D0_REGNUM && regnum <= SPARC64_D62_REGNUM)
> -    return builtin_type (gdbarch)->builtin_double;
> -  if (regnum >= SPARC64_Q0_REGNUM && regnum <= SPARC64_Q60_REGNUM)
> -    return builtin_type (gdbarch)->builtin_long_double;
> +  if (regnum >= gdbarch_num_regs (gdbarch))
> +    return sparc64_pseudo_register_type (gdbarch, regnum);
>  
>    internal_error (__FILE__, __LINE__, _("invalid regnum"));
>  }
> @@ -344,7 +368,7 @@ sparc64_pseudo_register_read (struct gdb
>    enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>    enum register_status status;
>  
> -  gdb_assert (regnum >= SPARC64_NUM_REGS);
> +  regnum -= gdbarch_num_regs (gdbarch);
>  
>    if (regnum >= SPARC64_D0_REGNUM && regnum <= SPARC64_D30_REGNUM)
>      {
> @@ -421,7 +445,8 @@ sparc64_pseudo_register_write (struct gd
>  			       int regnum, const gdb_byte *buf)
>  {
>    enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> -  gdb_assert (regnum >= SPARC64_NUM_REGS);
> +
> +  regnum -= gdbarch_num_regs (gdbarch);
>  
>    if (regnum >= SPARC64_D0_REGNUM && regnum <= SPARC64_D30_REGNUM)
>      {
> @@ -638,6 +663,7 @@ static void
>  sparc64_store_floating_fields (struct regcache *regcache, struct type *type,
>  			       const gdb_byte *valbuf, int element, int bitpos)
>  {
> +  struct gdbarch *gdbarch = get_regcache_arch (regcache);
>    int len = TYPE_LENGTH (type);
>  
>    gdb_assert (element < 16);
> @@ -652,14 +678,15 @@ sparc64_store_floating_fields (struct re
>  	  gdb_assert (bitpos == 0);
>  	  gdb_assert ((element % 2) == 0);
>  
> -	  regnum = SPARC64_Q0_REGNUM + element / 2;
> +	  regnum = gdbarch_num_regs (gdbarch) + SPARC64_Q0_REGNUM + element / 2;
>  	  regcache_cooked_write (regcache, regnum, valbuf);
>  	}
>        else if (len == 8)
>  	{
>  	  gdb_assert (bitpos == 0 || bitpos == 64);
>  
> -	  regnum = SPARC64_D0_REGNUM + element + bitpos / 64;
> +	  regnum = gdbarch_num_regs (gdbarch) + SPARC64_D0_REGNUM
> +                   + element + bitpos / 64;
>  	  regcache_cooked_write (regcache, regnum, valbuf + (bitpos / 8));
>  	}
>        else
> @@ -712,6 +739,8 @@ static void
>  sparc64_extract_floating_fields (struct regcache *regcache, struct type *type,
>  				 gdb_byte *valbuf, int bitpos)
>  {
> +  struct gdbarch *gdbarch = get_regcache_arch (regcache);
> +
>    if (sparc64_floating_p (type))
>      {
>        int len = TYPE_LENGTH (type);
> @@ -721,14 +750,15 @@ sparc64_extract_floating_fields (struct
>  	{
>  	  gdb_assert (bitpos == 0 || bitpos == 128);
>  
> -	  regnum = SPARC64_Q0_REGNUM + bitpos / 128;
> +	  regnum = gdbarch_num_regs (gdbarch) + SPARC64_Q0_REGNUM
> +                   + bitpos / 128;
>  	  regcache_cooked_read (regcache, regnum, valbuf + (bitpos / 8));
>  	}
>        else if (len == 8)
>  	{
>  	  gdb_assert (bitpos % 64 == 0 && bitpos >= 0 && bitpos < 256);
>  
> -	  regnum = SPARC64_D0_REGNUM + bitpos / 64;
> +	  regnum = gdbarch_num_regs (gdbarch) + SPARC64_D0_REGNUM + bitpos / 64;
>  	  regcache_cooked_read (regcache, regnum, valbuf + (bitpos / 8));
>  	}
>        else
> @@ -911,13 +941,13 @@ sparc64_store_arguments (struct regcache
>  	  /* Float Complex or double Complex arguments.  */
>  	  if (element < 16)
>  	    {
> -	      regnum = SPARC64_D0_REGNUM + element;
> +	      regnum = gdbarch_num_regs (gdbarch) + SPARC64_D0_REGNUM + element;
>  
>  	      if (len == 16)
>  		{
> -		  if (regnum < SPARC64_D30_REGNUM)
> +		  if (regnum < gdbarch_num_regs (gdbarch) + SPARC64_D30_REGNUM)
>  		    regcache_cooked_write (regcache, regnum + 1, valbuf + 8);
> -		  if (regnum < SPARC64_D10_REGNUM)
> +		  if (regnum < gdbarch_num_regs (gdbarch) + SPARC64_D10_REGNUM)
>  		    regcache_cooked_write (regcache,
>  					   SPARC_O0_REGNUM + element + 1,
>  					   valbuf + 8);
> @@ -932,12 +962,14 @@ sparc64_store_arguments (struct regcache
>  	      if (element % 2)
>  		element++;
>  	      if (element < 16)
> -		regnum = SPARC64_Q0_REGNUM + element / 2;
> +		regnum = gdbarch_num_regs (gdbarch) + SPARC64_Q0_REGNUM
> +                         + element / 2;
>  	    }
>  	  else if (len == 8)
>  	    {
>  	      if (element < 16)
> -		regnum = SPARC64_D0_REGNUM + element;
> +		regnum = gdbarch_num_regs (gdbarch) + SPARC64_D0_REGNUM
> +                         + element;
>  	    }
>  	  else if (len == 4)
>  	    {
> @@ -952,7 +984,8 @@ sparc64_store_arguments (struct regcache
>  	      valbuf = buf;
>  	      len = 8;
>  	      if (element < 16)
> -		regnum = SPARC64_D0_REGNUM + element;
> +		regnum = gdbarch_num_regs (gdbarch) + SPARC64_D0_REGNUM
> +                         + element;
>  	    }
>  	}
>        else
> @@ -969,19 +1002,24 @@ sparc64_store_arguments (struct regcache
>  
>  	  /* If we're storing the value in a floating-point register,
>               also store it in the corresponding %0 register(s).  */
> -	  if (regnum >= SPARC64_D0_REGNUM && regnum <= SPARC64_D10_REGNUM)
> -	    {
> -	      gdb_assert (element < 6);
> -	      regnum = SPARC_O0_REGNUM + element;
> -	      regcache_cooked_write (regcache, regnum, valbuf);
> -	    }
> -	  else if (regnum >= SPARC64_Q0_REGNUM && regnum <= SPARC64_Q8_REGNUM)
> -	    {
> -	      gdb_assert (element < 5);
> -	      regnum = SPARC_O0_REGNUM + element;
> -	      regcache_cooked_write (regcache, regnum, valbuf);
> -	      regcache_cooked_write (regcache, regnum + 1, valbuf + 8);
> -	    }
> +	  if (regnum >= gdbarch_num_regs (gdbarch))
> +            {

The indentation looks odd to me.

> +              regnum -= gdbarch_num_regs (gdbarch);
> +
> +              if (regnum >= SPARC64_D0_REGNUM && regnum <= SPARC64_D10_REGNUM)
> +	        {
> +	          gdb_assert (element < 6);
> +	          regnum = SPARC_O0_REGNUM + element;
> +	          regcache_cooked_write (regcache, regnum, valbuf);
> +                }
> +              else if (regnum >= SPARC64_Q0_REGNUM && regnum <= SPARC64_Q8_REGNUM)
> +                {
> +                  gdb_assert (element < 5);
> +                  regnum = SPARC_O0_REGNUM + element;
> +                  regcache_cooked_write (regcache, regnum, valbuf);
> +                  regcache_cooked_write (regcache, regnum + 1, valbuf + 8);
> +	        }
> +            }
>  	}

Looks all these changes can be moved to patch #1.

>  
>        /* Always store the argument in memory.  */
> diff -Npur a/gdb/testsuite/gdb.xml/tdesc-regs.exp b/gdb/testsuite/gdb.xml/tdesc-regs.exp
> --- a/gdb/testsuite/gdb.xml/tdesc-regs.exp	2016-02-09 19:19:39.000000000 +0000
> +++ b/gdb/testsuite/gdb.xml/tdesc-regs.exp	2016-12-06 15:53:35.418621207 +0000
> @@ -49,6 +49,12 @@ switch -glob -- [istarget] {
>      "s390*-*-*" {
>  	set core-regs {s390-core32.xml s390-acr.xml s390-fpr.xml}
>      }
> +    "sparc-*-*" {
> +        set core-regs {sparc32-cpu.xml sparc32-fpu.xml sparc32-cp0.xml}
> +    }
> +    "sparc64-*-*" {
> +        set core-regs {sparc64-cpu.xml sparc64-fpu.xml sparc64-cp0.xml}
> +    }

You need 'set regdir "sparc/"'

>      "spu*-*-*" {
>  	# This may be either the spu-linux-nat target, or the Cell/B.E.
>  	# multi-architecture debugger in SPU standalone executable mode.

> ChangeLog entry:
> 2016-12-07  Ivo Raisr  <ivo.raisr@oracle.com>
> 
> 	PR tdep/20936
> 	Provide and use sparc32 and sparc64 target description XML files.
> 	* sparc32-cp0.xml, sparc32-cpu.xml, sparc32-fpu.xml: New files for
> 	sparc 32-bit.
> 	* sparc64-cp0.xml, sparc64-cpu.xml, sparc64-fpu.xml: New files
> 	for sparc 64-bit.
> 	* sparc32-solaris.xml, sparc64-solaris.xml: New files for sparc32
> 	and sparc64 on Solaris.
> 	* sparc-solaris.c, sparc64-solaris.c: Generated.

These file names should be prefixed with "feature/sparc/"

> 	* sparc-tdep.h: Deal with sparc32 and sparc64 differences
> 	in target descriptions. Separate real and pseudo registers.
> 	* sparc-tdep.c: Validate and use registers of the target description.
> 	Pseudo registers are numbered after all real registers from the target
> 	description; deal with it.
> 	* sparc64-tdep.h: Separate real and pseudo registers.
> 	* sparc64-tdep.c: Pseudo registers are numbered after all real
> 	registers from the target description; deal with it.

Please describe the change on the function level, take a look at
https://sourceware.org/gdb/wiki/ContributionChecklist there are two
sections about ChangeLog.

> 	* gdb.texinfo: New node "Sparc Features".

This should go to a separated entry,

gdb/doc:

2016-12-07  Ivo Raisr  <ivo.raisr@oracle.com>

	* gdb.texinfo (Standard Target Features): Document SPARC features.
	(Sparc Features): New node.

> 	* tdesc-regs.exp: Provide sparc core registers for the tests.

Likewise,

gdb/testsuite:

2016-12-07  Ivo Raisr  <ivo.raisr@oracle.com>

	* gdb.xml/tdesc-regs.exp: Provide sparc core registers for
	the tests.

-- 
Yao (齐尧)



More information about the Gdb-patches mailing list