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

Pedro Alves
2013-05-07T14:49:36Z

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