[PATCH v3] Convert the RX target to make use of target descriptions.

Yoshinori Sato ysato@users.sourceforge.jp
Sat Aug 24 13:41:00 GMT 2019


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



More information about the Gdb-patches mailing list