This is the mail archive of the gdb-patches@sourceware.org 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: [RFA] Change AUXV bit checked to decide the size of the FPSCR


El miÃ, 25-03-2009 a las 09:39 -0700, Joel Brobecker escribiÃ:
> (Thiago - I like the quality of the work you produce, very nice!)

Many thanks!

Also thanks for the quick review. After I push enough Python patches
upstream, I hope I can try to do some patch review work too, we will see
then if I am any good at it. :-)

> > For this reason, I'm changing GDB to check for DFP to decide what is the
> > size of the FPSCR (it changed from 32 bits to 64 bits with ISA 2.05 and
> > newer). Since for now the only higher bits used are for Decimal Floating
> > Point, I am changing the code to check the DFP bit in AUXV.
> 
> This sounds more like a work-around than a real fix.

I agree. Truth be told, IMHO the right fix would be to have this bit set
in AUXV for the Power 7. But changing it at this point would have
consequences, and besides I don't think I'm in a position to fight this.

> I don't mind
> your approach, but I think it deserves a clear comment in the code
> where you use that flag to determine the size of your registers.

Great. I added this comment:

  /* Power ISA 2.05 (implemented by Power 6 and newer processors) increases
     the FPSCR from 32 bits to 64 bits. Even though Power 7 supports this
     ISA version, it doesn't have PPC_FEATURE_ARCH_2_05 set, only
     PPC_FEATURE_ARCH_2_06.  Since for now the only bits used in the higher
     half of the register are for Decimal Floating Point, we check if that
     feature is available to decide the size of the FPSCR.  */

> For the record, another solution that you probably considered was to
> check for PPC_FEATURE_ARCH_2_05 *or* PPC_FEATURE_ARCH_2_06, but I can
> see how this might become cumbersome as future revisions get added.

Yes, it would get ugly with time. I don't think DFP support will be
dropped anytime soon, so checking for the feature will work for a good
while.

> > gdb/
> > 	* ppc-linux-nat.c (PPC_FEATURE_ARCH_2_05): Remove #define.
> > 	(PPC_FEATURE_HAS_DFP): New #define.
> > 	(ppc_linux_read_description): Check for DFP feature instead of
> > 	ISA 2.05 to decide on size of the FPSCR.
> > 
> > gdbserver/
> > 	* linux-ppc-low.c (PPC_FEATURE_ARCH_2_05): Remove #define.
> > 	(PPC_FEATURE_HAS_DFP): New #define.
> > 	(ppc_arch_setup): Check for DFP feature instead of ISA 2.05 to decide on
> > 	size of the FPSCR.
> 
> Both OK with the requested comments added.

Committed the following.
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


2009-03-25  Thiago Jung Bauermann  <bauerman@br.ibm.com>

gdb/
	* ppc-linux-nat.c (PPC_FEATURE_ARCH_2_05): Remove #define.
	(PPC_FEATURE_HAS_DFP): New #define.
	(ppc_linux_read_description): Check for DFP feature instead of
	ISA 2.05 to decide on size of the FPSCR.

gdbserver/
	* linux-ppc-low.c (PPC_FEATURE_ARCH_2_05): Remove #define.
	(PPC_FEATURE_HAS_DFP): New #define.
	(ppc_arch_setup): Check for DFP feature instead of ISA 2.05 to decide on
	size of the FPSCR.

Index: src/gdb/gdbserver/linux-ppc-low.c
===================================================================
--- src.orig/gdb/gdbserver/linux-ppc-low.c	2009-02-02 22:12:16.000000000 -0200
+++ src/gdb/gdbserver/linux-ppc-low.c	2009-03-25 18:10:05.000000000 -0300
@@ -28,7 +28,7 @@
 #define PPC_FEATURE_HAS_VSX		0x00000080
 #define PPC_FEATURE_HAS_ALTIVEC         0x10000000
 #define PPC_FEATURE_HAS_SPE             0x00800000
-#define PPC_FEATURE_ARCH_2_05           0x00001000
+#define PPC_FEATURE_HAS_DFP             0x00000400
 
 static unsigned long ppc_hwcap;
 
