This is the mail archive of the mailing list for the GDB project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Bug 20936 - provide sparc and sparcv9 target description XML files

On 12.12.2016 13:53, Yao Qi wrote:

Hi Ivo,
Your patch does two orthogonal things IMO,

  - Pseudo register support enhancement, patch #1
  - XML target description support and sparc*-tdep.c updates, patch #2,

Can you split them to two patches?

Hi Yao,

Thank you for looking into my changes.
I think I can see your motivation behind splitting the patch into two,
for better readability and manageability.
However my intention was only to provide support for registers
supplied by target description (Valgrind shadow registers in addition
to sparc real hardware ones).
Pseudo register changes were just a necessity to get this done
because Valgrind shadow registers are considered as "real" registers
and thus they are numbered before any pseudo registers.
This also means I cannot draw a clear line between your suggested
patch #1 and #2. Can you take my whole patch as is?

I prefer to using register names provided by target description, does
the code below work for you?

   if (tdesc_has_registers (gdbarch_target_desc (gdbarch)))
     return tdesc_register_name (gdbarch, rawnum);
   else if (regnum >= 0 && regnum < gdbarch_num_regs (gdbarch))
     return sparc32_register_names[regnum];
     return sparc32_pseudo_register_name (gdbarch, regnum);

Yes, it does. Fixed also in other functions returning register names and types.

-  /* Pseudo registers.  */
+/* Pseudo registers.  */
+enum sparc32_pseudo_regnum
    SPARC32_D0_REGNUM,		/* %d0 */
Explicitly set it to zero.


@@ -969,19 +1002,24 @@ sparc64_store_arguments (struct regcache
/* If we're storing the value in a floating-point register,
               also store it in the corresponding %0 register(s).  */
-	  if (regnum >= SPARC64_D0_REGNUM && regnum <= SPARC64_D10_REGNUM)
-	    {
-	      gdb_assert (element < 6);
-	      regnum = SPARC_O0_REGNUM + element;
-	      regcache_cooked_write (regcache, regnum, valbuf);
-	    }
-	  else if (regnum >= SPARC64_Q0_REGNUM && regnum <= SPARC64_Q8_REGNUM)
-	    {
-	      gdb_assert (element < 5);
-	      regnum = SPARC_O0_REGNUM + element;
-	      regcache_cooked_write (regcache, regnum, valbuf);
-	      regcache_cooked_write (regcache, regnum + 1, valbuf + 8);
-	    }
+	  if (regnum >= gdbarch_num_regs (gdbarch))
+            {
The indentation looks odd to me.

Indeed, just looking at the patch it does. This is because the code mixes spaces and tabs and I wanted to preserve as much of existing code as possible, to keep the patch size down. Please look at the resulting file - indentation is ok.

        /* Always store the argument in memory.  */
diff -Npur a/gdb/testsuite/gdb.xml/tdesc-regs.exp b/gdb/testsuite/gdb.xml/tdesc-regs.exp
--- a/gdb/testsuite/gdb.xml/tdesc-regs.exp	2016-02-09 19:19:39.000000000 +0000
+++ b/gdb/testsuite/gdb.xml/tdesc-regs.exp	2016-12-06 15:53:35.418621207 +0000
@@ -49,6 +49,12 @@ switch -glob -- [istarget] {
      "s390*-*-*" {
  	set core-regs {s390-core32.xml s390-acr.xml s390-fpr.xml}
+    "sparc-*-*" {
+        set core-regs {sparc32-cpu.xml sparc32-fpu.xml sparc32-cp0.xml}
+    }
+    "sparc64-*-*" {
+        set core-regs {sparc64-cpu.xml sparc64-fpu.xml sparc64-cp0.xml}
+    }
You need 'set regdir "sparc/"'


	* sparc32-cp0.xml, sparc32-cpu.xml, sparc32-fpu.xml: New files for
	sparc 32-bit.
	* sparc64-cp0.xml, sparc64-cpu.xml, sparc64-fpu.xml: New files
	for sparc 64-bit.
	* sparc32-solaris.xml, sparc64-solaris.xml: New files for sparc32
	and sparc64 on Solaris.
	* sparc-solaris.c, sparc64-solaris.c: Generated.
These file names should be prefixed with "feature/sparc/"


	* sparc-tdep.h: Deal with sparc32 and sparc64 differences
	in target descriptions. Separate real and pseudo registers.
	* sparc-tdep.c: Validate and use registers of the target description.
	Pseudo registers are numbered after all real registers from the target
	description; deal with it.
	* sparc64-tdep.h: Separate real and pseudo registers.
	* sparc64-tdep.c: Pseudo registers are numbered after all real
	registers from the target description; deal with it.
Please describe the change on the function level, take a look at there are two
sections about ChangeLog.


	* gdb.texinfo: New node "Sparc Features".
This should go to a separated entry,


2016-12-07  Ivo Raisr  <>

	* gdb.texinfo (Standard Target Features): Document SPARC features.
	(Sparc Features): New node.


	* tdesc-regs.exp: Provide sparc core registers for the tests.


2016-12-07  Ivo Raisr  <>

	* gdb.xml/tdesc-regs.exp: Provide sparc core registers for
	the tests.


Please find a new patch and 3 new ChangeLog entries. Let me know your comments.


Attachment: sparc_tdesc.patch-002
Description: Text document

Attachment: ChangeLog.doc.entry
Description: Text document

Attachment: ChangeLog.entry-002
Description: Text document

Attachment: ChangeLog.testsuite.entry
Description: Text document

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]