This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v3] Convert the RX target to make use of target descriptions.
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 <ysato@users.sourceforge.jp>
> >
> > * 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 <ysato@users.sourceforge.jp>
> >
> > * 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