From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 120785 invoked by alias); 24 Aug 2019 13:41:42 -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 120777 invoked by uid 89); 24 Aug 2019 13:41:42 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-20.5 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_LOW,SPF_SOFTFAIL autolearn=ham version=3.3.1 spammy=H*UA:6.0, H*u:6.0, Convert X-HELO: mail01.asahi-net.or.jp Received: from mail01.asahi-net.or.jp (HELO mail01.asahi-net.or.jp) (202.224.55.13) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 24 Aug 2019 13:41:40 +0000 Received: from h61-195-96-97.vps.ablenet.jp (h61-195-96-97.ablenetvps.ne.jp [61.195.96.97]) (Authenticated sender: PQ4Y-STU) by mail01.asahi-net.or.jp (Postfix) with ESMTPA id AE9E5136C4C; Sat, 24 Aug 2019 22:41:34 +0900 (JST) Received: from yo-satoh-debian.ysato.ml (ZM005235.ppp.dion.ne.jp [222.8.5.235]) by h61-195-96-97.vps.ablenet.jp (Postfix) with ESMTPSA id 82544240085; Sat, 24 Aug 2019 22:41:33 +0900 (JST) Date: Sat, 24 Aug 2019 13:41:00 -0000 Message-ID: <87tva6k7eb.wl-ysato@users.sourceforge.jp> From: Yoshinori Sato To: Simon Marchi Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v3] Convert the RX target to make use of target descriptions. In-Reply-To: <820b918e-ec65-68de-6ac6-8ea89243b3d1@simark.ca> References: <20190822143026.82014-1-ysato@users.sourceforge.jp> <820b918e-ec65-68de-6ac6-8ea89243b3d1@simark.ca> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM/1.14.9 (=?ISO-8859-4?Q?Goj=F2?=) APEL/10.8 EasyPG/1.0.0 Emacs/25.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-IsSubscribed: yes X-SW-Source: 2019-08/txt/msg00563.txt.bz2 On Sat, 24 Aug 2019 00:01:24 +0900, Simon Marchi wrote: > > Hi Yoshinori, > > I looked at the patch because I was curious about RX. I found a few formatting nits that you might want to fix before pushing. > > On 2019-08-22 10:30 a.m., Yoshinori Sato wrote: > > gdb/ChangeLog > > > > 2019-08-22 Yoshinori Sato > > > > * gdb/rx-tdep.c (rx_register_names): New. > > (rx_register_name): delete. > > (rx_psw_type): delete. > > (rx_fpsw_type): delete. > > (rx_register_type): delete. > > Use capital letters, "Delete.". OK. > > (rx_gdbarch_init): Convert target-descriptions. > > (_initialize_rx_tdep): Add initialize_tdesc_rx. > > * gdb/features/Makefile: Add rx.xml. > > * gdb/features/rx.xml: New. > > * gdb/features/rx.c: Likewise. > > * gdb/NEWS: Mention target description support. > > > > gdb/doc/ChangeLog: > > > > 2019-08-22 Yoshinori Sato > > > > * gdb.texinfo (Standard Target Features): Add RX Features sub-section. > > --- > > gdb/NEWS | 2 + > > gdb/doc/gdb.texinfo | 10 ++++ > > gdb/features/Makefile | 2 + > > gdb/features/rx.c | 76 +++++++++++++++++++++++ > > gdb/features/rx.xml | 70 ++++++++++++++++++++++ > > gdb/rx-tdep.c | 162 +++++++++++++++----------------------------------- > > 6 files changed, 207 insertions(+), 115 deletions(-) > > create mode 100644 gdb/features/rx.c > > create mode 100644 gdb/features/rx.xml > > > > diff --git a/gdb/NEWS b/gdb/NEWS > > index 462247f486..dc32e10a2c 100644 > > --- a/gdb/NEWS > > +++ b/gdb/NEWS > > @@ -27,6 +27,8 @@ > > provide the exitcode or exit status of the shell commands launched by > > GDB commands such as "shell", "pipe" and "make". > > > > +* The RX port now supports XML target descriptions. > > + > > * Python API > > > > ** The gdb.Value type has a new method 'format_string' which returns a > > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > > index 523e3d0bfe..ad70807953 100644 > > --- a/gdb/doc/gdb.texinfo > > +++ b/gdb/doc/gdb.texinfo > > @@ -44068,6 +44068,7 @@ registers using the capitalization used in the description. > > * OpenRISC 1000 Features:: > > * PowerPC Features:: > > * RISC-V Features:: > > +* RX Features:: > > * S/390 and System z Features:: > > * Sparc Features:: > > * TIC6x Features:: > > @@ -44498,6 +44499,15 @@ target has floating point hardware, but can be moved into the csr > > feature if the target has the floating point control registers, but no > > other floating point hardware. > > > > +@node RX Features > > +@subsection RX Features > > +@cindex target descriptions, RX Features > > + > > +The @samp{org.gnu.gdb.rx.core} feature is required for RX > > +targets. It should contain the registers @samp{r0} through > > +@samp{r15}, @samp{usp}, @samp{isp}, @samp{psw}, @samp{pc}, @samp{intb}, > > +@samp{bpsw}, @samp{bpc}, @samp{fintv}, @samp{fpsw}, and @samp{acc}. > > + > > @node S/390 and System z Features > > @subsection S/390 and System z Features > > @cindex target descriptions, S/390 features > > diff --git a/gdb/features/Makefile b/gdb/features/Makefile > > index 0c84faf405..2b65d46df0 100644 > > --- a/gdb/features/Makefile > > +++ b/gdb/features/Makefile > > @@ -161,6 +161,7 @@ XMLTOC = \ > > rs6000/powerpc-vsx64.xml \ > > rs6000/powerpc-vsx64l.xml \ > > rs6000/rs6000.xml \ > > + rx.xml \ > > s390-linux32.xml \ > > s390-linux32v1.xml \ > > s390-linux32v2.xml \ > > @@ -238,6 +239,7 @@ FEATURE_XMLFILES = aarch64-core.xml \ > > riscv/64bit-cpu.xml \ > > riscv/64bit-csr.xml \ > > riscv/64bit-fpu.xml \ > > + rx.xml \ > > tic6x-c6xp.xml \ > > tic6x-core.xml \ > > tic6x-gp.xml > > diff --git a/gdb/features/rx.c b/gdb/features/rx.c > > new file mode 100644 > > index 0000000000..8144103e48 > > --- /dev/null > > +++ b/gdb/features/rx.c > > @@ -0,0 +1,76 @@ > > +/* THIS FILE IS GENERATED. -*- buffer-read-only: t -*- vi:set ro: > > + Original: rx.xml.tmp */ > > + > > +#include "defs.h" > > +#include "osabi.h" > > +#include "target-descriptions.h" > > + > > +struct target_desc *tdesc_rx; > > +static void > > +initialize_tdesc_rx (void) > > +{ > > + struct target_desc *result = allocate_target_description (); > > + struct tdesc_feature *feature; > > + > > + feature = tdesc_create_feature (result, "org.gnu.gdb.rx.core"); > > + tdesc_type_with_fields *type_with_fields; > > + type_with_fields = tdesc_create_flags (feature, "psw_flags", 4); > > + tdesc_add_flag (type_with_fields, 0, "C"); > > + tdesc_add_flag (type_with_fields, 1, "Z"); > > + tdesc_add_flag (type_with_fields, 2, "S"); > > + tdesc_add_flag (type_with_fields, 3, "O"); > > + tdesc_add_flag (type_with_fields, 16, "I"); > > + tdesc_add_flag (type_with_fields, 17, "U"); > > + tdesc_add_flag (type_with_fields, 20, "PM"); > > + tdesc_add_bitfield (type_with_fields, "IPL", 24, 27); > > The old code (done by hand) did: > > append_flags_type_flag (tdep->rx_psw_type, 24, "IPL0"); > append_flags_type_flag (tdep->rx_psw_type, 25, "IPL1"); > append_flags_type_flag (tdep->rx_psw_type, 26, "IPL2"); > append_flags_type_flag (tdep->rx_psw_type, 27, "IPL3"); > > Is the result identical for the end user? No. Since this is a 4-bit value, the definition of target-description is correct, but it is not good that it is not compatible. Returns the same result as the previous definition. > > @@ -1065,16 +967,44 @@ rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) > > return arches->gdbarch; > > } > > > > - /* None found, create a new architecture from the information > > - provided. */ > > + if (tdesc == NULL) > > + tdesc = tdesc_rx; > > + > > + /* Check any target description for validity. */ > > + if (tdesc_has_registers (tdesc)) > > + { > > + const struct tdesc_feature *feature; > > + bool valid_p = true; > > + > > + feature = tdesc_find_feature (tdesc, "org.gnu.gdb.rx.core"); > > + > > + if (feature != NULL) > > + { > > + int i; > > Declare `i` in the for loop. OK. > > + > > + tdesc_data = tdesc_data_alloc (); > > + for (i = 0; i < RX_NUM_REGS; i++) > > + valid_p &= tdesc_numbered_register (feature, tdesc_data, i, > > + rx_register_names[i]); > > + } > > + > > + if (valid_p == false) > > Use !valid_p. OK. > > + { > > + tdesc_data_cleanup (tdesc_data); > > + return NULL; > > + } > > + } > > + > > + gdb_assert(tdesc_data != NULL); > > Missing space before parenthesis. OK. > Thanks, > > Simon I'll fix and post v4. Thanks. -- Yosinori Sato