This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
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