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

Pedro Alves
2013-05-07T15:21:51Z

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