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

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Yoshinori Sato
gdb/ChangeLog

2019-08-22  Yoshinori Sato <[hidden email]>

        * gdb/rx-tdep.c (rx_register_names): New.
        (rx_register_name): delete.
        (rx_psw_type): delete.
        (rx_fpsw_type): delete.
        (rx_register_type): delete.
        (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 <[hidden email]>

        * 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);
+
+  type_with_fields = tdesc_create_flags (feature, "fpsw_flags", 4);
+  tdesc_add_bitfield (type_with_fields, "RM", 0, 1);
+  tdesc_add_flag (type_with_fields, 2, "CV");
+  tdesc_add_flag (type_with_fields, 3, "CO");
+  tdesc_add_flag (type_with_fields, 4, "CZ");
+  tdesc_add_flag (type_with_fields, 5, "CU");
+  tdesc_add_flag (type_with_fields, 6, "CX");
+  tdesc_add_flag (type_with_fields, 7, "CE");
+  tdesc_add_flag (type_with_fields, 8, "DN");
+  tdesc_add_flag (type_with_fields, 10, "EV");
+  tdesc_add_flag (type_with_fields, 11, "EO");
+  tdesc_add_flag (type_with_fields, 12, "EZ");
+  tdesc_add_flag (type_with_fields, 13, "EU");
+  tdesc_add_flag (type_with_fields, 14, "EX");
+  tdesc_add_flag (type_with_fields, 26, "FV");
+  tdesc_add_flag (type_with_fields, 27, "FO");
+  tdesc_add_flag (type_with_fields, 28, "FZ");
+  tdesc_add_flag (type_with_fields, 29, "FU");
+  tdesc_add_flag (type_with_fields, 30, "FX");
+  tdesc_add_flag (type_with_fields, 31, "FS");
+
+  tdesc_create_reg (feature, "r0", 0, 1, NULL, 32, "data_ptr");
+  tdesc_create_reg (feature, "r1", 1, 1, NULL, 32, "uint32");
+  tdesc_create_reg (feature, "r2", 2, 1, NULL, 32, "uint32");
+  tdesc_create_reg (feature, "r3", 3, 1, NULL, 32, "uint32");
+  tdesc_create_reg (feature, "r4", 4, 1, NULL, 32, "uint32");
+  tdesc_create_reg (feature, "r5", 5, 1, NULL, 32, "uint32");
+  tdesc_create_reg (feature, "r6", 6, 1, NULL, 32, "uint32");
+  tdesc_create_reg (feature, "r7", 7, 1, NULL, 32, "uint32");
+  tdesc_create_reg (feature, "r8", 8, 1, NULL, 32, "uint32");
+  tdesc_create_reg (feature, "r9", 9, 1, NULL, 32, "uint32");
+  tdesc_create_reg (feature, "r10", 10, 1, NULL, 32, "uint32");
+  tdesc_create_reg (feature, "r11", 11, 1, NULL, 32, "uint32");
+  tdesc_create_reg (feature, "r12", 12, 1, NULL, 32, "uint32");
+  tdesc_create_reg (feature, "r13", 13, 1, NULL, 32, "uint32");
+  tdesc_create_reg (feature, "r14", 14, 1, NULL, 32, "uint32");
+  tdesc_create_reg (feature, "r15", 15, 1, NULL, 32, "uint32");
+  tdesc_create_reg (feature, "usp", 16, 1, NULL, 32, "data_ptr");
+  tdesc_create_reg (feature, "isp", 17, 1, NULL, 32, "data_ptr");
+  tdesc_create_reg (feature, "psw", 18, 1, NULL, 32, "psw_flags");
+  tdesc_create_reg (feature, "pc", 19, 1, NULL, 32, "code_ptr");
+  tdesc_create_reg (feature, "intb", 20, 1, NULL, 32, "data_ptr");
+  tdesc_create_reg (feature, "bpsw", 21, 1, NULL, 32, "psw_flags");
+  tdesc_create_reg (feature, "bpc", 22, 1, NULL, 32, "code_ptr");
+  tdesc_create_reg (feature, "fintv", 23, 1, NULL, 32, "code_ptr");
+  tdesc_create_reg (feature, "fpsw", 24, 1, NULL, 32, "fpsw_flags");
+  tdesc_create_reg (feature, "acc", 25, 1, NULL, 64, "uint64");
+
+  tdesc_rx = result;
+}
diff --git a/gdb/features/rx.xml b/gdb/features/rx.xml
new file mode 100644
index 0000000000..b5aa9ac4a8
--- /dev/null
+++ b/gdb/features/rx.xml
@@ -0,0 +1,70 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2019 Free Software Foundation, Inc.
+
+     Copying and distribution of this file, with or without modification,
+     are permitted in any medium without royalty provided the copyright
+     notice and this notice are preserved.  -->
+
+<!DOCTYPE feature SYSTEM "gdb-target.dtd">
+<feature name="org.gnu.gdb.rx.core">
+  <reg name="r0" bitsize="32" type="data_ptr"/>
+  <reg name="r1" bitsize="32" type="uint32"/>
+  <reg name="r2" bitsize="32" type="uint32"/>
+  <reg name="r3" bitsize="32" type="uint32"/>
+  <reg name="r4" bitsize="32" type="uint32"/>
+  <reg name="r5" bitsize="32" type="uint32"/>
+  <reg name="r6" bitsize="32" type="uint32"/>
+  <reg name="r7" bitsize="32" type="uint32"/>
+  <reg name="r8" bitsize="32" type="uint32"/>
+  <reg name="r9" bitsize="32" type="uint32"/>
+  <reg name="r10" bitsize="32" type="uint32"/>
+  <reg name="r11" bitsize="32" type="uint32"/>
+  <reg name="r12" bitsize="32" type="uint32"/>
+  <reg name="r13" bitsize="32" type="uint32"/>
+  <reg name="r14" bitsize="32" type="uint32"/>
+  <reg name="r15" bitsize="32" type="uint32"/>
+
+  <flags id="psw_flags" size="4">
+    <field name="C" start="0" end="0"/>
+    <field name="Z" start="1" end="1"/>
+    <field name="S" start="2" end="2"/>
+    <field name="O" start="3" end="3"/>
+    <field name="I" start="16" end="16"/>
+    <field name="U" start="17" end="17"/>
+    <field name="PM" start="20" end="20"/>
+    <field name="IPL" start="24" end="27"/>
+  </flags>
+
+  <flags id="fpsw_flags" size="4">
+    <field name="RM" start="0" end="1"/>
+    <field name="CV" start="2" end="2"/>
+    <field name="CO" start="3" end="3"/>
+    <field name="CZ" start="4" end="4"/>
+    <field name="CU" start="5" end="5"/>
+    <field name="CX" start="6" end="6"/>
+    <field name="CE" start="7" end="7"/>
+    <field name="DN" start="8" end="8"/>
+    <field name="EV" start="10" end="10"/>
+    <field name="EO" start="11" end="11"/>
+    <field name="EZ" start="12" end="12"/>
+    <field name="EU" start="13" end="13"/>
+    <field name="EX" start="14" end="14"/>
+    <field name="FV" start="26" end="26"/>
+    <field name="FO" start="27" end="27"/>
+    <field name="FZ" start="28" end="28"/>
+    <field name="FU" start="29" end="29"/>
+    <field name="FX" start="30" end="30"/>
+    <field name="FS" start="31" end="31"/>
+  </flags>
+
+  <reg name="usp" bitsize="32" type="data_ptr"/>
+  <reg name="isp" bitsize="32" type="data_ptr"/>
+  <reg name="psw" bitsize="32" type="psw_flags"/>
+  <reg name="pc" bitsize="32" type="code_ptr"/>
+  <reg name="intb" bitsize="32" type="data_ptr"/>
+  <reg name="bpsw" bitsize="32" type="psw_flags"/>
+  <reg name="bpc" bitsize="32" type="code_ptr"/>
+  <reg name="fintv" bitsize="32" type="code_ptr"/>
+  <reg name="fpsw" bitsize="32" type="fpsw_flags"/>
+  <reg name="acc" bitsize="64" type="uint64"/>
+</feature>
diff --git a/gdb/rx-tdep.c b/gdb/rx-tdep.c
index 4cbf919db9..1e6bebba76 100644
--- a/gdb/rx-tdep.c
+++ b/gdb/rx-tdep.c
@@ -33,11 +33,15 @@
 #include "value.h"
 #include "gdbcore.h"
 #include "dwarf2-frame.h"
