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

[Pedro Alves <palves@redhat.com>]

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