From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 89144 invoked by alias); 12 Dec 2016 12:54:05 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 89064 invoked by uid 89); 12 Dec 2016 12:54:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: =?ISO-8859-1?Q?No, score=1.0 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS,UNWANTED_LANGUAGE_BODY autolearn=no version=3.3.2 spammy=be, =e5=b0=a7, UD:xml, SPARC?= X-HELO: mail-wm0-f66.google.com Received: from mail-wm0-f66.google.com (HELO mail-wm0-f66.google.com) (74.125.82.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 12 Dec 2016 12:53:54 +0000 Received: by mail-wm0-f66.google.com with SMTP id u144so11088974wmu.0 for ; Mon, 12 Dec 2016 04:53:53 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=LK/p5/mDgXihNQCEu7NZESZjVV/hZEEPGucsiP6LnlM=; b=bqnqkZ4JVgzVqzAaxLbDPLcimuPKKCw/6f3DWLL8ZJDt7bKbYBOghheofmBUMDsznl kqtgr0TaQ3B5JcV89C1cttu/yi02Kv8JHBHNASZUKZ0Kc1wtye+qu6MOq78N+NYT2+H0 Lsnv2MFO6fQ9rHYcdC2pIBOerj3C6afXP4VAE8Pp0gvCQVOjfAjI/PKU3DaESD3XuMQu YUVA6JdqV8+RkGZ0d2Epn8XbDIPiQMeBjeUvCb3+F1yEm+uWWDLjIIsY1pocVcUZHK/1 1QheGrayCQDFRGWvmA9Ev3+s/+OWshpx1fhUCZJT7/++PJ6zK3pMPE4/jUBSLp6uArj5 7ubQ== X-Gm-Message-State: AKaTC009IOCYVTCdpONSX025olkSGBMOngevg3UhUCWwWgBnsr4t6J2PnuyLzcBe0pqFEg== X-Received: by 10.28.236.205 with SMTP id h74mr8810635wmi.104.1481547231616; Mon, 12 Dec 2016 04:53:51 -0800 (PST) Received: from E107787-LIN (gcc1-power7.osuosl.org. [140.211.15.137]) by smtp.gmail.com with ESMTPSA id c187sm35283898wmd.13.2016.12.12.04.53.47 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Mon, 12 Dec 2016 04:53:50 -0800 (PST) Date: Mon, 12 Dec 2016 12:54:00 -0000 From: Yao Qi To: Ivo Raisr Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] Bug 20936 - provide sparc and sparcv9 target description XML files Message-ID: <20161212125331.GB25542@E107787-LIN> References: <46200a1e-29f7-8e20-c0b5-3f6f25c82d45@oracle.com> <20161206152616.GC28789@E107787-LIN> <83d4c58d-0834-4fc2-6194-72408510aa8a@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <83d4c58d-0834-4fc2-6194-72408510aa8a@oracle.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes X-SW-Source: 2016-12/txt/msg00267.txt.bz2 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 > > 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 * 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 * gdb.xml/tdesc-regs.exp: Provide sparc core registers for the tests. -- Yao (齐尧)