+#include "remote.h"
+#include "target-descriptions.h"
 
 #include "elf/rx.h"
 #include "elf-bfd.h"
 #include <algorithm>
 
+#include "features/rx.c"
+
 /* Certain important register numbers.  */
 enum
 {
@@ -114,117 +118,13 @@ struct rx_prologue
   int reg_offset[RX_NUM_REGS];
 };
 
-/* Implement the "register_name" gdbarch method.  */
-static const char *
-rx_register_name (struct gdbarch *gdbarch, int regnr)
-{
-  static const char *const reg_names[] = {
-    "r0",
-    "r1",
-    "r2",
-    "r3",
-    "r4",
-    "r5",
-    "r6",
-    "r7",
-    "r8",
-    "r9",
-    "r10",
-    "r11",
-    "r12",
-    "r13",
-    "r14",
-    "r15",
-    "usp",
-    "isp",
-    "psw",
-    "pc",
-    "intb",
-    "bpsw",
-    "bpc",
-    "fintv",
-    "fpsw",
-    "acc"
-  };
-
-  return reg_names[regnr];
-}
-
-/* Construct the flags type for PSW and BPSW.  */
-
-static struct type *
-rx_psw_type (struct gdbarch *gdbarch)
-{
-  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
-
-  if (tdep->rx_psw_type == NULL)
-    {
-      tdep->rx_psw_type = arch_flags_type (gdbarch, "rx_psw_type", 32);
-      append_flags_type_flag (tdep->rx_psw_type, 0, "C");
-      append_flags_type_flag (tdep->rx_psw_type, 1, "Z");
-      append_flags_type_flag (tdep->rx_psw_type, 2, "S");
-      append_flags_type_flag (tdep->rx_psw_type, 3, "O");
-      append_flags_type_flag (tdep->rx_psw_type, 16, "I");
-      append_flags_type_flag (tdep->rx_psw_type, 17, "U");
-      append_flags_type_flag (tdep->rx_psw_type, 20, "PM");
-      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");
-    }
-  return tdep->rx_psw_type;
-}
-
-/* Construct flags type for FPSW.  */
-
-static struct type *
-rx_fpsw_type (struct gdbarch *gdbarch)
-{
-  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
-
-  if (tdep->rx_fpsw_type == NULL)
-    {
-      tdep->rx_fpsw_type = arch_flags_type (gdbarch, "rx_fpsw_type", 32);
-      append_flags_type_flag (tdep->rx_fpsw_type, 0, "RM0");
-      append_flags_type_flag (tdep->rx_fpsw_type, 1, "RM1");
-      append_flags_type_flag (tdep->rx_fpsw_type, 2, "CV");
-      append_flags_type_flag (tdep->rx_fpsw_type, 3, "CO");
-      append_flags_type_flag (tdep->rx_fpsw_type, 4, "CZ");
-      append_flags_type_flag (tdep->rx_fpsw_type, 5, "CU");
-      append_flags_type_flag (tdep->rx_fpsw_type, 6, "CX");
-      append_flags_type_flag (tdep->rx_fpsw_type, 7, "CE");
-      append_flags_type_flag (tdep->rx_fpsw_type, 8, "DN");
-      append_flags_type_flag (tdep->rx_fpsw_type, 10, "EV");
-      append_flags_type_flag (tdep->rx_fpsw_type, 11, "EO");
-      append_flags_type_flag (tdep->rx_fpsw_type, 12, "EZ");
-      append_flags_type_flag (tdep->rx_fpsw_type, 13, "EU");
-      append_flags_type_flag (tdep->rx_fpsw_type, 14, "EX");
-      append_flags_type_flag (tdep->rx_fpsw_type, 26, "FV");
-      append_flags_type_flag (tdep->rx_fpsw_type, 27, "FO");
-      append_flags_type_flag (tdep->rx_fpsw_type, 28, "FZ");
-      append_flags_type_flag (tdep->rx_fpsw_type, 29, "FU");
-      append_flags_type_flag (tdep->rx_fpsw_type, 30, "FX");
-      append_flags_type_flag (tdep->rx_fpsw_type, 31, "FS");
-    }
-
-  return tdep->rx_fpsw_type;
-}
-
-/* Implement the "register_type" gdbarch method.  */
-static struct type *
-rx_register_type (struct gdbarch *gdbarch, int reg_nr)
-{
-  if (reg_nr == RX_PC_REGNUM)
-    return builtin_type (gdbarch)->builtin_func_ptr;
-  else if (reg_nr == RX_PSW_REGNUM || reg_nr == RX_BPSW_REGNUM)
-    return rx_psw_type (gdbarch);
-  else if (reg_nr == RX_FPSW_REGNUM)
-    return rx_fpsw_type (gdbarch);
-  else if (reg_nr == RX_ACC_REGNUM)
-    return builtin_type (gdbarch)->builtin_unsigned_long_long;
-  else
-    return builtin_type (gdbarch)->builtin_unsigned_long;
-}
+/* RX register names */
+static const char *const rx_register_names[] = {
+  "r0",  "r1",  "r2",  "r3",  "r4",  "r5",  "r6",  "r7",
+  "r8",  "r9",  "r10", "r11", "r12", "r13", "r14", "r15",
+  "usp", "isp", "psw", "pc",  "intb", "bpsw","bpc","fintv",
+  "fpsw", "acc",
+};
 
 
 /* Function for finding saved registers in a 'struct pv_area'; this
@@ -1044,6 +944,8 @@ rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   struct gdbarch *gdbarch;
   struct gdbarch_tdep *tdep;
   int elf_flags;
+  struct tdesc_arch_data *tdesc_data = NULL;
+  const struct target_desc *tdesc = info.target_desc;
 
   /* Extract the elf_flags if available.  */
   if (info.abfd != NULL
@@ -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;
+
+  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)
+ {
+  tdesc_data_cleanup (tdesc_data);
+  return NULL;
+ }
+    }
+
+  gdb_assert(tdesc_data != NULL);
+
   tdep = XCNEW (struct gdbarch_tdep);
   gdbarch = gdbarch_alloc (&info, tdep);
   tdep->elf_flags = elf_flags;
 
   set_gdbarch_num_regs (gdbarch, RX_NUM_REGS);