@@ -274,14 +274,21 @@ ppc_arch_setup (void)
       ppc_get_hwcap (&ppc_hwcap);
       if (ppc_hwcap & PPC_FEATURE_HAS_VSX)
 	{
-	  if (ppc_hwcap & PPC_FEATURE_ARCH_2_05)
+	  /* Power ISA 2.05 (implemented by Power 6 and newer processors)
+	     increases the FPSCR from 32 bits to 64 bits. Even though Power 7
+	     supports this ISA version, it doesn't have PPC_FEATURE_ARCH_2_05
+	     set, only PPC_FEATURE_ARCH_2_06.  Since for now the only bits
+	     used in the higher half of the register are for Decimal Floating
+	     Point, we check if that feature is available to decide the size
+	     of the FPSCR.  */
+	  if (ppc_hwcap & PPC_FEATURE_HAS_DFP)
 	    init_registers_powerpc_isa205_vsx64l ();
 	  else
 	    init_registers_powerpc_vsx64l ();
 	}
       else if (ppc_hwcap & PPC_FEATURE_HAS_ALTIVEC)
 	{
-	  if (ppc_hwcap & PPC_FEATURE_ARCH_2_05)
+	  if (ppc_hwcap & PPC_FEATURE_HAS_DFP)
 	    init_registers_powerpc_isa205_altivec64l ();
 	  else
 	    init_registers_powerpc_altivec64l ();
@@ -297,14 +304,14 @@ ppc_arch_setup (void)
   ppc_get_hwcap (&ppc_hwcap);
   if (ppc_hwcap & PPC_FEATURE_HAS_VSX)
     {
-      if (ppc_hwcap & PPC_FEATURE_ARCH_2_05)
+      if (ppc_hwcap & PPC_FEATURE_HAS_DFP)
 	init_registers_powerpc_isa205_vsx32l ();
       else
 	init_registers_powerpc_vsx32l ();
     }
   else if (ppc_hwcap & PPC_FEATURE_HAS_ALTIVEC)
     {
-      if (ppc_hwcap & PPC_FEATURE_ARCH_2_05)
+      if (ppc_hwcap & PPC_FEATURE_HAS_DFP)
 	init_registers_powerpc_isa205_altivec32l ();
       else
 	init_registers_powerpc_altivec32l ();
Index: src/gdb/ppc-linux-nat.c
===================================================================
--- src.orig/gdb/ppc-linux-nat.c	2009-03-04 10:46:06.000000000 -0300
+++ src/gdb/ppc-linux-nat.c	2009-03-25 18:11:00.000000000 -0300
@@ -63,8 +63,8 @@
 #ifndef PPC_FEATURE_BOOKE
 #define PPC_FEATURE_BOOKE 0x00008000
 #endif
-#ifndef PPC_FEATURE_ARCH_2_05
-#define PPC_FEATURE_ARCH_2_05	0x00001000 /* ISA 2.05 */
+#ifndef PPC_FEATURE_HAS_DFP
+#define PPC_FEATURE_HAS_DFP	0x00000400  /* Decimal Floating Point.  */
 #endif
 
 /* Glibc's headers don't define PTRACE_GETVRREGS so we cannot use a
@@ -1290,7 +1290,13 @@ ppc_linux_read_description (struct targe
 	perror_with_name (_("Unable to fetch AltiVec registers"));
     }
 
-  if (ppc_linux_get_hwcap () & PPC_FEATURE_ARCH_2_05)
+  /* Power ISA 2.05 (implemented by Power 6 and newer processors) increases
+     the FPSCR from 32 bits to 64 bits. Even though Power 7 supports this
+     ISA version, it doesn't have PPC_FEATURE_ARCH_2_05 set, only
+     PPC_FEATURE_ARCH_2_06.  Since for now the only bits used in the higher
+     half of the register are for Decimal Floating Point, we check if that
+     feature is available to decide the size of the FPSCR.  */
+  if (ppc_linux_get_hwcap () & PPC_FEATURE_HAS_DFP)
     isa205 = 1;
 
   /* Check for 64-bit inferior process.  This is the case when the host is



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