* [RFA] Change AUXV bit checked to decide the size of the FPSCR
@ 2009-03-23 15:22 Thiago Jung Bauermann
2009-03-25 16:54 ` Joel Brobecker
0 siblings, 1 reply; 3+ messages in thread
From: Thiago Jung Bauermann @ 2009-03-23 15:22 UTC (permalink / raw)
To: gdb-patches ml
Hi,
I recently found out that even though Power7 will support ISA 2.05, it
will have the PPC_FEATURE_ARCH_2_06 bit set, but not the
PPC_FEATURE_ARCH_2_05 bit (even though it will also support ISA 2.05).
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.
Regression-tested in Linux on a Power6 machine, running the testsuite in
both native and gdbserver configurations.
Ok?
--
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center
2009-03-23 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.
diff --git a/gdb/gdbserver/linux-ppc-low.c b/gdb/gdbserver/linux-ppc-low.c
index b184d5c..29ae832 100644
--- a/gdb/gdbserver/linux-ppc-low.c
+++ b/gdb/gdbserver/linux-ppc-low.c
@@ -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,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_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 +297,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 ();
diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index 034201b..0c0f04c 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -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,7 @@ ppc_linux_read_description (struct target_ops *ops)
perror_with_name (_("Unable to fetch AltiVec registers"));
}
- if (ppc_linux_get_hwcap () & PPC_FEATURE_ARCH_2_05)
+ 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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFA] Change AUXV bit checked to decide the size of the FPSCR
2009-03-23 15:22 [RFA] Change AUXV bit checked to decide the size of the FPSCR Thiago Jung Bauermann
@ 2009-03-25 16:54 ` Joel Brobecker
2009-03-25 21:27 ` Thiago Jung Bauermann
0 siblings, 1 reply; 3+ messages in thread
From: Joel Brobecker @ 2009-03-25 16:54 UTC (permalink / raw)
To: Thiago Jung Bauermann; +Cc: gdb-patches ml
(Thiago - I like the quality of the work you produce, very nice!)
> 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 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.
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.
> 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.
--
Joel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFA] Change AUXV bit checked to decide the size of the FPSCR
2009-03-25 16:54 ` Joel Brobecker
@ 2009-03-25 21:27 ` Thiago Jung Bauermann
0 siblings, 0 replies; 3+ messages in thread
From: Thiago Jung Bauermann @ 2009-03-25 21:27 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches ml
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2009-03-25 21:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-03-23 15:22 [RFA] Change AUXV bit checked to decide the size of the FPSCR Thiago Jung Bauermann
2009-03-25 16:54 ` Joel Brobecker
2009-03-25 21:27 ` Thiago Jung Bauermann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox