[patch/rfc] gdb: clean up x86 cpuid implementations

[patch/rfc] gdb: clean up x86 cpuid implementations

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 10677 bytes --]

We've currently got 3 files doing open coded implementations of cpuid.
Each has its own set of workarounds and varying levels of how well
they're written and are generally hardcoded to specific cpuid functions.
If you try to build the latest gdb as a PIE on an i386 system, the build
will fail because one of them lacks PIC workarounds (wrt ebx).

Specifically, we have:
common/linux-btrace.c:
	two copies of cpuid asm w/specific args, one has no workarounds
	while the other implicitly does to avoid memcpy
go32-nat.c:
	one copy of cpuid asm w/specific args, kind of has workarounds to
	avoid memcpy
gdb/testsuite/gdb.arch/i386-cpuid.h:
	one general cpuid asm w/many workarounds

Fortunately, that last header there is pretty damn good -- it handles
lots of edge cases, the code is nice & tight (uses gcc asm operands
rather than manual movs), and is already almost a general library type
header.

So what I've done is pull that test header out and into gdb/common/
(not sure if there's a better place), generalized the API slightly,
and then cut over all the existing call points to this new header.

Since the func has support for "is cpuid supported on this proc", it
makes it trivial to push the i386/x86_64 ifdefs down into this one
header too.  Now the api is clear (returns 0 if supported, 1 otherwise)
and can be safely used for all targets.

I've verified the gdb.arch testsuite still passes, and this code compiles
for an armv7a host as well as x86_64.  The go32-nat code *looks* correct,
but apparently that's for a DOS target so there's no hope of me testing
it :).

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 gdb/Makefile.in                                 |  2 +-
 gdb/{testsuite/gdb.arch => common}/i386-cpuid.h | 50 +++++++++++++++----------
 gdb/common/linux-btrace.c                       | 41 +++++---------------
 gdb/go32-nat.c                                  | 27 ++++++-------
 gdb/testsuite/gdb.arch/i386-avx.c               |  2 +-
 gdb/testsuite/gdb.arch/i386-avx.exp             |  2 +-
 gdb/testsuite/gdb.arch/i386-sse.c               |  5 ++-
 gdb/testsuite/gdb.arch/i386-sse.exp             |  2 +-
 8 files changed, 60 insertions(+), 71 deletions(-)
 rename gdb/{testsuite/gdb.arch => common}/i386-cpuid.h (87%)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 625ca4a..bfdeb77 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -846,7 +846,7 @@ common/common-utils.h common/xml-utils.h common/buffer.h common/ptid.h \
 common/format.h common/host-defs.h utils.h common/queue.h common/gdb_string.h \
 common/linux-osdata.h gdb-dlfcn.h auto-load.h probe.h stap-probe.h \
 gdb_bfd.h sparc-ravenscar-thread.h ppc-ravenscar-thread.h common/linux-btrace.h \
-ctf.h
+ctf.h common/i386-cpuid.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
diff --git a/gdb/testsuite/gdb.arch/i386-cpuid.h b/gdb/common/i386-cpuid.h
similarity index 87%
rename from gdb/testsuite/gdb.arch/i386-cpuid.h
rename to gdb/common/i386-cpuid.h
index 084a083..c1725bf 100644
--- a/gdb/testsuite/gdb.arch/i386-cpuid.h
+++ b/gdb/common/i386-cpuid.h
@@ -23,6 +23,11 @@
  * <http://www.gnu.org/licenses/>.
  */
 
+#ifndef I386_CPUID_COMMON_H
+#define I386_CPUID_COMMON_H
+
+#if defined(__i386__) || defined(__x86_64__)
+
 /* %ecx */
 #define bit_SSE3	(1 << 0)
 #define bit_PCLMUL	(1 << 1)
@@ -165,36 +170,43 @@ __get_cpuid_max (unsigned int __ext, unsigned int *__sig)
 
 /* Return cpuid data for requested cpuid level, as found in returned
    eax, ebx, ecx and edx registers.  The function checks if cpuid is
-   supported and returns 1 for valid cpuid information or 0 for
-   unsupported cpuid level.  All pointers are required to be non-null.  */
+   supported and returns 0 for valid cpuid information or 1 for
+   unsupported cpuid level.  Pointers may be non-null.  */
 
 static __inline int
-__get_cpuid (unsigned int __level,
-	     unsigned int *__eax, unsigned int *__ebx,
-	     unsigned int *__ecx, unsigned int *__edx)
+i386_cpuid (unsigned int __level,
+	    unsigned int *__eax, unsigned int *__ebx,
+	    unsigned int *__ecx, unsigned int *__edx)
 {
+  unsigned int __scratch;
   unsigned int __ext = __level & 0x80000000;
 
+  if (!__eax)
+    __eax = &__scratch;
+  if (!__ebx)
+    __ebx = &__scratch;
+  if (!__ecx)
+    __ecx = &__scratch;
+  if (!__edx)
+    __edx = &__scratch;
+
   if (__get_cpuid_max (__ext, 0) < __level)
-    return 0;
+    return 1;
 
   __cpuid (__level, *__eax, *__ebx, *__ecx, *__edx);
-  return 1;
+  return 0;
 }
 
-#ifndef NOINLINE
-#define NOINLINE __attribute__ ((noinline))
-#endif
-
-unsigned int i386_cpuid (void) NOINLINE;
+#else
 
-unsigned int NOINLINE
-i386_cpuid (void)
+static __inline int
+i386_cpuid (unsigned int __level,
+	    unsigned int *__eax, unsigned int *__ebx,
+	    unsigned int *__ecx, unsigned int *__edx)
 {
-  unsigned int eax, ebx, ecx, edx;
+  return 1;
+}
 
-  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
-    return 0;
+#endif /* i386 && x86_64 */
 
-  return edx;
-}
+#endif /* I386_CPUID_COMMON_H */
diff --git a/gdb/common/linux-btrace.c b/gdb/common/linux-btrace.c
index d3c8705..386fb12 100644
--- a/gdb/common/linux-btrace.c
+++ b/gdb/common/linux-btrace.c
@@ -30,6 +30,7 @@
 #include "gdb_assert.h"
 #include "regcache.h"
 #include "gdbthread.h"
+#include "i386-cpuid.h"
 
 #if HAVE_LINUX_PERF_EVENT_H
 
@@ -339,13 +340,10 @@ kernel_supports_btrace (void)
 static int
 intel_supports_btrace (void)
 {
-#if defined __i386__ || defined __x86_64__
   unsigned int cpuid, model, family;
 
-  __asm__ __volatile__ ("movl   $1, %%eax;"
-			"cpuid;"
-			: "=a" (cpuid)
-			:: "%ebx", "%ecx", "%edx");
+  if (i386_cpuid (1, &cpuid, NULL, NULL, NULL))
+    return 0;
 
   family = (cpuid >> 8) & 0xf;
   model = (cpuid >> 4) & 0xf;
@@ -376,12 +374,6 @@ intel_supports_btrace (void)
     }
 
   return 1;
-
-#else /* !defined __i386__ && !defined __x86_64__ */
-
-  return 0;
-
-#endif /* !defined __i386__ && !defined __x86_64__ */
 }
 
 /* Check whether the cpu supports branch tracing.  */
@@ -389,22 +381,15 @@ intel_supports_btrace (void)
 static int
 cpu_supports_btrace (void)
 {
-#if defined __i386__ || defined __x86_64__
+  unsigned int ebx, ecx, edx;
   char vendor[13];
 
-  __asm__ __volatile__ ("xorl   %%ebx, %%ebx;"
-			"xorl   %%ecx, %%ecx;"
-			"xorl   %%edx, %%edx;"
-			"movl   $0,    %%eax;"
-			"cpuid;"
-			"movl   %%ebx,  %0;"
-			"movl   %%edx,  %1;"
-			"movl   %%ecx,  %2;"
-			: "=m" (vendor[0]),
-			  "=m" (vendor[4]),
-			  "=m" (vendor[8])
-			:
-			: "%eax", "%ebx", "%ecx", "%edx");
+  if (i386_cpuid (0, NULL, &ebx, &ecx, &edx))
+    return 0;
+
+  memcpy (&vendor[0], &ebx, 4);
+  memcpy (&vendor[4], &ecx, 4);
+  memcpy (&vendor[8], &edx, 4);
   vendor[12] = '\0';
 
   if (strcmp (vendor, "GenuineIntel") == 0)
@@ -412,12 +397,6 @@ cpu_supports_btrace (void)
 
   /* Don't know about others.  Let's assume they do.  */
   return 1;
-
-#else /* !defined __i386__ && !defined __x86_64__ */
-
-  return 0;
-
-#endif /* !defined __i386__ && !defined __x86_64__ */
 }
 
 /* See linux-btrace.h.  */
diff --git a/gdb/go32-nat.c b/gdb/go32-nat.c
index ea6d1ca..72adc88 100644
--- a/gdb/go32-nat.c
+++ b/gdb/go32-nat.c
@@ -96,6 +96,7 @@
 #include "buildsym.h"
 #include "i387-tdep.h"
 #include "i386-tdep.h"
+#include "i386-cpuid.h"
 #include "value.h"
 #include "regcache.h"
 #include "gdb_string.h"
@@ -1140,23 +1141,17 @@ go32_sysinfo (char *arg, int from_tty)
     strcpy (u.machine, "Unknown x86");
   else if (u.machine[0] == 'i' && u.machine[1] > 4)
     {
+      unsigned int eax, ebx, ecx, edx;
+
       /* CPUID with EAX = 0 returns the Vendor ID.  */
-      __asm__ __volatile__ ("xorl   %%ebx, %%ebx;"
-			    "xorl   %%ecx, %%ecx;"
-			    "xorl   %%edx, %%edx;"
-			    "movl   $0,    %%eax;"
-			    "cpuid;"
-			    "movl   %%ebx,  %0;"
-			    "movl   %%edx,  %1;"
-			    "movl   %%ecx,  %2;"
-			    "movl   %%eax,  %3;"
-			    : "=m" (cpuid_vendor[0]),
-			      "=m" (cpuid_vendor[4]),
-			      "=m" (cpuid_vendor[8]),
-			      "=m" (cpuid_max)
-			    :
-			    : "%eax", "%ebx", "%ecx", "%edx");
-      cpuid_vendor[12] = '\0';
+      if (!i386_cpuid (0, &eax, &ebx, &ecx, &edx))
+	{
+	  cpuid_max = eax;
+	  memcpy (&vendor[0], &ebx, 4);
+	  memcpy (&vendor[4], &ecx, 4);
+	  memcpy (&vendor[8], &edx, 4);
+	  cpuid_vendor[12] = '\0';
+	}
     }
 
   printf_filtered ("CPU Type.......................%s", u.machine);
diff --git a/gdb/testsuite/gdb.arch/i386-avx.c b/gdb/testsuite/gdb.arch/i386-avx.c
index bcfa18f..7fd1217 100644
--- a/gdb/testsuite/gdb.arch/i386-avx.c
+++ b/gdb/testsuite/gdb.arch/i386-avx.c
@@ -53,7 +53,7 @@ have_avx (void)
 {
   unsigned int eax, ebx, ecx, edx;
 
-  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
+  if (!i386_cpuid (1, &eax, &ebx, &ecx, &edx))
     return 0;
 
   if ((ecx & (bit_AVX | bit_OSXSAVE)) == (bit_AVX | bit_OSXSAVE))
diff --git a/gdb/testsuite/gdb.arch/i386-avx.exp b/gdb/testsuite/gdb.arch/i386-avx.exp
index 964806c..bbbc6f4 100644
--- a/gdb/testsuite/gdb.arch/i386-avx.exp
+++ b/gdb/testsuite/gdb.arch/i386-avx.exp
@@ -34,7 +34,7 @@ if [get_compiler_info] {
 
 set additional_flags ""
 if [test_compiler_info gcc*] {
-    set additional_flags "additional_flags=-mavx"
+    set additional_flags "additional_flags=-mavx -I${srcdir}/../common"
 }
 
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug $additional_flags]] != "" } {
diff --git a/gdb/testsuite/gdb.arch/i386-sse.c b/gdb/testsuite/gdb.arch/i386-sse.c
index d431527..5d2d277 100644
--- a/gdb/testsuite/gdb.arch/i386-sse.c
+++ b/gdb/testsuite/gdb.arch/i386-sse.c
@@ -51,7 +51,10 @@ v4sf_t data[] =
 int
 have_sse (void)
 {
-  int edx = i386_cpuid ();
+  int edx;
+
+  if (i386_cpuid (1, NULL, NULL, NULL, &edx))
+    return 0;
 
   if (edx & bit_SSE)
     return 1;
diff --git a/gdb/testsuite/gdb.arch/i386-sse.exp b/gdb/testsuite/gdb.arch/i386-sse.exp
index 5923eca..c62a3a0 100644
--- a/gdb/testsuite/gdb.arch/i386-sse.exp
+++ b/gdb/testsuite/gdb.arch/i386-sse.exp
@@ -34,7 +34,7 @@ if [get_compiler_info] {
 
 set additional_flags ""
 if [test_compiler_info gcc*] {
-    set additional_flags "additional_flags=-msse"
+    set additional_flags "additional_flags=-msse -I${srcdir}/../common"
 }
 
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug $additional_flags]] != "" } {

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [patch/rfc] gdb: clean up x86 cpuid implementations

[Eli Zaretskii]

> From: Mike Frysinger <vapier@gentoo.org>
> Date: Mon, 6 May 2013 14:51:24 -0400
> 
> We've currently got 3 files doing open coded implementations of cpuid.
> Each has its own set of workarounds and varying levels of how well
> they're written and are generally hardcoded to specific cpuid functions.
> If you try to build the latest gdb as a PIE on an i386 system, the build
> will fail because one of them lacks PIC workarounds (wrt ebx).

Sorry, I don't follow: what workarounds, and why are they needed?

And what's PIE got to do with the go32 target?

The current code in go32-nat.c was tested to work in all the
environments supported by that target, without GPFaulting or
triggering any other disasters.  I don't think we have the resources
to repeat all that testing with the new code, which tries to detect
newer CPUs, and so could trigger SIGILL.  So I'd like to leave
go32-nat.c alone, if possible.

Re: [patch/rfc] gdb: clean up x86 cpuid implementations

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 1748 bytes --]

On Monday 06 May 2013 15:44:11 Eli Zaretskii wrote:
> From: Mike Frysinger <vapier@gentoo.org>
> > We've currently got 3 files doing open coded implementations of cpuid.
> > Each has its own set of workarounds and varying levels of how well
> > they're written and are generally hardcoded to specific cpuid functions.
> > If you try to build the latest gdb as a PIE on an i386 system, the build
> > will fail because one of them lacks PIC workarounds (wrt ebx).
> 
> Sorry, I don't follow: what workarounds, and why are they needed?

with i386 targets, ebx cannot be an output when building as PIC.  if you build 
gdb as a PIE, the build fails because some of the code in gdb does exactly 
that.

if you look at the test header, you'll see various other workarounds for 
different gcc versions and assemblers.

> And what's PIE got to do with the go32 target?

nothing.  it just has its own open-coded implementation of cpuid which i 
cleaned up to use the new common header.

> The current code in go32-nat.c was tested to work in all the
> environments supported by that target, without GPFaulting or
> triggering any other disasters.  I don't think we have the resources
> to repeat all that testing with the new code, which tries to detect
> newer CPUs, and so could trigger SIGILL.  So I'd like to leave
> go32-nat.c alone, if possible.

on the flip side, if the new common header was tested, go32-nat (and any other 
DOS like target) wouldn't have to do anymore work, and would be easy to use 
cpuid in more places.

but in the end, i don't care about dead things like DOS, so if people prefer 
to leave the cruft in that one file, i can look the other way while still 
cleaning up everything else.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [patch/rfc] gdb: clean up x86 cpuid implementations

[Eli Zaretskii]

> From: Mike Frysinger <vapier@gentoo.org>
> Date: Mon, 6 May 2013 16:30:26 -0400
> Cc: gdb-patches@sourceware.org
> 
> > The current code in go32-nat.c was tested to work in all the
> > environments supported by that target, without GPFaulting or
> > triggering any other disasters.  I don't think we have the resources
> > to repeat all that testing with the new code, which tries to detect
> > newer CPUs, and so could trigger SIGILL.  So I'd like to leave
> > go32-nat.c alone, if possible.
> 
> on the flip side, if the new common header was tested, go32-nat (and any other 
> DOS like target) wouldn't have to do anymore work, and would be easy to use 
> cpuid in more places.

That's in theory.  In practice, go32 (a.k.a. DJGPP) needs a DPMI
server to be able to run 32-bit protected-mode code on top of a 16-bit
real-mode OS.  The DPMI environment has its own peculiar rules about
what can and cannot be done, and more importantly, different DPMI
providers bend those rules in different directions.  The net result is
that not everything that works for any other 386 out there will
necessarily work in the DPMI environment.

> but in the end, i don't care about dead things like DOS, so if people prefer 
> to leave the cruft in that one file, i can look the other way while still 
> cleaning up everything else.

Yes, please.

Thanks.

Re: [patch/rfc] gdb: clean up x86 cpuid implementations

[Doug Evans]

On Mon, May 6, 2013 at 7:41 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Mike Frysinger <vapier@gentoo.org>
>> but in the end, i don't care about dead things like DOS, so if people prefer
>> to leave the cruft in that one file, i can look the other way while still
>> cleaning up everything else.
>
> Yes, please.

I don't have an opinion on which way to go,
but if we leave DOS alone,
can we add a comment explaining why things are the way they are,
so that the next person won't have to dig through archives.
Thanks.

[PATCH v2] gdb: clean up x86 cpuid implementations

[Mike Frysinger]

We've currently got 3 files doing open coded implementations of cpuid.
Each has its own set of workarounds and varying levels of how well
they're written and are generally hardcoded to specific cpuid functions.
If you try to build the latest gdb as a PIE on an i386 system, the build
will fail because one of them lacks PIC workarounds (wrt ebx).

Specifically, we have:
common/linux-btrace.c:
	two copies of cpuid asm w/specific args, one has no workarounds
	while the other implicitly does to avoid memcpy
go32-nat.c:
	two copies of cpuid asm w/specific args, one has workarounds to
	avoid memcpy
gdb/testsuite/gdb.arch/i386-cpuid.h:
	one general cpuid asm w/many workarounds

Fortunately, that last header there is pretty damn good -- it handles
lots of edge cases, the code is nice & tight (uses gcc asm operands
rather than manual movs), and is already almost a general library type
header.

So what I've done is pull that test header out and into gdb/common/
(not sure if there's a better place), generalized the API slightly,
and then cut over all the existing call points to this new header.

Since the func has support for "is cpuid supported on this proc", it
makes it trivial to push the i386/x86_64 ifdefs down into this one
header too.  Now the api is clear (returns 0 if supported, 1 otherwise)
and can be safely used for all targets.

I've verified the gdb.arch testsuite still passes, and this code compiles
for an armv7a host as well as x86_64.  The go32-nat code has been left
ifdef-ed out until someone can test & verify the new stuff works (and if
it doesn't, figure out how to make the new code work).

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v2
	- leave the new code in go32-nat ifdef-ed out for someone to test

 gdb/Makefile.in                                 |  2 +-
 gdb/{testsuite/gdb.arch => common}/i386-cpuid.h | 50 +++++++++++++++----------
 gdb/common/linux-btrace.c                       | 41 +++++---------------
 gdb/go32-nat.c                                  | 22 +++++++++++
 gdb/testsuite/gdb.arch/i386-avx.c               |  2 +-
 gdb/testsuite/gdb.arch/i386-avx.exp             |  2 +-
 gdb/testsuite/gdb.arch/i386-sse.c               |  5 ++-
 gdb/testsuite/gdb.arch/i386-sse.exp             |  2 +-
 8 files changed, 71 insertions(+), 55 deletions(-)
 rename gdb/{testsuite/gdb.arch => common}/i386-cpuid.h (87%)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 625ca4a..bfdeb77 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -846,7 +846,7 @@ common/common-utils.h common/xml-utils.h common/buffer.h common/ptid.h \
 common/format.h common/host-defs.h utils.h common/queue.h common/gdb_string.h \
 common/linux-osdata.h gdb-dlfcn.h auto-load.h probe.h stap-probe.h \
 gdb_bfd.h sparc-ravenscar-thread.h ppc-ravenscar-thread.h common/linux-btrace.h \
-ctf.h
+ctf.h common/i386-cpuid.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
diff --git a/gdb/testsuite/gdb.arch/i386-cpuid.h b/gdb/common/i386-cpuid.h
similarity index 87%
rename from gdb/testsuite/gdb.arch/i386-cpuid.h
rename to gdb/common/i386-cpuid.h
index 084a083..c1725bf 100644
--- a/gdb/testsuite/gdb.arch/i386-cpuid.h
+++ b/gdb/common/i386-cpuid.h
@@ -23,6 +23,11 @@
  * <http://www.gnu.org/licenses/>.
  */
 
+#ifndef I386_CPUID_COMMON_H
+#define I386_CPUID_COMMON_H
+
+#if defined(__i386__) || defined(__x86_64__)
+
 /* %ecx */
 #define bit_SSE3	(1 << 0)
 #define bit_PCLMUL	(1 << 1)
@@ -165,36 +170,43 @@ __get_cpuid_max (unsigned int __ext, unsigned int *__sig)
 
 /* Return cpuid data for requested cpuid level, as found in returned
    eax, ebx, ecx and edx registers.  The function checks if cpuid is
-   supported and returns 1 for valid cpuid information or 0 for
-   unsupported cpuid level.  All pointers are required to be non-null.  */
+   supported and returns 0 for valid cpuid information or 1 for
+   unsupported cpuid level.  Pointers may be non-null.  */
 
 static __inline int
-__get_cpuid (unsigned int __level,
-	     unsigned int *__eax, unsigned int *__ebx,
-	     unsigned int *__ecx, unsigned int *__edx)
+i386_cpuid (unsigned int __level,
+	    unsigned int *__eax, unsigned int *__ebx,
+	    unsigned int *__ecx, unsigned int *__edx)
 {
+  unsigned int __scratch;
   unsigned int __ext = __level & 0x80000000;
 
+  if (!__eax)
+    __eax = &__scratch;
+  if (!__ebx)
+    __ebx = &__scratch;
+  if (!__ecx)
+    __ecx = &__scratch;
+  if (!__edx)
+    __edx = &__scratch;
+
   if (__get_cpuid_max (__ext, 0) < __level)
-    return 0;
+    return 1;
 
   __cpuid (__level, *__eax, *__ebx, *__ecx, *__edx);
-  return 1;
+  return 0;
 }
 
-#ifndef NOINLINE
-#define NOINLINE __attribute__ ((noinline))
-#endif
-
-unsigned int i386_cpuid (void) NOINLINE;
+#else
 
-unsigned int NOINLINE
-i386_cpuid (void)
+static __inline int
+i386_cpuid (unsigned int __level,
+	    unsigned int *__eax, unsigned int *__ebx,
+	    unsigned int *__ecx, unsigned int *__edx)
 {
-  unsigned int eax, ebx, ecx, edx;
+  return 1;
+}
 
-  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
-    return 0;
+#endif /* i386 && x86_64 */
 
-  return edx;
-}
+#endif /* I386_CPUID_COMMON_H */
diff --git a/gdb/common/linux-btrace.c b/gdb/common/linux-btrace.c
index d3c8705..386fb12 100644
--- a/gdb/common/linux-btrace.c
+++ b/gdb/common/linux-btrace.c
@@ -30,6 +30,7 @@
 #include "gdb_assert.h"
 #include "regcache.h"
 #include "gdbthread.h"
+#include "i386-cpuid.h"
 
 #if HAVE_LINUX_PERF_EVENT_H
 
@@ -339,13 +340,10 @@ kernel_supports_btrace (void)
 static int
 intel_supports_btrace (void)
 {
-#if defined __i386__ || defined __x86_64__
   unsigned int cpuid, model, family;
 
-  __asm__ __volatile__ ("movl   $1, %%eax;"
-			"cpuid;"
-			: "=a" (cpuid)
-			:: "%ebx", "%ecx", "%edx");
+  if (i386_cpuid (1, &cpuid, NULL, NULL, NULL))
+    return 0;
 
   family = (cpuid >> 8) & 0xf;
   model = (cpuid >> 4) & 0xf;
@@ -376,12 +374,6 @@ intel_supports_btrace (void)
     }
 
   return 1;
-
-#else /* !defined __i386__ && !defined __x86_64__ */
-
-  return 0;
-
-#endif /* !defined __i386__ && !defined __x86_64__ */
 }
 
 /* Check whether the cpu supports branch tracing.  */
@@ -389,22 +381,15 @@ intel_supports_btrace (void)
 static int
 cpu_supports_btrace (void)
 {
-#if defined __i386__ || defined __x86_64__
+  unsigned int ebx, ecx, edx;
   char vendor[13];
 
-  __asm__ __volatile__ ("xorl   %%ebx, %%ebx;"
-			"xorl   %%ecx, %%ecx;"
-			"xorl   %%edx, %%edx;"
-			"movl   $0,    %%eax;"
-			"cpuid;"
-			"movl   %%ebx,  %0;"
-			"movl   %%edx,  %1;"
-			"movl   %%ecx,  %2;"
-			: "=m" (vendor[0]),
-			  "=m" (vendor[4]),
-			  "=m" (vendor[8])
-			:
-			: "%eax", "%ebx", "%ecx", "%edx");
+  if (i386_cpuid (0, NULL, &ebx, &ecx, &edx))
+    return 0;
+
+  memcpy (&vendor[0], &ebx, 4);
+  memcpy (&vendor[4], &ecx, 4);
+  memcpy (&vendor[8], &edx, 4);
   vendor[12] = '\0';
 
   if (strcmp (vendor, "GenuineIntel") == 0)
@@ -412,12 +397,6 @@ cpu_supports_btrace (void)
 
   /* Don't know about others.  Let's assume they do.  */
   return 1;
-
-#else /* !defined __i386__ && !defined __x86_64__ */
-
-  return 0;
-
-#endif /* !defined __i386__ && !defined __x86_64__ */
 }
 
 /* See linux-btrace.h.  */
diff --git a/gdb/go32-nat.c b/gdb/go32-nat.c
index ea6d1ca..9cde0e7 100644
--- a/gdb/go32-nat.c
+++ b/gdb/go32-nat.c
@@ -96,6 +96,7 @@
 #include "buildsym.h"
 #include "i387-tdep.h"
 #include "i386-tdep.h"
+#include "i386-cpuid.h"
 #include "value.h"
 #include "regcache.h"
 #include "gdb_string.h"
@@ -1141,6 +1142,21 @@ go32_sysinfo (char *arg, int from_tty)
   else if (u.machine[0] == 'i' && u.machine[1] > 4)
     {
       /* CPUID with EAX = 0 returns the Vendor ID.  */
+#if 0
+      /* Ideally we would use i386_cpuid(), but it needs someone to run
+         native tests first to make sure things actually work.  They should.
+         http://sourceware.org/ml/gdb-patches/2013-05/msg00164.html  */
+      unsigned int eax, ebx, ecx, edx;
+
+      if (!i386_cpuid (0, &eax, &ebx, &ecx, &edx))
+	{
+	  cpuid_max = eax;
+	  memcpy (&vendor[0], &ebx, 4);
+	  memcpy (&vendor[4], &ecx, 4);
+	  memcpy (&vendor[8], &edx, 4);
+	  cpuid_vendor[12] = '\0';
+	}
+#else
       __asm__ __volatile__ ("xorl   %%ebx, %%ebx;"
 			    "xorl   %%ecx, %%ecx;"
 			    "xorl   %%edx, %%edx;"
@@ -1157,6 +1173,7 @@ go32_sysinfo (char *arg, int from_tty)
 			    :
 			    : "%eax", "%ebx", "%ecx", "%edx");
       cpuid_vendor[12] = '\0';
+#endif
     }
 
   printf_filtered ("CPU Type.......................%s", u.machine);
@@ -1182,6 +1199,10 @@ go32_sysinfo (char *arg, int from_tty)
       int amd_p = strcmp (cpuid_vendor, "AuthenticAMD") == 0;
       unsigned cpu_family, cpu_model;
 
+#if 0
+      /* See comment above about cpuid usage.  */
+      i386_cpuid (1, &cpuid_eax, &cpuid_ebx, NULL, &cpuid_edx);
+#else
       __asm__ __volatile__ ("movl   $1, %%eax;"
 			    "cpuid;"
 			    : "=a" (cpuid_eax),
@@ -1189,6 +1210,7 @@ go32_sysinfo (char *arg, int from_tty)
 			      "=d" (cpuid_edx)
 			    :
 			    : "%ecx");
+#endif
       brand_idx = cpuid_ebx & 0xff;
       cpu_family = (cpuid_eax >> 8) & 0xf;
       cpu_model  = (cpuid_eax >> 4) & 0xf;
