This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Bug 20936 - provide sparc and sparcv9 target description XML files
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 (齐尧)