Re: [ping 2] [RFA][PATCH v4 0/5] Add TDB regset support

[Andreas Arnez <arnez at linux dot vnet dot ibm dot com>]

Andreas Arnez <arnez@linux.vnet.ibm.com> writes:

> In the scope of this patch set the PowerPC change is purely optional.
> I'll just remove it from the next version of the patch set.

Actually, before I do that, here's another try.

Would it help to merge the iterator function with the logic of
ppc_regset_from_core_section(), like outlined in the patch below?

I see the following advantages:

- One less gdbarch function needed: gdbarch_regset_from_core_section
  becomes obsolete.

- The regset description is more "self-contained": it now includes the
  name, description, and size.

- Even less code than the previous version, all condensed in a single
  function.

Thoughts?


diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
index fc09560..854b356 100644
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -256,54 +256,6 @@ ppc_linux_return_value (struct gdbarch *gdbarch, struct value *function,
 				      readbuf, writebuf);
 }
 
-static struct core_regset_section ppc_linux_vsx_regset_sections[] =
-{
-  { ".reg", 48 * 4, "general-purpose" },
-  { ".reg2", 264, "floating-point" },
-  { ".reg-ppc-vmx", 544, "ppc Altivec" },
-  { ".reg-ppc-vsx", 256, "POWER7 VSX" },
-  { NULL, 0}
-};
-
-static struct core_regset_section ppc_linux_vmx_regset_sections[] =
-{
-  { ".reg", 48 * 4, "general-purpose" },
-  { ".reg2", 264, "floating-point" },
-  { ".reg-ppc-vmx", 544, "ppc Altivec" },
-  { NULL, 0}
-};
-
-static struct core_regset_section ppc_linux_fp_regset_sections[] =
-{
-  { ".reg", 48 * 4, "general-purpose" },
-  { ".reg2", 264, "floating-point" },
-  { NULL, 0}
-};
-
-static struct core_regset_section ppc64_linux_vsx_regset_sections[] =
-{
-  { ".reg", 48 * 8, "general-purpose" },
-  { ".reg2", 264, "floating-point" },
-  { ".reg-ppc-vmx", 544, "ppc Altivec" },
-  { ".reg-ppc-vsx", 256, "POWER7 VSX" },
-  { NULL, 0}
-};
-
-static struct core_regset_section ppc64_linux_vmx_regset_sections[] =
-{
-  { ".reg", 48 * 8, "general-purpose" },
-  { ".reg2", 264, "floating-point" },
-  { ".reg-ppc-vmx", 544, "ppc Altivec" },
-  { NULL, 0}
-};
-
-static struct core_regset_section ppc64_linux_fp_regset_sections[] =
-{
-  { ".reg", 48 * 8, "general-purpose" },
-  { ".reg2", 264, "floating-point" },
-  { NULL, 0}
-};
-
 /* PLT stub in executable.  */
 static struct ppc_insn_pattern powerpc32_plt_stub[] =
   {
@@ -498,35 +450,40 @@ static const struct regset ppc32_linux_gregset = {
   &ppc32_linux_reg_offsets,
   ppc_linux_supply_gregset,
   ppc_linux_collect_gregset,
-  NULL
+  NULL,
+  ".reg", "general-purpose", 48 * 4
 };
 
 static const struct regset ppc64_linux_gregset = {
   &ppc64_linux_reg_offsets,
   ppc_linux_supply_gregset,
   ppc_linux_collect_gregset,
-  NULL
+  NULL,
+  ".reg", "general-purpose", 48 * 8
 };
 
 static const struct regset ppc32_linux_fpregset = {
   &ppc32_linux_reg_offsets,
   ppc_supply_fpregset,
   ppc_collect_fpregset,
-  NULL
+  NULL,
+  ".reg2", "floating-point", 264
 };
 
 static const struct regset ppc32_linux_vrregset = {
   &ppc32_linux_reg_offsets,
   ppc_supply_vrregset,
   ppc_collect_vrregset,
-  NULL
+  NULL,
+  ".reg-ppc-vmx", "ppc Altivec", 544
 };
 
 static const struct regset ppc32_linux_vsxregset = {
   &ppc32_linux_reg_offsets,
   ppc_supply_vsxregset,
   ppc_collect_vsxregset,
-  NULL
+  NULL,
+  ".reg-ppc-vsx", "POWER7 VSX", 256
 };
 
 const struct regset *
@@ -541,25 +498,26 @@ ppc_linux_fpregset (void)
   return &ppc32_linux_fpregset;
 }
 