diff --git a/gdb/testsuite/gdb.arch/i386-avx.c b/gdb/testsuite/gdb.arch/i386-avx.c
index bcfa18f..7fd1217 100644
--- a/gdb/testsuite/gdb.arch/i386-avx.c
+++ b/gdb/testsuite/gdb.arch/i386-avx.c
@@ -53,7 +53,7 @@ have_avx (void)
 {
   unsigned int eax, ebx, ecx, edx;
 
-  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
+  if (!i386_cpuid (1, &eax, &ebx, &ecx, &edx))
     return 0;
 
   if ((ecx & (bit_AVX | bit_OSXSAVE)) == (bit_AVX | bit_OSXSAVE))
diff --git a/gdb/testsuite/gdb.arch/i386-avx.exp b/gdb/testsuite/gdb.arch/i386-avx.exp
index 964806c..bbbc6f4 100644
--- a/gdb/testsuite/gdb.arch/i386-avx.exp
+++ b/gdb/testsuite/gdb.arch/i386-avx.exp
@@ -34,7 +34,7 @@ if [get_compiler_info] {
 
 set additional_flags ""
 if [test_compiler_info gcc*] {
-    set additional_flags "additional_flags=-mavx"
+    set additional_flags "additional_flags=-mavx -I${srcdir}/../common"
 }
 
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug $additional_flags]] != "" } {
diff --git a/gdb/testsuite/gdb.arch/i386-sse.c b/gdb/testsuite/gdb.arch/i386-sse.c
index d431527..5d2d277 100644
--- a/gdb/testsuite/gdb.arch/i386-sse.c
+++ b/gdb/testsuite/gdb.arch/i386-sse.c
@@ -51,7 +51,10 @@ v4sf_t data[] =
 int
 have_sse (void)
 {
-  int edx = i386_cpuid ();
+  int edx;
+
+  if (i386_cpuid (1, NULL, NULL, NULL, &edx))
+    return 0;
 
   if (edx & bit_SSE)
     return 1;
diff --git a/gdb/testsuite/gdb.arch/i386-sse.exp b/gdb/testsuite/gdb.arch/i386-sse.exp
index 5923eca..c62a3a0 100644
--- a/gdb/testsuite/gdb.arch/i386-sse.exp
+++ b/gdb/testsuite/gdb.arch/i386-sse.exp
@@ -34,7 +34,7 @@ if [get_compiler_info] {
 
 set additional_flags ""
 if [test_compiler_info gcc*] {
-    set additional_flags "additional_flags=-msse"
+    set additional_flags "additional_flags=-msse -I${srcdir}/../common"
 }
 
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug $additional_flags]] != "" } {
-- 
1.8.2.1

Re: [PATCH v2] gdb: clean up x86 cpuid implementations

[Pedro Alves]

On 05/07/2013 02:29 PM, Mike Frysinger wrote:
> 
> Fortunately, that last header there is pretty damn good -- it handles
> lots of edge cases, the code is nice & tight (uses gcc asm operands
> rather than manual movs), and is already almost a general library type
> header.

The top of the header says:

/* Helper file for i386 platform.  Runtime check for MMX/SSE/SSE2/AVX
 * support. Copied from gcc 4.4.

I'd rather not fork the gcc file.  If we need to wrap its functions/macros
for gdb's purpose, I'd rather do that in a separate file that
#includes (a copy of) gcc's, verbatim, so we can pull updates from upstream
easily.  In fact, diffing our copy against gcc's shows we're already
out of date --- see below.  The bits removed are gdb-specific additions.

I wonder whether pushing the file down to libiberty, so both gcc
and gdb could share it would be viable?

--- testsuite/gdb.arch/i386-cpuid.h	2013-04-29 20:19:08.501275440 +0100
+++ /home/pedro/src/gcc/src/gcc/config/i386/cpuid.h	2013-01-17 13:29:24.753297159 +0000
@@ -1,6 +1,4 @@
-/* Helper file for i386 platform.  Runtime check for MMX/SSE/SSE2/AVX
- * support. Copied from gcc 4.4.
- *
+/*
  * Copyright (C) 2007-2013 Free Software Foundation, Inc.
  *
  * This file is free software; you can redistribute it and/or modify it
@@ -26,6 +24,7 @@
 /* %ecx */
 #define bit_SSE3	(1 << 0)
 #define bit_PCLMUL	(1 << 1)
+#define bit_LZCNT	(1 << 5)
 #define bit_SSSE3	(1 << 9)
 #define bit_FMA		(1 << 12)
 #define bit_CMPXCHG16B	(1 << 13)
@@ -37,6 +36,8 @@
 #define bit_XSAVE	(1 << 26)
 #define bit_OSXSAVE	(1 << 27)
 #define bit_AVX		(1 << 28)
+#define bit_F16C	(1 << 29)
+#define bit_RDRND	(1 << 30)

 /* %edx */
 #define bit_CMPXCHG8B	(1 << 8)
@@ -51,49 +52,133 @@
 #define bit_LAHF_LM	(1 << 0)
 #define bit_ABM		(1 << 5)
 #define bit_SSE4a	(1 << 6)
+#define bit_PRFCHW	(1 << 8)
 #define bit_XOP         (1 << 11)
 #define bit_LWP 	(1 << 15)
 #define bit_FMA4        (1 << 16)
+#define bit_TBM         (1 << 21)

 /* %edx */
+#define bit_MMXEXT	(1 << 22)
 #define bit_LM		(1 << 29)
 #define bit_3DNOWP	(1 << 30)
 #define bit_3DNOW	(1 << 31)

+/* Extended Features (%eax == 7) */
+#define bit_FSGSBASE	(1 << 0)
+#define bit_BMI	(1 << 3)
+#define bit_HLE	(1 << 4)
+#define bit_AVX2	(1 << 5)
+#define bit_BMI2	(1 << 8)
+#define bit_RTM	(1 << 11)
+#define bit_RDSEED	(1 << 18)
+#define bit_ADX	(1 << 19)
+
+/* Extended State Enumeration Sub-leaf (%eax == 13, %ecx == 1) */
+#define bit_XSAVEOPT	(1 << 0)
+
+/* Signatures for different CPU implementations as returned in uses
+   of cpuid with level 0.  */
+#define signature_AMD_ebx	0x68747541
+#define signature_AMD_ecx	0x444d4163
+#define signature_AMD_edx	0x69746e65
+
+#define signature_CENTAUR_ebx	0x746e6543
+#define signature_CENTAUR_ecx	0x736c7561
+#define signature_CENTAUR_edx	0x48727561
+
+#define signature_CYRIX_ebx	0x69727943
+#define signature_CYRIX_ecx	0x64616574
+#define signature_CYRIX_edx	0x736e4978
+
+#define signature_INTEL_ebx	0x756e6547
+#define signature_INTEL_ecx	0x6c65746e
+#define signature_INTEL_edx	0x49656e69
+
+#define signature_TM1_ebx	0x6e617254
+#define signature_TM1_ecx	0x55504361
+#define signature_TM1_edx	0x74656d73
+
+#define signature_TM2_ebx	0x756e6547
+#define signature_TM2_ecx	0x3638784d
+#define signature_TM2_edx	0x54656e69
+
+#define signature_NSC_ebx	0x646f6547
+#define signature_NSC_ecx	0x43534e20
+#define signature_NSC_edx	0x79622065
+
+#define signature_NEXGEN_ebx	0x4778654e
+#define signature_NEXGEN_ecx	0x6e657669
+#define signature_NEXGEN_edx	0x72446e65
+
+#define signature_RISE_ebx	0x65736952
+#define signature_RISE_ecx	0x65736952
+#define signature_RISE_edx	0x65736952
+
+#define signature_SIS_ebx	0x20536953
+#define signature_SIS_ecx	0x20536953
+#define signature_SIS_edx	0x20536953
+
+#define signature_UMC_ebx	0x20434d55
+#define signature_UMC_ecx	0x20434d55
+#define signature_UMC_edx	0x20434d55
+
+#define signature_VIA_ebx	0x20414956
+#define signature_VIA_ecx	0x20414956
+#define signature_VIA_edx	0x20414956
+
+#define signature_VORTEX_ebx	0x74726f56
+#define signature_VORTEX_ecx	0x436f5320
+#define signature_VORTEX_edx	0x36387865

 #if defined(__i386__) && defined(__PIC__)
 /* %ebx may be the PIC register.  */
 #if __GNUC__ >= 3
 #define __cpuid(level, a, b, c, d)			\
-  __asm__ ("xchg{l}\t{%%}ebx, %1\n\t"			\
+  __asm__ ("xchg{l}\t{%%}ebx, %k1\n\t"			\
 	   "cpuid\n\t"					\
-	   "xchg{l}\t{%%}ebx, %1\n\t"			\
-	   : "=a" (a), "=r" (b), "=c" (c), "=d" (d)	\
+	   "xchg{l}\t{%%}ebx, %k1\n\t"			\
+	   : "=a" (a), "=&r" (b), "=c" (c), "=d" (d)	\
 	   : "0" (level))

 #define __cpuid_count(level, count, a, b, c, d)		\
-  __asm__ ("xchg{l}\t{%%}ebx, %1\n\t"			\
+  __asm__ ("xchg{l}\t{%%}ebx, %k1\n\t"			\
 	   "cpuid\n\t"					\
-	   "xchg{l}\t{%%}ebx, %1\n\t"			\
-	   : "=a" (a), "=r" (b), "=c" (c), "=d" (d)	\
+	   "xchg{l}\t{%%}ebx, %k1\n\t"			\
+	   : "=a" (a), "=&r" (b), "=c" (c), "=d" (d)	\
 	   : "0" (level), "2" (count))
 #else
 /* Host GCCs older than 3.0 weren't supporting Intel asm syntax
    nor alternatives in i386 code.  */
 #define __cpuid(level, a, b, c, d)			\
-  __asm__ ("xchgl\t%%ebx, %1\n\t"			\
+  __asm__ ("xchgl\t%%ebx, %k1\n\t"			\
 	   "cpuid\n\t"					\
-	   "xchgl\t%%ebx, %1\n\t"			\
-	   : "=a" (a), "=r" (b), "=c" (c), "=d" (d)	\
+	   "xchgl\t%%ebx, %k1\n\t"			\
+	   : "=a" (a), "=&r" (b), "=c" (c), "=d" (d)	\
 	   : "0" (level))

 #define __cpuid_count(level, count, a, b, c, d)		\
-  __asm__ ("xchgl\t%%ebx, %1\n\t"			\
+  __asm__ ("xchgl\t%%ebx, %k1\n\t"			\
 	   "cpuid\n\t"					\
-	   "xchgl\t%%ebx, %1\n\t"			\
-	   : "=a" (a), "=r" (b), "=c" (c), "=d" (d)	\
+	   "xchgl\t%%ebx, %k1\n\t"			\
+	   : "=a" (a), "=&r" (b), "=c" (c), "=d" (d)	\
 	   : "0" (level), "2" (count))
 #endif
+#elif defined(__x86_64__) && (defined(__code_model_medium__) || defined(__code_model_large__)) && defined(__PIC__)
+/* %rbx may be the PIC register.  */
+#define __cpuid(level, a, b, c, d)			\
+  __asm__ ("xchg{q}\t{%%}rbx, %q1\n\t"			\
+	   "cpuid\n\t"					\
+	   "xchg{q}\t{%%}rbx, %q1\n\t"			\
+	   : "=a" (a), "=&r" (b), "=c" (c), "=d" (d)	\
+	   : "0" (level))
+
+#define __cpuid_count(level, count, a, b, c, d)		\
+  __asm__ ("xchg{q}\t{%%}rbx, %q1\n\t"			\
+	   "cpuid\n\t"					\
+	   "xchg{q}\t{%%}rbx, %q1\n\t"			\
+	   : "=a" (a), "=&r" (b), "=c" (c), "=d" (d)	\
+	   : "0" (level), "2" (count))
 #else
 #define __cpuid(level, a, b, c, d)			\
   __asm__ ("cpuid\n\t"					\
@@ -119,8 +204,8 @@ __get_cpuid_max (unsigned int __ext, uns
   unsigned int __eax, __ebx, __ecx, __edx;

 #ifndef __x86_64__
-#if __GNUC__ >= 3
   /* See if we can use cpuid.  On AMD64 we always can.  */
+#if __GNUC__ >= 3
   __asm__ ("pushf{l|d}\n\t"
 	   "pushf{l|d}\n\t"
 	   "pop{l}\t%0\n\t"
@@ -181,20 +266,3 @@ __get_cpuid (unsigned int __level,
   __cpuid (__level, *__eax, *__ebx, *__ecx, *__edx);
   return 1;
 }
-
-#ifndef NOINLINE
-#define NOINLINE __attribute__ ((noinline))
-#endif
-
-unsigned int i386_cpuid (void) NOINLINE;
-
-unsigned int NOINLINE
-i386_cpuid (void)
-{
-  unsigned int eax, ebx, ecx, edx;
-
-  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
-    return 0;
-
-  return edx;
-}

Re: [PATCH v2] gdb: clean up x86 cpuid implementations

[Pedro Alves]

On 05/07/2013 03:08 PM, Pedro Alves wrote:
> On 05/07/2013 02:29 PM, Mike Frysinger wrote:
>>
>> Fortunately, that last header there is pretty damn good -- it handles
>> lots of edge cases, the code is nice & tight (uses gcc asm operands
>> rather than manual movs), and is already almost a general library type
>> header.
> 
> The top of the header says:
> 
> /* Helper file for i386 platform.  Runtime check for MMX/SSE/SSE2/AVX
>  * support. Copied from gcc 4.4.
> 
> I'd rather not fork the gcc file.  If we need to wrap its functions/macros
> for gdb's purpose, I'd rather do that in a separate file that
> #includes (a copy of) gcc's, verbatim, so we can pull updates from upstream
> easily.  In fact, diffing our copy against gcc's shows we're already
> out of date --- see below.  The bits removed are gdb-specific additions.
> 
> I wonder whether pushing the file down to libiberty, so both gcc
> and gdb could share it would be viable?

Actually, it seems like __get_cpuid is a gcc built-in nowadays, but I don't
when it was added.  We could make use of it, and only fallback to the header
copy if the host compiler doesn't have the builtin.

-- 
Pedro Alves

Re: [PATCH v2] gdb: clean up x86 cpuid implementations

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 1511 bytes --]

On Tuesday 07 May 2013 10:19:06 Pedro Alves wrote:
> On 05/07/2013 03:08 PM, Pedro Alves wrote:
> > On 05/07/2013 02:29 PM, Mike Frysinger wrote:
> >> Fortunately, that last header there is pretty damn good -- it handles
> >> lots of edge cases, the code is nice & tight (uses gcc asm operands
> >> rather than manual movs), and is already almost a general library type
> >> header.
> > 
> > The top of the header says:
> > 
> > /* Helper file for i386 platform.  Runtime check for MMX/SSE/SSE2/AVX
> >  * support. Copied from gcc 4.4.
> > 
> > I'd rather not fork the gcc file.  If we need to wrap its
> > functions/macros for gdb's purpose, I'd rather do that in a separate
> > file that
> > #includes (a copy of) gcc's, verbatim, so we can pull updates from
> > upstream easily.  In fact, diffing our copy against gcc's shows we're
> > already out of date --- see below.  The bits removed are gdb-specific
> > additions.
> > 
> > I wonder whether pushing the file down to libiberty, so both gcc
> > and gdb could share it would be viable?
> 
> Actually, it seems like __get_cpuid is a gcc built-in nowadays, but I don't
> when it was added.  We could make use of it, and only fallback to the
> header copy if the host compiler doesn't have the builtin.

yes, gcc introduced a cpuid.h starting with gcc-4.3.0.  i wanted to focus on 
getting everyone on the same header first before tackling that.  i didn't think 
people would be ok with x86 builds requiring gcc-4.3.0 ?
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2] gdb: clean up x86 cpuid implementations

[Pedro Alves]

On 05/07/2013 03:31 PM, Mike Frysinger wrote:
> On Tuesday 07 May 2013 10:19:06 Pedro Alves wrote:
>> On 05/07/2013 03:08 PM, Pedro Alves wrote:
>>> On 05/07/2013 02:29 PM, Mike Frysinger wrote:
>>>> Fortunately, that last header there is pretty damn good -- it handles
>>>> lots of edge cases, the code is nice & tight (uses gcc asm operands
>>>> rather than manual movs), and is already almost a general library type
>>>> header.
>>>
>>> The top of the header says:
>>>
>>> /* Helper file for i386 platform.  Runtime check for MMX/SSE/SSE2/AVX
>>>  * support. Copied from gcc 4.4.
>>>
>>> I'd rather not fork the gcc file.  If we need to wrap its
>>> functions/macros for gdb's purpose, I'd rather do that in a separate
>>> file that
>>> #includes (a copy of) gcc's, verbatim, so we can pull updates from
>>> upstream easily.  In fact, diffing our copy against gcc's shows we're
>>> already out of date --- see below.  The bits removed are gdb-specific
>>> additions.
>>>
>>> I wonder whether pushing the file down to libiberty, so both gcc
>>> and gdb could share it would be viable?
>>
>> Actually, it seems like __get_cpuid is a gcc built-in nowadays, but I don't
>> when it was added.  We could make use of it, and only fallback to the
>> header copy if the host compiler doesn't have the builtin.
> 
> yes, gcc introduced a cpuid.h starting with gcc-4.3.0.  i wanted to focus on 
> getting everyone on the same header first before tackling that.  

Your changes were effectively diverging our header from gcc's, not
converging.

i didn't think  people would be ok with x86 builds requiring gcc-4.3.0 ?

Right, and I did not suggest that?  The fallback part would take care
of < gcc 4.3 (and then at some point in the distant future older gccs
would become irrelevant and we drop the fallback).  But yes, an autocheck
can/could be done separately.

Really the main issue is with the forking of gcc's __get_cpuid,
like in

 static __inline int
-__get_cpuid (unsigned int __level,
-	     unsigned int *__eax, unsigned int *__ebx,
-	     unsigned int *__ecx, unsigned int *__edx)
+i386_cpuid (unsigned int __level,
+	    unsigned int *__eax, unsigned int *__ebx,
+	    unsigned int *__ecx, unsigned int *__edx)
 {
+  unsigned int __scratch;
   unsigned int __ext = __level & 0x80000000;

+  if (!__eax)
+    __eax = &__scratch;
+  if (!__ebx)
+    __ebx = &__scratch;
+  if (!__ecx)
+    __ecx = &__scratch;
+  if (!__edx)
+    __edx = &__scratch;
+
   if (__get_cpuid_max (__ext, 0) < __level)
-    return 0;
+    return 1;

instead of building on it.

-- 
Pedro Alves

Re: [PATCH v2] gdb: clean up x86 cpuid implementations

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 2781 bytes --]

On Tuesday 07 May 2013 10:49:29 Pedro Alves wrote:
> On 05/07/2013 03:31 PM, Mike Frysinger wrote:
> > On Tuesday 07 May 2013 10:19:06 Pedro Alves wrote:
> >> On 05/07/2013 03:08 PM, Pedro Alves wrote:
> >>> On 05/07/2013 02:29 PM, Mike Frysinger wrote:
> >>>> Fortunately, that last header there is pretty damn good -- it handles
> >>>> lots of edge cases, the code is nice & tight (uses gcc asm operands
> >>>> rather than manual movs), and is already almost a general library type
> >>>> header.
> >>> 
> >>> The top of the header says:
> >>> 
> >>> /* Helper file for i386 platform.  Runtime check for MMX/SSE/SSE2/AVX
> >>> 
> >>>  * support. Copied from gcc 4.4.
> >>> 
> >>> I'd rather not fork the gcc file.  If we need to wrap its
> >>> functions/macros for gdb's purpose, I'd rather do that in a separate
> >>> file that
> >>> #includes (a copy of) gcc's, verbatim, so we can pull updates from
> >>> upstream easily.  In fact, diffing our copy against gcc's shows we're
> >>> already out of date --- see below.  The bits removed are gdb-specific
> >>> additions.
> >>> 
> >>> I wonder whether pushing the file down to libiberty, so both gcc
> >>> and gdb could share it would be viable?
> >> 
> >> Actually, it seems like __get_cpuid is a gcc built-in nowadays, but I
> >> don't when it was added.  We could make use of it, and only fallback to
> >> the header copy if the host compiler doesn't have the builtin.
> > 
> > yes, gcc introduced a cpuid.h starting with gcc-4.3.0.  i wanted to focus
> > on getting everyone on the same header first before tackling that.
> 
> Your changes were effectively diverging our header from gcc's, not
> converging.

the header provides a simple API that sits between the gcc cpuid.h and gdb and 
provides a simpler interface (usable for all arches, and arguments are 
optional).  what happens internally (include gcc cpuid.h or some copy or 
whatever) is irrelevant to the external consumers.

> > i didn't think  people would be ok with x86 builds requiring gcc-4.3.0 ?
> 
> Right, and I did not suggest that?

i didn't say you did.  i was asking a question to see if we could avoid having 
a copy at all.

> The fallback part would take care
> of < gcc 4.3 (and then at some point in the distant future older gccs
> would become irrelevant and we drop the fallback).  But yes, an autocheck
> can/could be done separately.

if we're going to ship a copy of the gcc header, then we might as always just 
use that instead of switching between the gcc version and the local copy.  
otherwise we get into the situation where the API used by people works for the 
people who do development and often have the latest gcc version and people who 
build with older versions.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH v3] gdb: clean up x86 cpuid implementations

[Mike Frysinger]

We've currently got 3 files doing open coded implementations of cpuid.
Each has its own set of workarounds and varying levels of how well
they're written and are generally hardcoded to specific cpuid functions.
If you try to build the latest gdb as a PIE on an i386 system, the build
will fail because one of them lacks PIC workarounds (wrt ebx).

Specifically, we have:
common/linux-btrace.c:
	two copies of cpuid asm w/specific args, one has no workarounds
	while the other implicitly does to avoid memcpy
go32-nat.c:
	two copies of cpuid asm w/specific args, one has workarounds to
	avoid memcpy
gdb/testsuite/gdb.arch/i386-cpuid.h:
	one general cpuid asm w/many workarounds copied from older gcc

Fortunately, that last header there is pretty damn good -- it handles
lots of edge cases, the code is nice & tight (uses gcc asm operands
rather than manual movs), and is already almost a general library type
header.  It's also the basis of what is now the public cpuid.h that is
shipped with gcc-4.3+.

So what I've done is pull that test header out and into gdb/common/
(not sure if there's a better place), put a wrapper API around it,
and then cut over all the existing call points to this new header.

Since the func already has support for "is cpuid supported on this proc",
it makes it trivial to push the i386/x86_64 ifdefs down into this wrapper
API too.  Now it can be safely used for all targets and gcc will elide
the unused code for us.

I've verified the gdb.arch testsuite still passes, and this code compiles
for an armv7a host as well as x86_64.  The go32-nat code has been left
ifdef-ed out until someone can test & verify the new stuff works (and if
it doesn't, figure out how to make the new code work).

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v3
	- split wrapper api and gcc cpuid.h into sep files

 gdb/Makefile.in                                    |   2 +-
 gdb/common/i386-cpuid.h                            |  68 ++++++++++
 .../i386-cpuid.h => common/i386-gcc-cpuid.h}       | 140 ++++++++++++++++-----
 gdb/common/linux-btrace.c                          |  41 ++----
 gdb/go32-nat.c                                     |  22 ++++
 gdb/testsuite/gdb.arch/i386-avx.c                  |   2 +-
 gdb/testsuite/gdb.arch/i386-avx.exp                |   2 +-
 gdb/testsuite/gdb.arch/i386-sse.c                  |   5 +-
 gdb/testsuite/gdb.arch/i386-sse.exp                |   2 +-
 9 files changed, 214 insertions(+), 70 deletions(-)
 create mode 100644 gdb/common/i386-cpuid.h
 rename gdb/{testsuite/gdb.arch/i386-cpuid.h => common/i386-gcc-cpuid.h} (59%)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 625ca4a..e3f96e1 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -846,7 +846,7 @@ common/common-utils.h common/xml-utils.h common/buffer.h common/ptid.h \
 common/format.h common/host-defs.h utils.h common/queue.h common/gdb_string.h \
 common/linux-osdata.h gdb-dlfcn.h auto-load.h probe.h stap-probe.h \
 gdb_bfd.h sparc-ravenscar-thread.h ppc-ravenscar-thread.h common/linux-btrace.h \
-ctf.h
+ctf.h common/i386-cpuid.h common/i386-gcc-cpuid.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
diff --git a/gdb/common/i386-cpuid.h b/gdb/common/i386-cpuid.h
new file mode 100644
index 0000000..fca3233
--- /dev/null
+++ b/gdb/common/i386-cpuid.h
@@ -0,0 +1,68 @@
+/*
+ * Copyright (C) 2007-2013 Free Software Foundation, Inc.
+ *
+ * This file is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 3, or (at your option) any
+ * later version.
+ *
+ * This file is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * Under Section 7 of GPL version 3, you are granted additional
+ * permissions described in the GCC Runtime Library Exception, version
+ * 3.1, as published by the Free Software Foundation.
+ *
+ * You should have received a copy of the GNU General Public License and
+ * a copy of the GCC Runtime Library Exception along with this program;
+ * see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef I386_CPUID_COMMON_H
+#define I386_CPUID_COMMON_H
+
+/* Always include the header for the cpu bit defines.  */
+#include "i386-gcc-cpuid.h"
+
+#if defined(__i386__) || defined(__x86_64__)
+
+/* Return cpuid data for requested cpuid level, as found in returned
+   eax, ebx, ecx and edx registers.  The function checks if cpuid is
+   supported and returns 1 for valid cpuid information or 0 for
+   unsupported cpuid level.  Pointers may be non-null.  */
+
+static __inline int
+i386_cpuid (unsigned int __level,
+	    unsigned int *__eax, unsigned int *__ebx,
+	    unsigned int *__ecx, unsigned int *__edx)
+{
+  unsigned int __scratch;
+
+  if (!__eax)
+    __eax = &__scratch;
+  if (!__ebx)
+    __ebx = &__scratch;
+  if (!__ecx)
+    __ecx = &__scratch;
+  if (!__edx)
+    __edx = &__scratch;
+
+  return __get_cpuid (__level, __eax, __ebx, __ecx, __edx);
+}
+
+#else
+
+static __inline int
+i386_cpuid (unsigned int __level,
+	    unsigned int *__eax, unsigned int *__ebx,
+	    unsigned int *__ecx, unsigned int *__edx)
+{
+  return 0;
+}
+
+#endif /* i386 && x86_64 */
+
+#endif /* I386_CPUID_COMMON_H */
diff --git a/gdb/testsuite/gdb.arch/i386-cpuid.h b/gdb/common/i386-gcc-cpuid.h
similarity index 59%
rename from gdb/testsuite/gdb.arch/i386-cpuid.h
rename to gdb/common/i386-gcc-cpuid.h
index 084a083..e045ba8 100644
--- a/gdb/testsuite/gdb.arch/i386-cpuid.h
+++ b/gdb/common/i386-gcc-cpuid.h
@@ -1,6 +1,8 @@
-/* Helper file for i386 platform.  Runtime check for MMX/SSE/SSE2/AVX
- * support. Copied from gcc 4.4.
- *
+/*
+ * Helper cpuid.h file copied from gcc-4.8.0.  Code in gdb should not
+ * include this directly, but pull in i386-cpuid.h and use that func.
+ */
+/*
  * Copyright (C) 2007-2013 Free Software Foundation, Inc.
  *
  * This file is free software; you can redistribute it and/or modify it
@@ -26,6 +28,7 @@
 /* %ecx */
 #define bit_SSE3	(1 << 0)
 #define bit_PCLMUL	(1 << 1)
+#define bit_LZCNT	(1 << 5)
 #define bit_SSSE3	(1 << 9)
 #define bit_FMA		(1 << 12)
 #define bit_CMPXCHG16B	(1 << 13)
@@ -37,6 +40,8 @@
 #define bit_XSAVE	(1 << 26)
 #define bit_OSXSAVE	(1 << 27)
 #define bit_AVX		(1 << 28)
+#define bit_F16C	(1 << 29)
+#define bit_RDRND	(1 << 30)
 
 /* %edx */
 #define bit_CMPXCHG8B	(1 << 8)
@@ -51,49 +56,133 @@
 #define bit_LAHF_LM	(1 << 0)
 #define bit_ABM		(1 << 5)
 #define bit_SSE4a	(1 << 6)
+#define bit_PRFCHW	(1 << 8)
 #define bit_XOP         (1 << 11)
 #define bit_LWP 	(1 << 15)
 #define bit_FMA4        (1 << 16)
+#define bit_TBM         (1 << 21)
 
 /* %edx */
+#define bit_MMXEXT	(1 << 22)
 #define bit_LM		(1 << 29)
 #define bit_3DNOWP	(1 << 30)
 #define bit_3DNOW	(1 << 31)
 
+/* Extended Features (%eax == 7) */
+#define bit_FSGSBASE	(1 << 0)
+#define bit_BMI	(1 << 3)
+#define bit_HLE	(1 << 4)
+#define bit_AVX2	(1 << 5)
+#define bit_BMI2	(1 << 8)
+#define bit_RTM	(1 << 11)
+#define bit_RDSEED	(1 << 18)
+#define bit_ADX	(1 << 19)
+
+/* Extended State Enumeration Sub-leaf (%eax == 13, %ecx == 1) */
+#define bit_XSAVEOPT	(1 << 0)
+
+/* Signatures for different CPU implementations as returned in uses
+   of cpuid with level 0.  */
+#define signature_AMD_ebx	0x68747541
+#define signature_AMD_ecx	0x444d4163
+#define signature_AMD_edx	0x69746e65
+
+#define signature_CENTAUR_ebx	0x746e6543
+#define signature_CENTAUR_ecx	0x736c7561
+#define signature_CENTAUR_edx	0x48727561
+
+#define signature_CYRIX_ebx	0x69727943
+#define signature_CYRIX_ecx	0x64616574
+#define signature_CYRIX_edx	0x736e4978
+
+#define signature_INTEL_ebx	0x756e6547
+#define signature_INTEL_ecx	0x6c65746e
+#define signature_INTEL_edx	0x49656e69
+
+#define signature_TM1_ebx	0x6e617254
+#define signature_TM1_ecx	0x55504361
+#define signature_TM1_edx	0x74656d73
+
+#define signature_TM2_ebx	0x756e6547
+#define signature_TM2_ecx	0x3638784d
+#define signature_TM2_edx	0x54656e69
+
+#define signature_NSC_ebx	0x646f6547
+#define signature_NSC_ecx	0x43534e20
+#define signature_NSC_edx	0x79622065
+
+#define signature_NEXGEN_ebx	0x4778654e
+#define signature_NEXGEN_ecx	0x6e657669
+#define signature_NEXGEN_edx	0x72446e65
+
+#define signature_RISE_ebx	0x65736952
+#define signature_RISE_ecx	0x65736952
+#define signature_RISE_edx	0x65736952
+
+#define signature_SIS_ebx	0x20536953
+#define signature_SIS_ecx	0x20536953
+#define signature_SIS_edx	0x20536953
+
+#define signature_UMC_ebx	0x20434d55
+#define signature_UMC_ecx	0x20434d55
+#define signature_UMC_edx	0x20434d55
+
+#define signature_VIA_ebx	0x20414956
+#define signature_VIA_ecx	0x20414956
+#define signature_VIA_edx	0x20414956
+
+#define signature_VORTEX_ebx	0x74726f56
+#define signature_VORTEX_ecx	0x436f5320
+#define signature_VORTEX_edx	0x36387865
 
 #if defined(__i386__) && defined(__PIC__)
 /* %ebx may be the PIC register.  */
 #if __GNUC__ >= 3
 #define __cpuid(level, a, b, c, d)			\
-  __asm__ ("xchg{l}\t{%%}ebx, %1\n\t"			\
+  __asm__ ("xchg{l}\t{%%}ebx, %k1\n\t"			\
 	   "cpuid\n\t"					\
-	   "xchg{l}\t{%%}ebx, %1\n\t"			\
-	   : "=a" (a), "=r" (b), "=c" (c), "=d" (d)	\
+	   "xchg{l}\t{%%}ebx, %k1\n\t"			\
+	   : "=a" (a), "=&r" (b), "=c" (c), "=d" (d)	\
 	   : "0" (level))
 
 #define __cpuid_count(level, count, a, b, c, d)		\
-  __asm__ ("xchg{l}\t{%%}ebx, %1\n\t"			\
+  __asm__ ("xchg{l}\t{%%}ebx, %k1\n\t"			\
 	   "cpuid\n\t"					\
-	   "xchg{l}\t{%%}ebx, %1\n\t"			\
-	   : "=a" (a), "=r" (b), "=c" (c), "=d" (d)	\
+	   "xchg{l}\t{%%}ebx, %k1\n\t"			\
+	   : "=a" (a), "=&r" (b), "=c" (c), "=d" (d)	\
 	   : "0" (level), "2" (count))
 #else
 /* Host GCCs older than 3.0 weren't supporting Intel asm syntax
    nor alternatives in i386 code.  */
 #define __cpuid(level, a, b, c, d)			\
-  __asm__ ("xchgl\t%%ebx, %1\n\t"			\
+  __asm__ ("xchgl\t%%ebx, %k1\n\t"			\
 	   "cpuid\n\t"					\
-	   "xchgl\t%%ebx, %1\n\t"			\
-	   : "=a" (a), "=r" (b), "=c" (c), "=d" (d)	\
+	   "xchgl\t%%ebx, %k1\n\t"			\
+	   : "=a" (a), "=&r" (b), "=c" (c), "=d" (d)	\
 	   : "0" (level))
 
 #define __cpuid_count(level, count, a, b, c, d)		\
-  __asm__ ("xchgl\t%%ebx, %1\n\t"			\
+  __asm__ ("xchgl\t%%ebx, %k1\n\t"			\
 	   "cpuid\n\t"					\
-	   "xchgl\t%%ebx, %1\n\t"			\
-	   : "=a" (a), "=r" (b), "=c" (c), "=d" (d)	\
+	   "xchgl\t%%ebx, %k1\n\t"			\
+	   : "=a" (a), "=&r" (b), "=c" (c), "=d" (d)	\
 	   : "0" (level), "2" (count))
 #endif
+#elif defined(__x86_64__) && (defined(__code_model_medium__) || defined(__code_model_large__)) && defined(__PIC__)
+/* %rbx may be the PIC register.  */
+#define __cpuid(level, a, b, c, d)			\
+  __asm__ ("xchg{q}\t{%%}rbx, %q1\n\t"			\
+	   "cpuid\n\t"					\
+	   "xchg{q}\t{%%}rbx, %q1\n\t"			\
+	   : "=a" (a), "=&r" (b), "=c" (c), "=d" (d)	\
+	   : "0" (level))
+
+#define __cpuid_count(level, count, a, b, c, d)		\
+  __asm__ ("xchg{q}\t{%%}rbx, %q1\n\t"			\
+	   "cpuid\n\t"					\
+	   "xchg{q}\t{%%}rbx, %q1\n\t"			\
+	   : "=a" (a), "=&r" (b), "=c" (c), "=d" (d)	\
+	   : "0" (level), "2" (count))
 #else
 #define __cpuid(level, a, b, c, d)			\
   __asm__ ("cpuid\n\t"					\
@@ -118,9 +207,9 @@ __get_cpuid_max (unsigned int __ext, unsigned int *__sig)
 {
   unsigned int __eax, __ebx, __ecx, __edx;
 
-#ifndef __x86_64__
-#if __GNUC__ >= 3
+#ifdef __i386__
   /* See if we can use cpuid.  On AMD64 we always can.  */
+#if __GNUC__ >= 3
   __asm__ ("pushf{l|d}\n\t"
 	   "pushf{l|d}\n\t"
 	   "pop{l}\t%0\n\t"
@@ -181,20 +270,3 @@ __get_cpuid (unsigned int __level,
   __cpuid (__level, *__eax, *__ebx, *__ecx, *__edx);
   return 1;
 }
-
-#ifndef NOINLINE
-#define NOINLINE __attribute__ ((noinline))
-#endif
-
-unsigned int i386_cpuid (void) NOINLINE;
-
-unsigned int NOINLINE
-i386_cpuid (void)
-{
-  unsigned int eax, ebx, ecx, edx;
-
-  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
-    return 0;
-
-  return edx;
-}
diff --git a/gdb/common/linux-btrace.c b/gdb/common/linux-btrace.c
index d3c8705..6804e55 100644
--- a/gdb/common/linux-btrace.c
+++ b/gdb/common/linux-btrace.c
@@ -30,6 +30,7 @@
 #include "gdb_assert.h"
 #include "regcache.h"
 #include "gdbthread.h"
+#include "i386-cpuid.h"
 
 #if HAVE_LINUX_PERF_EVENT_H
 
@@ -339,13 +340,10 @@ kernel_supports_btrace (void)
 static int
 intel_supports_btrace (void)
 {
-#if defined __i386__ || defined __x86_64__
   unsigned int cpuid, model, family;
 
-  __asm__ __volatile__ ("movl   $1, %%eax;"
-			"cpuid;"
-			: "=a" (cpuid)
-			:: "%ebx", "%ecx", "%edx");
+  if (!i386_cpuid (1, &cpuid, NULL, NULL, NULL))
+    return 0;
 
   family = (cpuid >> 8) & 0xf;
   model = (cpuid >> 4) & 0xf;
@@ -376,12 +374,6 @@ intel_supports_btrace (void)
     }
 
   return 1;
-
-#else /* !defined __i386__ && !defined __x86_64__ */
-
-  return 0;
-
-#endif /* !defined __i386__ && !defined __x86_64__ */
 }
 
 /* Check whether the cpu supports branch tracing.  */
@@ -389,22 +381,15 @@ intel_supports_btrace (void)
 static int
 cpu_supports_btrace (void)
 {
-#if defined __i386__ || defined __x86_64__
+  unsigned int ebx, ecx, edx;
   char vendor[13];
 
-  __asm__ __volatile__ ("xorl   %%ebx, %%ebx;"
-			"xorl   %%ecx, %%ecx;"
-			"xorl   %%edx, %%edx;"
-			"movl   $0,    %%eax;"
-			"cpuid;"
-			"movl   %%ebx,  %0;"
-			"movl   %%edx,  %1;"
-			"movl   %%ecx,  %2;"
-			: "=m" (vendor[0]),
-			  "=m" (vendor[4]),
-			  "=m" (vendor[8])
-			:
-			: "%eax", "%ebx", "%ecx", "%edx");
+  if (!i386_cpuid (0, NULL, &ebx, &ecx, &edx))
+    return 0;
+
+  memcpy (&vendor[0], &ebx, 4);
+  memcpy (&vendor[4], &ecx, 4);
+  memcpy (&vendor[8], &edx, 4);
   vendor[12] = '\0';
 
   if (strcmp (vendor, "GenuineIntel") == 0)
@@ -412,12 +397,6 @@ cpu_supports_btrace (void)
 
   /* Don't know about others.  Let's assume they do.  */
   return 1;
-
-#else /* !defined __i386__ && !defined __x86_64__ */
-
-  return 0;
-
-#endif /* !defined __i386__ && !defined __x86_64__ */
 }
 
 /* See linux-btrace.h.  */
diff --git a/gdb/go32-nat.c b/gdb/go32-nat.c
index ea6d1ca..0471d8f 100644
--- a/gdb/go32-nat.c
+++ b/gdb/go32-nat.c
@@ -96,6 +96,7 @@
 #include "buildsym.h"
 #include "i387-tdep.h"
 #include "i386-tdep.h"
+#include "i386-cpuid.h"
 #include "value.h"
 #include "regcache.h"
 #include "gdb_string.h"
@@ -1141,6 +1142,21 @@ go32_sysinfo (char *arg, int from_tty)
   else if (u.machine[0] == 'i' && u.machine[1] > 4)
     {
       /* CPUID with EAX = 0 returns the Vendor ID.  */
+#if 0
+      /* Ideally we would use i386_cpuid(), but it needs someone to run
+         native tests first to make sure things actually work.  They should.
+         http://sourceware.org/ml/gdb-patches/2013-05/msg00164.html  */
+      unsigned int eax, ebx, ecx, edx;
+
+      if (i386_cpuid (0, &eax, &ebx, &ecx, &edx))
+	{
+	  cpuid_max = eax;
+	  memcpy (&vendor[0], &ebx, 4);
+	  memcpy (&vendor[4], &ecx, 4);
+	  memcpy (&vendor[8], &edx, 4);
+	  cpuid_vendor[12] = '\0';
+	}
+#else
       __asm__ __volatile__ ("xorl   %%ebx, %%ebx;"
 			    "xorl   %%ecx, %%ecx;"
 			    "xorl   %%edx, %%edx;"
@@ -1157,6 +1173,7 @@ go32_sysinfo (char *arg, int from_tty)
 			    :
 			    : "%eax", "%ebx", "%ecx", "%edx");
       cpuid_vendor[12] = '\0';
+#endif
     }
 
   printf_filtered ("CPU Type.......................%s", u.machine);
@@ -1182,6 +1199,10 @@ go32_sysinfo (char *arg, int from_tty)
       int amd_p = strcmp (cpuid_vendor, "AuthenticAMD") == 0;
       unsigned cpu_family, cpu_model;
 
+#if 0
+      /* See comment above about cpuid usage.  */
+      i386_cpuid (1, &cpuid_eax, &cpuid_ebx, NULL, &cpuid_edx);
+#else
       __asm__ __volatile__ ("movl   $1, %%eax;"
 			    "cpuid;"
 			    : "=a" (cpuid_eax),
@@ -1189,6 +1210,7 @@ go32_sysinfo (char *arg, int from_tty)
 			      "=d" (cpuid_edx)
 			    :
 			    : "%ecx");
+#endif
       brand_idx = cpuid_ebx & 0xff;
       cpu_family = (cpuid_eax >> 8) & 0xf;
       cpu_model  = (cpuid_eax >> 4) & 0xf;
diff --git a/gdb/testsuite/gdb.arch/i386-avx.c b/gdb/testsuite/gdb.arch/i386-avx.c
index bcfa18f..7fd1217 100644
--- a/gdb/testsuite/gdb.arch/i386-avx.c
+++ b/gdb/testsuite/gdb.arch/i386-avx.c
@@ -53,7 +53,7 @@ have_avx (void)
 {
   unsigned int eax, ebx, ecx, edx;
 
-  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
+  if (!i386_cpuid (1, &eax, &ebx, &ecx, &edx))
     return 0;
 
   if ((ecx & (bit_AVX | bit_OSXSAVE)) == (bit_AVX | bit_OSXSAVE))
diff --git a/gdb/testsuite/gdb.arch/i386-avx.exp b/gdb/testsuite/gdb.arch/i386-avx.exp
index 964806c..bbbc6f4 100644
--- a/gdb/testsuite/gdb.arch/i386-avx.exp
+++ b/gdb/testsuite/gdb.arch/i386-avx.exp
@@ -34,7 +34,7 @@ if [get_compiler_info] {
 
 set additional_flags ""
 if [test_compiler_info gcc*] {
-    set additional_flags "additional_flags=-mavx"
+    set additional_flags "additional_flags=-mavx -I${srcdir}/../common"
 }
 
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug $additional_flags]] != "" } {
diff --git a/gdb/testsuite/gdb.arch/i386-sse.c b/gdb/testsuite/gdb.arch/i386-sse.c
index d431527..8617c8a 100644
--- a/gdb/testsuite/gdb.arch/i386-sse.c
+++ b/gdb/testsuite/gdb.arch/i386-sse.c
@@ -51,7 +51,10 @@ v4sf_t data[] =
 int
 have_sse (void)
 {
-  int edx = i386_cpuid ();
+  int edx;
+
+  if (!i386_cpuid (1, NULL, NULL, NULL, &edx))
+    return 0;
 
   if (edx & bit_SSE)
     return 1;
diff --git a/gdb/testsuite/gdb.arch/i386-sse.exp b/gdb/testsuite/gdb.arch/i386-sse.exp
index 5923eca..c62a3a0 100644
--- a/gdb/testsuite/gdb.arch/i386-sse.exp
+++ b/gdb/testsuite/gdb.arch/i386-sse.exp
@@ -34,7 +34,7 @@ if [get_compiler_info] {
 
 set additional_flags ""
 if [test_compiler_info gcc*] {
-    set additional_flags "additional_flags=-msse"
+    set additional_flags "additional_flags=-msse -I${srcdir}/../common"
 }
 
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug $additional_flags]] != "" } {
-- 
1.8.2.1

Re: [PATCH v2] gdb: clean up x86 cpuid implementations

[Pedro Alves]

On 05/07/2013 04:05 PM, Mike Frysinger wrote:
> On Tuesday 07 May 2013 10:49:29 Pedro Alves wrote:
>> On 05/07/2013 03:31 PM, Mike Frysinger wrote:
>>> On Tuesday 07 May 2013 10:19:06 Pedro Alves wrote:
>>>> On 05/07/2013 03:08 PM, Pedro Alves wrote:
>>>>> On 05/07/2013 02:29 PM, Mike Frysinger wrote:
>>>>>> Fortunately, that last header there is pretty damn good -- it handles
>>>>>> lots of edge cases, the code is nice & tight (uses gcc asm operands
>>>>>> rather than manual movs), and is already almost a general library type
>>>>>> header.
>>>>>
>>>>> The top of the header says:
>>>>>
>>>>> /* Helper file for i386 platform.  Runtime check for MMX/SSE/SSE2/AVX
>>>>>
>>>>>  * support. Copied from gcc 4.4.
>>>>>
>>>>> I'd rather not fork the gcc file.  If we need to wrap its
>>>>> functions/macros for gdb's purpose, I'd rather do that in a separate
>>>>> file that
>>>>> #includes (a copy of) gcc's, verbatim, so we can pull updates from
>>>>> upstream easily.  In fact, diffing our copy against gcc's shows we're
>>>>> already out of date --- see below.  The bits removed are gdb-specific
>>>>> additions.
>>>>>
>>>>> I wonder whether pushing the file down to libiberty, so both gcc
>>>>> and gdb could share it would be viable?
>>>>
>>>> Actually, it seems like __get_cpuid is a gcc built-in nowadays, but I
>>>> don't when it was added.  We could make use of it, and only fallback to
>>>> the header copy if the host compiler doesn't have the builtin.
>>>
>>> yes, gcc introduced a cpuid.h starting with gcc-4.3.0.  i wanted to focus
>>> on getting everyone on the same header first before tackling that.
>>
>> Your changes were effectively diverging our header from gcc's, not
>> converging.
> 
> the header provides a simple API that sits between the gcc cpuid.h and gdb and 
> provides a simpler interface (usable for all arches, and arguments are 
> optional).  

Yes, and I'm pointing out that the header is 99% a _modified_ copy
of gcc's cpuid.h, which makes updating the file from upstream
harder than necessary.  Do the simpler interface in a separate file,
that consumes gcc's interface, is all I'm saying.

> what happens internally (include gcc cpuid.h or some copy or 
> whatever) is irrelevant to the external consumers.

Of course.  That's really a straw man, given my point is about
simplifying maintenance of the API's internals, not the API
itself.

>>> i didn't think  people would be ok with x86 builds requiring gcc-4.3.0 ?
>>
>> Right, and I did not suggest that?
> 
> i didn't say you did.  i was asking a question to see if we could avoid having 
> a copy at all.

Maybe that's because I'm not a native speaker, but "would people be ok?"
would a variant of that question that didn't sound like implying I
had said it, to me.

-- 
Pedro Alves

Re: [patch/rfc] gdb: clean up x86 cpuid implementations

[Eli Zaretskii]

> Date: Mon, 6 May 2013 21:26:40 -0700
> From: Doug Evans <dje@google.com>
> Cc: Mike Frysinger <vapier@gentoo.org>, gdb-patches <gdb-patches@sourceware.org>
> 
> On Mon, May 6, 2013 at 7:41 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> >> From: Mike Frysinger <vapier@gentoo.org>
> >> but in the end, i don't care about dead things like DOS, so if people prefer
> >> to leave the cruft in that one file, i can look the other way while still
> >> cleaning up everything else.
> >
> > Yes, please.
> 
> I don't have an opinion on which way to go,
> but if we leave DOS alone,
> can we add a comment explaining why things are the way they are,
> so that the next person won't have to dig through archives.

Certainly.  Will do once I see what is actually committed.

Re: [PATCH v3] gdb: clean up x86 cpuid implementations

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 15 bytes --]

ping ...
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3] gdb: clean up x86 cpuid implementations

[Pedro Alves]

On 06/17/2013 07:05 AM, Mike Frysinger wrote:
> ping ...

... pong.

ENOCHANGELOG

OK.

-- 
Pedro Alves

Re: [PATCH v3] gdb: clean up x86 cpuid implementations

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 355 bytes --]

On Monday 17 June 2013 13:40:52 Pedro Alves wrote:
> On 06/17/2013 07:05 AM, Mike Frysinger wrote:
> > ping ...
> 
> ... pong.
> 
> ENOCHANGELOG

i usually wait for the code to get reviewed before i waste time autogenerating 
the ChangeLog entries.  if people are happy with the code now, i can move on 
to the last part before merging.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3] gdb: clean up x86 cpuid implementations

[Pedro Alves]

On 06/18/2013 06:52 PM, Mike Frysinger wrote:
> On Monday 17 June 2013 13:40:52 Pedro Alves wrote:
>> On 06/17/2013 07:05 AM, Mike Frysinger wrote:
>>> ping ...
>>
>> ... pong.
>>
>> ENOCHANGELOG
> 
> i usually wait for the code to get reviewed before i waste time autogenerating 
> the ChangeLog entries.  if people are happy with the code now, i can move on 
> to the last part before merging.

Well, that's not the project's policy.  ChangeLogs are supposed to
be reviewed too.

But let's stop this before it gets into a ChangeLog-are-useless
argument...  I know you write good ChangeLog entries.  ;-)

-- 
Pedro Alves

Re: [PATCH v3] gdb: clean up x86 cpuid implementations

[Joel Brobecker]

> Well, that's not the project's policy.  ChangeLogs are supposed to
> be reviewed too.

And also sometimes ChangeLogs help review the change itself as well.

-- 
Joel

Re: [PATCH v3] gdb: clean up x86 cpuid implementations

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 2011 bytes --]

On Tuesday 18 June 2013 14:19:15 Pedro Alves wrote:
> On 06/18/2013 06:52 PM, Mike Frysinger wrote:
> > On Monday 17 June 2013 13:40:52 Pedro Alves wrote:
> >> On 06/17/2013 07:05 AM, Mike Frysinger wrote:
> >>> ping ...
> >> 
> >> ... pong.
> >> 
> >> ENOCHANGELOG
> > 
> > i usually wait for the code to get reviewed before i waste time
> > autogenerating the ChangeLog entries.  if people are happy with the code
> > now, i can move on to the last part before merging.
> 
> Well, that's not the project's policy.  ChangeLogs are supposed to
> be reviewed too.
> 
> But let's stop this before it gets into a ChangeLog-are-useless
> argument...  I know you write good ChangeLog entries.  ;-)

i'm not trying to argue against ChangeLogs (i know it's a lost cause w/gdb).  
just pointing out that i prefer to wait to the last minute to write it since 
it has no real bearing on the actual code review.

here's the ChangeLog entries for this patch.
gdb/:
2013-06-18  Mike Frysinger  <vapier@gentoo.org>

	* Makefile.in (HFILES_NO_SRCDIR): Add common/i386-cpuid.h and
	common/i386-gcc-cpuid.h.
	* common/i386-cpuid.h: New wrapper header around i386-gcc-cpuid.h.
	* common/i386-gcc-cpuid.h: Rename from testsuite/gdb.arch/i386-cpuid.h.
	Copy the latest version from upstream gcc.
	* common/linux-btrace.c: Include i386-cpuid.h.
	(intel_supports_btrace): Delete x86 ifdefs and replace inline asm with
	call to i386_cpuid.
	(cpu_supports_btrace): Likewise.
	* go32-nat.c: Include i386-cpuid.h.
	(go32_sysinfo): Add (disabled) calls to i386_cpuid with comments.

gdb/testsuite/:
2013-06-18  Mike Frysinger  <vapier@gentoo.org>

	* gdb.arch/i386-avx.c (have_avx): Change __get_cpuid call to i386_cpuid.
	* gdb.arch/i386-avx.exp (additional_flags): Add -I${srcdir}/../common.
	* gdb.arch/i386-cpuid.h: Moved to ../common/i386-gcc-cpuid.h.
	* gdb.arch/i386-sse.c: Call new i386_cpuid function.
	* gdb.arch/i386-see.exp (additional_flags): Add -I${srcdir}/../common.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3] gdb: clean up x86 cpuid implementations

[Pedro Alves]

On 06/19/2013 03:46 AM, Mike Frysinger wrote:

> i'm not trying to argue against ChangeLogs (i know it's a lost cause w/gdb).  
> just pointing out that i prefer to wait to the last minute to write it since 
> it has no real bearing on the actual code review.

It does have real bearing on the code review.

> here's the ChangeLog entries for this patch.
> gdb/:
> 2013-06-18  Mike Frysinger  <vapier@gentoo.org>
> 
> 	* Makefile.in (HFILES_NO_SRCDIR): Add common/i386-cpuid.h and
> 	common/i386-gcc-cpuid.h.

For example, from the diff alone, one can't tell where these were
being added.

> 	* common/i386-cpuid.h: New wrapper header around i386-gcc-cpuid.h.
> 	* common/i386-gcc-cpuid.h: Rename from testsuite/gdb.arch/i386-cpuid.h.
> 	Copy the latest version from upstream gcc.

And this is the only mention in the whole patch+intro of the importing
from the latest version from upstream gcc.  ;-)

> 	* common/linux-btrace.c: Include i386-cpuid.h.
> 	(intel_supports_btrace): Delete x86 ifdefs and replace inline asm with
> 	call to i386_cpuid.
> 	(cpu_supports_btrace): Likewise.
> 	* go32-nat.c: Include i386-cpuid.h.
> 	(go32_sysinfo): Add (disabled) calls to i386_cpuid with comments.
> 
> gdb/testsuite/:
> 2013-06-18  Mike Frysinger  <vapier@gentoo.org>
> 
> 	* gdb.arch/i386-avx.c (have_avx): Change __get_cpuid call to i386_cpuid.
> 	* gdb.arch/i386-avx.exp (additional_flags): Add -I${srcdir}/../common.
> 	* gdb.arch/i386-cpuid.h: Moved to ../common/i386-gcc-cpuid.h.
> 	* gdb.arch/i386-sse.c: Call new i386_cpuid function.
> 	* gdb.arch/i386-see.exp (additional_flags): Add -I${srcdir}/../common.

FAOD, this is OK.

However, BTW, I failed to notice this before, but:

> +++ b/gdb/common/i386-cpuid.h
> @@ -0,0 +1,68 @@
> +/*
> + * Copyright (C) 2007-2013 Free Software Foundation, Inc.
> + *
> + * This file is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 3, or (at your option) any
> + * later version.
> + *
> + * This file is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * Under Section 7 of GPL version 3, you are granted additional
> + * permissions described in the GCC Runtime Library Exception, version
> + * 3.1, as published by the Free Software Foundation.
> + *
> + * You should have received a copy of the GNU General Public License and
> + * a copy of the GCC Runtime Library Exception along with this program;
> + * see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
> + * <http://www.gnu.org/licenses/>.
> + */

This header only contains gdb bits.  I don't see a reason for making
it GPL w/ runtime exception, rather than regular GPLv3+.
Was it just a copy/paste?

Thanks,
-- 
Pedro Alves

Re: [PATCH v3] gdb: clean up x86 cpuid implementations

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 1696 bytes --]

On Wednesday 19 June 2013 07:43:42 Pedro Alves wrote:
> On 06/19/2013 03:46 AM, Mike Frysinger wrote:
> However, BTW, I failed to notice this before, but:
> > +++ b/gdb/common/i386-cpuid.h
> > @@ -0,0 +1,68 @@
> > +/*
> > + * Copyright (C) 2007-2013 Free Software Foundation, Inc.
> > + *
> > + * This file is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License as published by the
> > + * Free Software Foundation; either version 3, or (at your option) any
> > + * later version.
> > + *
> > + * This file is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * General Public License for more details.
> > + *
> > + * Under Section 7 of GPL version 3, you are granted additional
> > + * permissions described in the GCC Runtime Library Exception, version
> > + * 3.1, as published by the Free Software Foundation.
> > + *
> > + * You should have received a copy of the GNU General Public License and
> > + * a copy of the GCC Runtime Library Exception along with this program;
> > + * see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
> > + * <http://www.gnu.org/licenses/>.
> > + */
> 
> This header only contains gdb bits.  I don't see a reason for making
> it GPL w/ runtime exception, rather than regular GPLv3+.
> Was it just a copy/paste?

from the top of the file:
 /*
 * Helper cpuid.h file copied from gcc-4.8.0.  Code in gdb should not
 * include this directly, but pull in i386-cpuid.h and use that func.
 */
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3] gdb: clean up x86 cpuid implementations

[Pedro Alves]

On 06/19/2013 03:53 PM, Mike Frysinger wrote:
> On Wednesday 19 June 2013 07:43:42 Pedro Alves wrote:
>> On 06/19/2013 03:46 AM, Mike Frysinger wrote:
>> However, BTW, I failed to notice this before, but:
>>> +++ b/gdb/common/i386-cpuid.h
>>> @@ -0,0 +1,68 @@
>>> +/*
>>> + * Copyright (C) 2007-2013 Free Software Foundation, Inc.
>>> + *
>>> + * This file is free software; you can redistribute it and/or modify it
>>> + * under the terms of the GNU General Public License as published by the
>>> + * Free Software Foundation; either version 3, or (at your option) any
>>> + * later version.
>>> + *
>>> + * This file is distributed in the hope that it will be useful, but
>>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> + * General Public License for more details.
>>> + *
>>> + * Under Section 7 of GPL version 3, you are granted additional
>>> + * permissions described in the GCC Runtime Library Exception, version
>>> + * 3.1, as published by the Free Software Foundation.
>>> + *
>>> + * You should have received a copy of the GNU General Public License and
>>> + * a copy of the GCC Runtime Library Exception along with this program;
>>> + * see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
>>> + * <http://www.gnu.org/licenses/>.
>>> + */
>>
>> This header only contains gdb bits.  I don't see a reason for making
>> it GPL w/ runtime exception, rather than regular GPLv3+.
>> Was it just a copy/paste?
> 
> from the top of the file:
>  /*
>  * Helper cpuid.h file copied from gcc-4.8.0.  Code in gdb should not
>  * include this directly, but pull in i386-cpuid.h and use that func.
>  */

That's the header of i386-gcc-cpuid.h.  But I'm talking about (and have
quoted) i386-cpuid.h, the wrapper.

-- 
Pedro Alves

Re: [PATCH v3] gdb: clean up x86 cpuid implementations

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 2367 bytes --]

On Wednesday 19 June 2013 11:05:57 Pedro Alves wrote:
> On 06/19/2013 03:53 PM, Mike Frysinger wrote:
> > On Wednesday 19 June 2013 07:43:42 Pedro Alves wrote:
> >> On 06/19/2013 03:46 AM, Mike Frysinger wrote:
> >> 
> >> However, BTW, I failed to notice this before, but:
> >>> +++ b/gdb/common/i386-cpuid.h
> >>> @@ -0,0 +1,68 @@
> >>> +/*
> >>> + * Copyright (C) 2007-2013 Free Software Foundation, Inc.
> >>> + *
> >>> + * This file is free software; you can redistribute it and/or modify
> >>> it + * under the terms of the GNU General Public License as published
> >>> by the + * Free Software Foundation; either version 3, or (at your
> >>> option) any + * later version.
> >>> + *
> >>> + * This file is distributed in the hope that it will be useful, but
> >>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> >>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> >>> + * General Public License for more details.
> >>> + *
> >>> + * Under Section 7 of GPL version 3, you are granted additional
> >>> + * permissions described in the GCC Runtime Library Exception, version
> >>> + * 3.1, as published by the Free Software Foundation.
> >>> + *
> >>> + * You should have received a copy of the GNU General Public License
> >>> and + * a copy of the GCC Runtime Library Exception along with this
> >>> program; + * see the files COPYING3 and COPYING.RUNTIME respectively. 
> >>> If not, see + * <http://www.gnu.org/licenses/>.
> >>> + */
> >> 
> >> This header only contains gdb bits.  I don't see a reason for making
> >> it GPL w/ runtime exception, rather than regular GPLv3+.
> >> Was it just a copy/paste?
> > 
> > from the top of the file:
> >  /*
> >  * Helper cpuid.h file copied from gcc-4.8.0.  Code in gdb should not
> >  * include this directly, but pull in i386-cpuid.h and use that func.
> >  */
> 
> That's the header of i386-gcc-cpuid.h.  But I'm talking about (and have
> quoted) i386-cpuid.h, the wrapper.

too many cpuids :p

technically, i386-cpuid.h was spawned from the gdb/testsuite/gdb.arch/i386-
cpuid.h which itself was spawned from the gcc sources (and both include the 
exception) which is why the header was retained.

then again, FSF owns both, so if we want to drop that, it shouldn't be an 
issue.  doesn't matter to me either way.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3] gdb: clean up x86 cpuid implementations

[Pedro Alves]

On 06/19/2013 04:20 PM, Mike Frysinger wrote:

> technically, i386-cpuid.h was spawned from the gdb/testsuite/gdb.arch/i386-
> cpuid.h which itself was spawned from the gcc sources (and both include the 
> exception) which is why the header was retained.
> then again, FSF owns both, so if we want to drop that, it shouldn't be an 
> issue.  doesn't matter to me either way.

I'd rather make the GDB header plain GPLv3+.  I find it simpler and least
surprising if the header with the exception is only the verbatim(-ish) gcc
imported header.  Later on, if/when we want to move/add code to/from the
GDB-specific header, we won't have to bother with this detail again.

Thanks,
-- 
Pedro Alves

[PATCH v4] gdb: clean up x86 cpuid implementations

[Mike Frysinger]

We've currently got 3 files doing open coded implementations of cpuid.
Each has its own set of workarounds and varying levels of how well
they're written and are generally hardcoded to specific cpuid functions.
If you try to build the latest gdb as a PIE on an i386 system, the build
will fail because one of them lacks PIC workarounds (wrt ebx).

Specifically, we have:
common/linux-btrace.c:
	two copies of cpuid asm w/specific args, one has no workarounds
	while the other implicitly does to avoid memcpy
go32-nat.c:
	two copies of cpuid asm w/specific args, one has workarounds to
	avoid memcpy
gdb/testsuite/gdb.arch/i386-cpuid.h:
	one general cpuid asm w/many workarounds copied from older gcc

Fortunately, that last header there is pretty damn good -- it handles
lots of edge cases, the code is nice & tight (uses gcc asm operands
rather than manual movs), and is already almost a general library type
header.  It's also the basis of what is now the public cpuid.h that is
shipped with gcc-4.3+.

So what I've done is pull that test header out and into gdb/common/
(not sure if there's a better place), synced to the version found in
gcc-4.8.0, put a wrapper API around it, and then cut over all the
existing call points to this new header.

Since the func already has support for "is cpuid supported on this proc",
it makes it trivial to push the i386/x86_64 ifdefs down into this wrapper
API too.  Now it can be safely used for all targets and gcc will elide
the unused code for us.

I've verified the gdb.arch testsuite still passes, and this code compiles
for an armv7a host as well as x86_64.  The go32-nat code has been left
ifdef-ed out until someone can test & verify the new stuff works (and if
it doesn't, figure out how to make the new code work).

Signed-off-by: Mike Frysinger <vapier@gentoo.org>

gdb/:
2013-06-18  Mike Frysinger  <vapier@gentoo.org>

	* Makefile.in (HFILES_NO_SRCDIR): Add common/i386-cpuid.h and
	common/i386-gcc-cpuid.h.
	* common/i386-cpuid.h: New wrapper header around i386-gcc-cpuid.h.
	* common/i386-gcc-cpuid.h: Rename from testsuite/gdb.arch/i386-cpuid.h.
	Copy the latest version from upstream gcc.
	* common/linux-btrace.c: Include i386-cpuid.h.
	(intel_supports_btrace): Delete x86 ifdefs and replace inline asm with
	call to i386_cpuid.
	(cpu_supports_btrace): Likewise.
	* go32-nat.c: Include i386-cpuid.h.
	(go32_sysinfo): Add (disabled) calls to i386_cpuid with comments.

gdb/testsuite/:
2013-06-18  Mike Frysinger  <vapier@gentoo.org>

	* gdb.arch/i386-avx.c (have_avx): Change __get_cpuid call to i386_cpuid.
	* gdb.arch/i386-avx.exp (additional_flags): Add -I${srcdir}/../common.
	* gdb.arch/i386-cpuid.h: Moved to ../common/i386-gcc-cpuid.h.
	* gdb.arch/i386-sse.c: Call new i386_cpuid function.
	* gdb.arch/i386-see.exp (additional_flags): Add -I${srcdir}/../common.
---
v4
	- adjust license in i386-cpuid.h

 gdb/Makefile.in                                    |   2 +-
 gdb/common/i386-cpuid.h                            |  63 ++++++++++
 .../i386-cpuid.h => common/i386-gcc-cpuid.h}       | 140 ++++++++++++++++-----
 gdb/common/linux-btrace.c                          |  41 ++----
 gdb/go32-nat.c                                     |  22 ++++
 gdb/testsuite/gdb.arch/i386-avx.c                  |   2 +-
 gdb/testsuite/gdb.arch/i386-avx.exp                |   2 +-
 gdb/testsuite/gdb.arch/i386-sse.c                  |   5 +-
 gdb/testsuite/gdb.arch/i386-sse.exp                |   2 +-
 9 files changed, 209 insertions(+), 70 deletions(-)
 create mode 100644 gdb/common/i386-cpuid.h
 rename gdb/{testsuite/gdb.arch/i386-cpuid.h => common/i386-gcc-cpuid.h} (59%)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index a6336a2..71058e5 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -850,7 +850,7 @@ common/common-utils.h common/xml-utils.h common/buffer.h common/ptid.h \
 common/format.h common/host-defs.h utils.h common/queue.h common/gdb_string.h \
 common/linux-osdata.h gdb-dlfcn.h auto-load.h probe.h stap-probe.h \
 gdb_bfd.h sparc-ravenscar-thread.h ppc-ravenscar-thread.h common/linux-btrace.h \
-ctf.h
+ctf.h common/i386-cpuid.h common/i386-gcc-cpuid.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
diff --git a/gdb/common/i386-cpuid.h b/gdb/common/i386-cpuid.h
new file mode 100644
index 0000000..8bb28c5
--- /dev/null
+++ b/gdb/common/i386-cpuid.h
@@ -0,0 +1,63 @@
+/* C API for x86 cpuid insn.
+   Copyright (C) 2007-2013 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This file is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by the
+   Free Software Foundation; either version 3, or (at your option) any
+   later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef I386_CPUID_COMMON_H
+#define I386_CPUID_COMMON_H
+
+/* Always include the header for the cpu bit defines.  */
+#include "i386-gcc-cpuid.h"
+
+#if defined(__i386__) || defined(__x86_64__)
+
+/* Return cpuid data for requested cpuid level, as found in returned
+   eax, ebx, ecx and edx registers.  The function checks if cpuid is
+   supported and returns 1 for valid cpuid information or 0 for
+   unsupported cpuid level.  Pointers may be non-null.  */
+
+static __inline int
+i386_cpuid (unsigned int __level,
+	    unsigned int *__eax, unsigned int *__ebx,
+	    unsigned int *__ecx, unsigned int *__edx)
+{
+  unsigned int __scratch;
+
+  if (!__eax)
+    __eax = &__scratch;
+  if (!__ebx)
+    __ebx = &__scratch;
+  if (!__ecx)
+    __ecx = &__scratch;
+  if (!__edx)
+    __edx = &__scratch;
+
+  return __get_cpuid (__level, __eax, __ebx, __ecx, __edx);
+}
+
+#else
+
+static __inline int
+i386_cpuid (unsigned int __level,
+	    unsigned int *__eax, unsigned int *__ebx,
+	    unsigned int *__ecx, unsigned int *__edx)
+{
+  return 0;
+}
+
+#endif /* i386 && x86_64 */
+
+#endif /* I386_CPUID_COMMON_H */
diff --git a/gdb/testsuite/gdb.arch/i386-cpuid.h b/gdb/common/i386-gcc-cpuid.h
similarity index 59%
rename from gdb/testsuite/gdb.arch/i386-cpuid.h
rename to gdb/common/i386-gcc-cpuid.h
index 084a083..e045ba8 100644
--- a/gdb/testsuite/gdb.arch/i386-cpuid.h
+++ b/gdb/common/i386-gcc-cpuid.h
@@ -1,6 +1,8 @@
-/* Helper file for i386 platform.  Runtime check for MMX/SSE/SSE2/AVX
- * support. Copied from gcc 4.4.
- *
+/*
+ * Helper cpuid.h file copied from gcc-4.8.0.  Code in gdb should not
+ * include this directly, but pull in i386-cpuid.h and use that func.
+ */
+/*
  * Copyright (C) 2007-2013 Free Software Foundation, Inc.
  *
  * This file is free software; you can redistribute it and/or modify it
@@ -26,6 +28,7 @@
 /* %ecx */
 #define bit_SSE3	(1 << 0)
 #define bit_PCLMUL	(1 << 1)
+#define bit_LZCNT	(1 << 5)
 #define bit_SSSE3	(1 << 9)
 #define bit_FMA		(1 << 12)
 #define bit_CMPXCHG16B	(1 << 13)
@@ -37,6 +40,8 @@
 #define bit_XSAVE	(1 << 26)
 #define bit_OSXSAVE	(1 << 27)
 #define bit_AVX		(1 << 28)
+#define bit_F16C	(1 << 29)
+#define bit_RDRND	(1 << 30)
 
 /* %edx */
 #define bit_CMPXCHG8B	(1 << 8)
@@ -51,49 +56,133 @@
 #define bit_LAHF_LM	(1 << 0)
 #define bit_ABM		(1 << 5)
 #define bit_SSE4a	(1 << 6)
+#define bit_PRFCHW	(1 << 8)
 #define bit_XOP         (1 << 11)
 #define bit_LWP 	(1 << 15)
 #define bit_FMA4        (1 << 16)
+#define bit_TBM         (1 << 21)
 
 /* %edx */
+#define bit_MMXEXT	(1 << 22)
 #define bit_LM		(1 << 29)
 #define bit_3DNOWP	(1 << 30)
 #define bit_3DNOW	(1 << 31)
 
+/* Extended Features (%eax == 7) */
+#define bit_FSGSBASE	(1 << 0)
+#define bit_BMI	(1 << 3)
+#define bit_HLE	(1 << 4)
+#define bit_AVX2	(1 << 5)
+#define bit_BMI2	(1 << 8)
+#define bit_RTM	(1 << 11)
+#define bit_RDSEED	(1 << 18)
+#define bit_ADX	(1 << 19)
+
+/* Extended State Enumeration Sub-leaf (%eax == 13, %ecx == 1) */
+#define bit_XSAVEOPT	(1 << 0)
+
+/* Signatures for different CPU implementations as returned in uses
+   of cpuid with level 0.  */
+#define signature_AMD_ebx	0x68747541
+#define signature_AMD_ecx	0x444d4163
+#define signature_AMD_edx	0x69746e65
+
+#define signature_CENTAUR_ebx	0x746e6543
+#define signature_CENTAUR_ecx	0x736c7561
+#define signature_CENTAUR_edx	0x48727561
+
+#define signature_CYRIX_ebx	0x69727943
+#define signature_CYRIX_ecx	0x64616574
+#define signature_CYRIX_edx	0x736e4978
+
+#define signature_INTEL_ebx	0x756e6547
+#define signature_INTEL_ecx	0x6c65746e
+#define signature_INTEL_edx	0x49656e69
+
+#define signature_TM1_ebx	0x6e617254
+#define signature_TM1_ecx	0x55504361
+#define signature_TM1_edx	0x74656d73
+
+#define signature_TM2_ebx	0x756e6547
+#define signature_TM2_ecx	0x3638784d
+#define signature_TM2_edx	0x54656e69
+
+#define signature_NSC_ebx	0x646f6547
+#define signature_NSC_ecx	0x43534e20
+#define signature_NSC_edx	0x79622065
+
+#define signature_NEXGEN_ebx	0x4778654e
+#define signature_NEXGEN_ecx	0x6e657669
+#define signature_NEXGEN_edx	0x72446e65
+
+#define signature_RISE_ebx	0x65736952
+#define signature_RISE_ecx	0x65736952
+#define signature_RISE_edx	0x65736952
+
+#define signature_SIS_ebx	0x20536953
+#define signature_SIS_ecx	0x20536953
+#define signature_SIS_edx	0x20536953
+
+#define signature_UMC_ebx	0x20434d55
+#define signature_UMC_ecx	0x20434d55
+#define signature_UMC_edx	0x20434d55
+
+#define signature_VIA_ebx	0x20414956
+#define signature_VIA_ecx	0x20414956
+#define signature_VIA_edx	0x20414956
+
+#define signature_VORTEX_ebx	0x74726f56
+#define signature_VORTEX_ecx	0x436f5320
+#define signature_VORTEX_edx	0x36387865
 
 #if defined(__i386__) && defined(__PIC__)
 /* %ebx may be the PIC register.  */
 #if __GNUC__ >= 3
 #define __cpuid(level, a, b, c, d)			\
-  __asm__ ("xchg{l}\t{%%}ebx, %1\n\t"			\
+  __asm__ ("xchg{l}\t{%%}ebx, %k1\n\t"			\
 	   "cpuid\n\t"					\
-	   "xchg{l}\t{%%}ebx, %1\n\t"			\
-	   : "=a" (a), "=r" (b), "=c" (c), "=d" (d)	\
+	   "xchg{l}\t{%%}ebx, %k1\n\t"			\
+	   : "=a" (a), "=&r" (b), "=c" (c), "=d" (d)	\
 	   : "0" (level))
 
 #define __cpuid_count(level, count, a, b, c, d)		\
-  __asm__ ("xchg{l}\t{%%}ebx, %1\n\t"			\
+  __asm__ ("xchg{l}\t{%%}ebx, %k1\n\t"			\
 	   "cpuid\n\t"					\
-	   "xchg{l}\t{%%}ebx, %1\n\t"			\
-	   : "=a" (a), "=r" (b), "=c" (c), "=d" (d)	\
+	   "xchg{l}\t{%%}ebx, %k1\n\t"			\
+	   : "=a" (a), "=&r" (b), "=c" (c), "=d" (d)	\
 	   : "0" (level), "2" (count))
 #else
 /* Host GCCs older than 3.0 weren't supporting Intel asm syntax
    nor alternatives in i386 code.  */
 #define __cpuid(level, a, b, c, d)			\
-  __asm__ ("xchgl\t%%ebx, %1\n\t"			\
+  __asm__ ("xchgl\t%%ebx, %k1\n\t"			\
 	   "cpuid\n\t"					\
-	   "xchgl\t%%ebx, %1\n\t"			\
-	   : "=a" (a), "=r" (b), "=c" (c), "=d" (d)	\
+	   "xchgl\t%%ebx, %k1\n\t"			\
+	   : "=a" (a), "=&r" (b), "=c" (c), "=d" (d)	\
 	   : "0" (level))
 
 #define __cpuid_count(level, count, a, b, c, d)		\
-  __asm__ ("xchgl\t%%ebx, %1\n\t"			\
+  __asm__ ("xchgl\t%%ebx, %k1\n\t"			\
 	   "cpuid\n\t"					\
-	   "xchgl\t%%ebx, %1\n\t"			\
-	   : "=a" (a), "=r" (b), "=c" (c), "=d" (d)	\
+	   "xchgl\t%%ebx, %k1\n\t"			\
+	   : "=a" (a), "=&r" (b), "=c" (c), "=d" (d)	\
 	   : "0" (level), "2" (count))
 #endif
+#elif defined(__x86_64__) && (defined(__code_model_medium__) || defined(__code_model_large__)) && defined(__PIC__)
+/* %rbx may be the PIC register.  */
+#define __cpuid(level, a, b, c, d)			\
+  __asm__ ("xchg{q}\t{%%}rbx, %q1\n\t"			\
+	   "cpuid\n\t"					\
+	   "xchg{q}\t{%%}rbx, %q1\n\t"			\
+	   : "=a" (a), "=&r" (b), "=c" (c), "=d" (d)	\
+	   : "0" (level))
+
+#define __cpuid_count(level, count, a, b, c, d)		\
+  __asm__ ("xchg{q}\t{%%}rbx, %q1\n\t"			\
+	   "cpuid\n\t"					\
+	   "xchg{q}\t{%%}rbx, %q1\n\t"			\
+	   : "=a" (a), "=&r" (b), "=c" (c), "=d" (d)	\
+	   : "0" (level), "2" (count))
 #else
 #define __cpuid(level, a, b, c, d)			\
   __asm__ ("cpuid\n\t"					\
@@ -118,9 +207,9 @@ __get_cpuid_max (unsigned int __ext, unsigned int *__sig)
 {
   unsigned int __eax, __ebx, __ecx, __edx;
 
-#ifndef __x86_64__
-#if __GNUC__ >= 3
+#ifdef __i386__
   /* See if we can use cpuid.  On AMD64 we always can.  */
+#if __GNUC__ >= 3
   __asm__ ("pushf{l|d}\n\t"
 	   "pushf{l|d}\n\t"
 	   "pop{l}\t%0\n\t"
@@ -181,20 +270,3 @@ __get_cpuid (unsigned int __level,
   __cpuid (__level, *__eax, *__ebx, *__ecx, *__edx);
   return 1;
 }
-
-#ifndef NOINLINE
-#define NOINLINE __attribute__ ((noinline))
-#endif
-
-unsigned int i386_cpuid (void) NOINLINE;
-
-unsigned int NOINLINE
-i386_cpuid (void)
-{
-  unsigned int eax, ebx, ecx, edx;
-
-  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
-    return 0;
-
-  return edx;
-}
diff --git a/gdb/common/linux-btrace.c b/gdb/common/linux-btrace.c
index 1f9a001..0ec13bb 100644
--- a/gdb/common/linux-btrace.c
+++ b/gdb/common/linux-btrace.c
@@ -31,6 +31,7 @@
 #include "regcache.h"
 #include "gdbthread.h"
 #include "gdb_wait.h"
+#include "i386-cpuid.h"
 
 #if HAVE_LINUX_PERF_EVENT_H
 
@@ -339,13 +340,10 @@ kernel_supports_btrace (void)
 static int
 intel_supports_btrace (void)
 {
-#if defined __i386__ || defined __x86_64__
   unsigned int cpuid, model, family;
 
-  __asm__ __volatile__ ("movl   $1, %%eax;"
-			"cpuid;"
-			: "=a" (cpuid)
-			:: "%ebx", "%ecx", "%edx");
+  if (!i386_cpuid (1, &cpuid, NULL, NULL, NULL))
+    return 0;
 
   family = (cpuid >> 8) & 0xf;
   model = (cpuid >> 4) & 0xf;
@@ -376,12 +374,6 @@ intel_supports_btrace (void)
     }
 
   return 1;
-
-#else /* !defined __i386__ && !defined __x86_64__ */
-
-  return 0;
-
-#endif /* !defined __i386__ && !defined __x86_64__ */
 }
 
 /* Check whether the cpu supports branch tracing.  */
@@ -389,22 +381,15 @@ intel_supports_btrace (void)
 static int
 cpu_supports_btrace (void)
 {
-#if defined __i386__ || defined __x86_64__
+  unsigned int ebx, ecx, edx;
   char vendor[13];
 
-  __asm__ __volatile__ ("xorl   %%ebx, %%ebx;"
-			"xorl   %%ecx, %%ecx;"
-			"xorl   %%edx, %%edx;"
-			"movl   $0,    %%eax;"
-			"cpuid;"
-			"movl   %%ebx,  %0;"
-			"movl   %%edx,  %1;"
-			"movl   %%ecx,  %2;"
-			: "=m" (vendor[0]),
-			  "=m" (vendor[4]),
-			  "=m" (vendor[8])
-			:
-			: "%eax", "%ebx", "%ecx", "%edx");
+  if (!i386_cpuid (0, NULL, &ebx, &ecx, &edx))
+    return 0;
+
+  memcpy (&vendor[0], &ebx, 4);
+  memcpy (&vendor[4], &ecx, 4);
+  memcpy (&vendor[8], &edx, 4);
   vendor[12] = '\0';
 
   if (strcmp (vendor, "GenuineIntel") == 0)
@@ -412,12 +397,6 @@ cpu_supports_btrace (void)
 
   /* Don't know about others.  Let's assume they do.  */
   return 1;
-
-#else /* !defined __i386__ && !defined __x86_64__ */
-
-  return 0;
-
-#endif /* !defined __i386__ && !defined __x86_64__ */
 }
 
 /* See linux-btrace.h.  */
diff --git a/gdb/go32-nat.c b/gdb/go32-nat.c
index 0d9bb9d..a3f75b2 100644
--- a/gdb/go32-nat.c
+++ b/gdb/go32-nat.c
@@ -96,6 +96,7 @@
 #include "buildsym.h"
 #include "i387-tdep.h"
 #include "i386-tdep.h"
+#include "i386-cpuid.h"
 #include "value.h"
 #include "regcache.h"
 #include "gdb_string.h"
@@ -1141,6 +1142,21 @@ go32_sysinfo (char *arg, int from_tty)
   else if (u.machine[0] == 'i' && u.machine[1] > 4)
     {
       /* CPUID with EAX = 0 returns the Vendor ID.  */
+#if 0
+      /* Ideally we would use i386_cpuid(), but it needs someone to run
+         native tests first to make sure things actually work.  They should.
+         http://sourceware.org/ml/gdb-patches/2013-05/msg00164.html  */
+      unsigned int eax, ebx, ecx, edx;
+
+      if (i386_cpuid (0, &eax, &ebx, &ecx, &edx))
+	{
+	  cpuid_max = eax;
+	  memcpy (&vendor[0], &ebx, 4);
+	  memcpy (&vendor[4], &ecx, 4);
+	  memcpy (&vendor[8], &edx, 4);
+	  cpuid_vendor[12] = '\0';
+	}
+#else
       __asm__ __volatile__ ("xorl   %%ebx, %%ebx;"
 			    "xorl   %%ecx, %%ecx;"
 			    "xorl   %%edx, %%edx;"
@@ -1157,6 +1173,7 @@ go32_sysinfo (char *arg, int from_tty)
 			    :
 			    : "%eax", "%ebx", "%ecx", "%edx");
       cpuid_vendor[12] = '\0';
+#endif
     }
 
   printf_filtered ("CPU Type.......................%s", u.machine);
@@ -1182,6 +1199,10 @@ go32_sysinfo (char *arg, int from_tty)
       int amd_p = strcmp (cpuid_vendor, "AuthenticAMD") == 0;
       unsigned cpu_family, cpu_model;
 
+#if 0
+      /* See comment above about cpuid usage.  */
+      i386_cpuid (1, &cpuid_eax, &cpuid_ebx, NULL, &cpuid_edx);
+#else
       __asm__ __volatile__ ("movl   $1, %%eax;"
 			    "cpuid;"
 			    : "=a" (cpuid_eax),
@@ -1189,6 +1210,7 @@ go32_sysinfo (char *arg, int from_tty)
 			      "=d" (cpuid_edx)
 			    :
 			    : "%ecx");
+#endif
       brand_idx = cpuid_ebx & 0xff;
       cpu_family = (cpuid_eax >> 8) & 0xf;
       cpu_model  = (cpuid_eax >> 4) & 0xf;
diff --git a/gdb/testsuite/gdb.arch/i386-avx.c b/gdb/testsuite/gdb.arch/i386-avx.c
index bcfa18f..7fd1217 100644
--- a/gdb/testsuite/gdb.arch/i386-avx.c
+++ b/gdb/testsuite/gdb.arch/i386-avx.c
@@ -53,7 +53,7 @@ have_avx (void)
 {
   unsigned int eax, ebx, ecx, edx;
 
-  if (!__get_cpuid (1, &eax, &ebx, &ecx, &edx))
+  if (!i386_cpuid (1, &eax, &ebx, &ecx, &edx))
     return 0;
 
   if ((ecx & (bit_AVX | bit_OSXSAVE)) == (bit_AVX | bit_OSXSAVE))
diff --git a/gdb/testsuite/gdb.arch/i386-avx.exp b/gdb/testsuite/gdb.arch/i386-avx.exp
index 964806c..bbbc6f4 100644
--- a/gdb/testsuite/gdb.arch/i386-avx.exp
+++ b/gdb/testsuite/gdb.arch/i386-avx.exp
@@ -34,7 +34,7 @@ if [get_compiler_info] {
 
 set additional_flags ""
 if [test_compiler_info gcc*] {
-    set additional_flags "additional_flags=-mavx"
+    set additional_flags "additional_flags=-mavx -I${srcdir}/../common"
 }
 
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug $additional_flags]] != "" } {
diff --git a/gdb/testsuite/gdb.arch/i386-sse.c b/gdb/testsuite/gdb.arch/i386-sse.c
index d431527..8617c8a 100644
--- a/gdb/testsuite/gdb.arch/i386-sse.c
+++ b/gdb/testsuite/gdb.arch/i386-sse.c
@@ -51,7 +51,10 @@ v4sf_t data[] =
 int
 have_sse (void)
 {
-  int edx = i386_cpuid ();
+  int edx;
+
+  if (!i386_cpuid (1, NULL, NULL, NULL, &edx))
+    return 0;
 
   if (edx & bit_SSE)
     return 1;
diff --git a/gdb/testsuite/gdb.arch/i386-sse.exp b/gdb/testsuite/gdb.arch/i386-sse.exp
index 5923eca..c62a3a0 100644
--- a/gdb/testsuite/gdb.arch/i386-sse.exp
+++ b/gdb/testsuite/gdb.arch/i386-sse.exp
@@ -34,7 +34,7 @@ if [get_compiler_info] {
 
 set additional_flags ""
 if [test_compiler_info gcc*] {
-    set additional_flags "additional_flags=-msse"
+    set additional_flags "additional_flags=-msse -I${srcdir}/../common"
 }
 
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug $additional_flags]] != "" } {
-- 
1.8.2.1

Re: [PATCH v4] gdb: clean up x86 cpuid implementations

[Pedro Alves]

On 06/19/2013 06:24 PM, Mike Frysinger wrote:

> gdb/:
> 2013-06-18  Mike Frysinger  <vapier@gentoo.org>
> 
> 	* Makefile.in (HFILES_NO_SRCDIR): Add common/i386-cpuid.h and
> 	common/i386-gcc-cpuid.h.
> 	* common/i386-cpuid.h: New wrapper header around i386-gcc-cpuid.h.
> 	* common/i386-gcc-cpuid.h: Rename from testsuite/gdb.arch/i386-cpuid.h.
> 	Copy the latest version from upstream gcc.
> 	* common/linux-btrace.c: Include i386-cpuid.h.
> 	(intel_supports_btrace): Delete x86 ifdefs and replace inline asm with
> 	call to i386_cpuid.
> 	(cpu_supports_btrace): Likewise.
> 	* go32-nat.c: Include i386-cpuid.h.
> 	(go32_sysinfo): Add (disabled) calls to i386_cpuid with comments.
> 
> gdb/testsuite/:
> 2013-06-18  Mike Frysinger  <vapier@gentoo.org>
> 
> 	* gdb.arch/i386-avx.c (have_avx): Change __get_cpuid call to i386_cpuid.
> 	* gdb.arch/i386-avx.exp (additional_flags): Add -I${srcdir}/../common.
> 	* gdb.arch/i386-cpuid.h: Moved to ../common/i386-gcc-cpuid.h.
> 	* gdb.arch/i386-sse.c: Call new i386_cpuid function.
> 	* gdb.arch/i386-see.exp (additional_flags): Add -I${srcdir}/../common.

OK.

Thanks!

-- 
Pedro Alves

Re: [PATCH v4] gdb: clean up x86 cpuid implementations

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 1260 bytes --]

On Wednesday 19 June 2013 13:37:50 Pedro Alves wrote:
> On 06/19/2013 06:24 PM, Mike Frysinger wrote:
> > gdb/:
> > 2013-06-18  Mike Frysinger  <vapier@gentoo.org>
> > 
> > 	* Makefile.in (HFILES_NO_SRCDIR): Add common/i386-cpuid.h and
> > 	common/i386-gcc-cpuid.h.
> > 	* common/i386-cpuid.h: New wrapper header around i386-gcc-cpuid.h.
> > 	* common/i386-gcc-cpuid.h: Rename from testsuite/gdb.arch/i386-cpuid.h.
> > 	Copy the latest version from upstream gcc.
> > 	* common/linux-btrace.c: Include i386-cpuid.h.
> > 	(intel_supports_btrace): Delete x86 ifdefs and replace inline asm with
> > 	call to i386_cpuid.
> > 	(cpu_supports_btrace): Likewise.
> > 	* go32-nat.c: Include i386-cpuid.h.
> > 	(go32_sysinfo): Add (disabled) calls to i386_cpuid with comments.
> > 
> > gdb/testsuite/:
> > 2013-06-18  Mike Frysinger  <vapier@gentoo.org>
> > 
> > 	* gdb.arch/i386-avx.c (have_avx): Change __get_cpuid call to i386_cpuid.
> > 	* gdb.arch/i386-avx.exp (additional_flags): Add -I${srcdir}/../common.
> > 	* gdb.arch/i386-cpuid.h: Moved to ../common/i386-gcc-cpuid.h.
> > 	* gdb.arch/i386-sse.c: Call new i386_cpuid function.
> > 	* gdb.arch/i386-see.exp (additional_flags): Add -I${srcdir}/../common.
> 
> OK.

pushed now
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Regression for btrace [Re: [PATCH v4] gdb: clean up x86 cpuid implementations]

[Jan Kratochvil]

On Thu, 20 Jun 2013 00:29:05 +0200, Mike Frysinger wrote:
> On Wednesday 19 June 2013 13:37:50 Pedro Alves wrote:
> > On 06/19/2013 06:24 PM, Mike Frysinger wrote:
> > > 	* common/linux-btrace.c: Include i386-cpuid.h.
> > > 	(intel_supports_btrace): Delete x86 ifdefs and replace inline asm with
> > > 	call to i386_cpuid.
> > > 	(cpu_supports_btrace): Likewise.
[...]
> pushed now

 Running gdb/testsuite/gdb.btrace/function_call_history.exp ...
+PASS: gdb.btrace/function_call_history.exp: record btrace
+PASS: gdb.btrace/function_call_history.exp: continue to breakpoint: cont to 43
+PASS: gdb.btrace/function_call_history.exp: set record function-call-history-size 0
+FAIL: gdb.btrace/function_call_history.exp: record function-call-history - with size unlimited
[...]
+FAIL: gdb.btrace/function_call_history.exp: show recursive function call history

My Intel CPU is not btrace-capable but after the patch Intel vendor is no
longer correctly identified.

-                       "movl   %%ebx,  %0;"
-                       "movl   %%edx,  %1;"
-                       "movl   %%ecx,  %2;"
-                       : "=m" (vendor[0]),
-                         "=m" (vendor[4]),
-                         "=m" (vendor[8])
-                       :
-                       : "%eax", "%ebx", "%ecx", "%edx");
+  if (!i386_cpuid (0, NULL, &ebx, &ecx, &edx))
+    return 0;
+
+  memcpy (&vendor[0], &ebx, 4);
+  memcpy (&vendor[4], &ecx, 4);
+  memcpy (&vendor[8], &edx, 4);

Here ecx and edx are exchanged by the patch.

OK to check in the fix this way?


Jan


gdb/
2013-06-21  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* common/linux-btrace.c (cpu_supports_btrace): Remove variable vendor,
	replace strcmp with signature_INTEL_ebx, signature_INTEL_ecx and
	signature_INTEL_edx comparisons.

diff --git a/gdb/common/linux-btrace.c b/gdb/common/linux-btrace.c
index 0ec13bb..b874c84 100644
--- a/gdb/common/linux-btrace.c
+++ b/gdb/common/linux-btrace.c
@@ -382,17 +382,12 @@ static int
 cpu_supports_btrace (void)
 {
   unsigned int ebx, ecx, edx;
-  char vendor[13];
 
   if (!i386_cpuid (0, NULL, &ebx, &ecx, &edx))
     return 0;
 
-  memcpy (&vendor[0], &ebx, 4);
-  memcpy (&vendor[4], &ecx, 4);
-  memcpy (&vendor[8], &edx, 4);
-  vendor[12] = '\0';
-
-  if (strcmp (vendor, "GenuineIntel") == 0)
+  if (ebx == signature_INTEL_ebx && ecx == signature_INTEL_ecx
+      && edx == signature_INTEL_edx)
     return intel_supports_btrace ();
 
   /* Don't know about others.  Let's assume they do.  */

Re: Regression for btrace [Re: [PATCH v4] gdb: clean up x86 cpuid implementations]

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 169 bytes --]

On Friday 21 June 2013 07:18:00 Jan Kratochvil wrote:
> OK to check in the fix this way?

i like how it has simplified the code!  lgtm (not that i'm an approver).
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: Regression for btrace [Re: [PATCH v4] gdb: clean up x86 cpuid implementations]

[Pedro Alves]

On 06/21/2013 04:33 PM, Mike Frysinger wrote:
> On Friday 21 June 2013 07:18:00 Jan Kratochvil wrote:
>> OK to check in the fix this way?
> 
> i like how it has simplified the code!  lgtm (not that i'm an approver).

FAOD, this is fine with me.

Thanks,
-- 
Pedro Alves

[commit] Re: Regression for btrace [Re: [PATCH v4] gdb: clean up x86 cpuid implementations]

[Jan Kratochvil]

On Fri, 21 Jun 2013 17:36:45 +0200, Pedro Alves wrote:
> On 06/21/2013 04:33 PM, Mike Frysinger wrote:
> > On Friday 21 June 2013 07:18:00 Jan Kratochvil wrote:
> >> OK to check in the fix this way?
> > 
> > i like how it has simplified the code!  lgtm (not that i'm an approver).
> 
> FAOD, this is fine with me.

Checked in:
	http://sourceware.org/ml/gdb-cvs/2013-06/msg00131.html


Jan