+  tdesc_use_registers (gdbarch, tdesc, tdesc_data);
+
   set_gdbarch_num_pseudo_regs (gdbarch, 0);
-  set_gdbarch_register_name (gdbarch, rx_register_name);
-  set_gdbarch_register_type (gdbarch, rx_register_type);
   set_gdbarch_pc_regnum (gdbarch, RX_PC_REGNUM);
   set_gdbarch_sp_regnum (gdbarch, RX_SP_REGNUM);
   set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
@@ -1092,6 +1022,7 @@ rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_ptr_bit (gdbarch, 32);
   set_gdbarch_float_bit (gdbarch, 32);
   set_gdbarch_float_format (gdbarch, floatformats_ieee_single);
+
   if (elf_flags & E_FLAG_RX_64BIT_DOUBLES)
     {
       set_gdbarch_double_bit (gdbarch, 64);
@@ -1132,4 +1063,5 @@ void
 _initialize_rx_tdep (void)
 {
   register_gdbarch_init (bfd_arch_rx, rx_gdbarch_init);
+  initialize_tdesc_rx ();
 }
--
2.11.0

Reply | Threaded
Open this post in threaded view
|

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

Eli Zaretskii
> From: Yoshinori Sato <[hidden email]>
> Cc: Yoshinori Sato <[hidden email]>
> Date: Thu, 22 Aug 2019 23:30:26 +0900
>
> gdb/ChangeLog
>
> 2019-08-22  Yoshinori Sato <[hidden email]>
>
> * gdb/rx-tdep.c (rx_register_names): New.
> (rx_register_name): delete.
> (rx_psw_type): delete.
> (rx_fpsw_type): delete.
> (rx_register_type): delete.
> (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 <[hidden email]>
>
> * gdb.texinfo (Standard Target Features): Add RX Features sub-section.

This is OK for the documentation part.

Thanks.
Reply | Threaded
Open this post in threaded view
|

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

Kevin Buettner
In reply to this post by Yoshinori Sato
On Thu, 22 Aug 2019 23:30:26 +0900
Yoshinori Sato <[hidden email]> wrote:

> gdb/ChangeLog
>
> 2019-08-22  Yoshinori Sato <[hidden email]>
>
> * gdb/rx-tdep.c (rx_register_names): New.
> (rx_register_name): delete.
> (rx_psw_type): delete.
> (rx_fpsw_type): delete.
> (rx_register_type): delete.
> (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.

One small nit - for gdb/features/rx.c, I'd prefer to see a ChangeLog
comment saying that it's been generated.  I.e.

  * gdb/features/rx.c: Generate.

The rest looks good to me.  If there are no other suggestions from
someone else, go ahead and commit it with that small tweak.

Kevin
Reply | Threaded
Open this post in threaded view
|

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

Yoshinori Sato
On Fri, 23 Aug 2019 01:56:52 +0900,
Kevin Buettner wrote:

>
> On Thu, 22 Aug 2019 23:30:26 +0900
> Yoshinori Sato <[hidden email]> wrote:
>
> > gdb/ChangeLog
> >
> > 2019-08-22  Yoshinori Sato <[hidden email]>
> >
> > * gdb/rx-tdep.c (rx_register_names): New.
> > (rx_register_name): delete.
> > (rx_psw_type): delete.
> > (rx_fpsw_type): delete.
> > (rx_register_type): delete.
> > (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.
>
> One small nit - for gdb/features/rx.c, I'd prefer to see a ChangeLog
> comment saying that it's been generated.  I.e.
>
>   * gdb/features/rx.c: Generate.
>
> The rest looks good to me.  If there are no other suggestions from
> someone else, go ahead and commit it with that small tweak.
>
> Kevin

OK.
This log will be fixed when I commit.

--
Yosinori Sato
Reply | Threaded
Open this post in threaded view
|

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

Simon Marchi-4
In reply to this post by Yoshinori Sato
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 <[hidden email]>
>
> * 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.".

> (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 <[hidden email]>
>
> * 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?

> @@ -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.

> +
> +  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.

> + {
> +  tdesc_data_cleanup (tdesc_data);
> +  return NULL;
> + }
> +    }
> +
> +  gdb_assert(tdesc_data != NULL);

Missing space before parenthesis.

Thanks,

Simon
Reply | Threaded
Open this post in threaded view
|

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

Yoshinori Sato
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 <[hidden email]>
> >
> > * 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 <[hidden email]>
> >
> > * 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