-static const struct regset *
-ppc_linux_regset_from_core_section (struct gdbarch *core_arch,
-				    const char *sect_name, size_t sect_size)
+static void
+ppc_linux_iterate_over_regset_sections (struct gdbarch *core_arch,
+					iterate_over_regset_sections_cb *cb,
+					void *cb_data,
+					const struct regcache *regcache)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (core_arch);
-  if (strcmp (sect_name, ".reg") == 0)
-    {
-      if (tdep->wordsize == 4)
-	return &ppc32_linux_gregset;
-      else
-	return &ppc64_linux_gregset;
-    }
-  if (strcmp (sect_name, ".reg2") == 0)
-    return &ppc32_linux_fpregset;
-  if (strcmp (sect_name, ".reg-ppc-vmx") == 0)
-    return &ppc32_linux_vrregset;
-  if (strcmp (sect_name, ".reg-ppc-vsx") == 0)
-    return &ppc32_linux_vsxregset;
-  return NULL;
+  int have_vsx = tdep->ppc_vr0_regnum != -1;
+  int have_altivec = tdep->ppc_vsr0_upper_regnum != -1;
+  int res;
+
+  res = cb (tdep->wordsize == 4 ?
+	    &ppc32_linux_gregset : &ppc64_linux_gregset,
+	    cb_data);
+  if (!res)
+    res = cb (&ppc32_linux_fpregset, cb_data);
+  if (!res && have_altivec)
+    res = cb (&ppc32_linux_vrregset, cb_data);
+  if (!res && have_vsx)
+    cb (&ppc32_linux_vsxregset, cb_data);
 }
 
 static void
@@ -1305,19 +1263,6 @@ ppc_linux_init_abi (struct gdbarch_info info,
       else
 	set_gdbarch_gcore_bfd_target (gdbarch, "elf32-powerpc");
 
-      /* Supported register sections.  */
-      if (tdesc_find_feature (info.target_desc,
-			      "org.gnu.gdb.power.vsx"))
-	set_gdbarch_core_regset_sections (gdbarch,
-					  ppc_linux_vsx_regset_sections);
-      else if (tdesc_find_feature (info.target_desc,
-			       "org.gnu.gdb.power.altivec"))
-	set_gdbarch_core_regset_sections (gdbarch,
-					  ppc_linux_vmx_regset_sections);
-      else
-	set_gdbarch_core_regset_sections (gdbarch,
-					  ppc_linux_fp_regset_sections);
-
       if (powerpc_so_ops.in_dynsym_resolve_code == NULL)
 	{
 	  powerpc_so_ops = svr4_so_ops;
@@ -1359,19 +1304,6 @@ ppc_linux_init_abi (struct gdbarch_info info,
 	set_gdbarch_gcore_bfd_target (gdbarch, "elf64-powerpcle");
       else
 	set_gdbarch_gcore_bfd_target (gdbarch, "elf64-powerpc");
-
-      /* Supported register sections.  */
-      if (tdesc_find_feature (info.target_desc,
-			      "org.gnu.gdb.power.vsx"))
-	set_gdbarch_core_regset_sections (gdbarch,
-					  ppc64_linux_vsx_regset_sections);
-      else if (tdesc_find_feature (info.target_desc,
-			       "org.gnu.gdb.power.altivec"))
-	set_gdbarch_core_regset_sections (gdbarch,
-					  ppc64_linux_vmx_regset_sections);
-      else
-	set_gdbarch_core_regset_sections (gdbarch,
-					  ppc64_linux_fp_regset_sections);
     }
 
   /* PPC32 uses a different prpsinfo32 compared to most other Linux
@@ -1383,6 +1315,8 @@ ppc_linux_init_abi (struct gdbarch_info info,
   set_gdbarch_regset_from_core_section (gdbarch,
 					ppc_linux_regset_from_core_section);
   set_gdbarch_core_read_description (gdbarch, ppc_linux_core_read_description);
+  set_gdbarch_iterate_over_regset_sections (gdbarch,
+					    ppc_linux_iterate_over_regset_sections);
 
   /* Enable TLS support.  */
   set_gdbarch_fetch_tls_load_module_address (gdbarch,