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

[Pedro Alves <palves@redhat.com>]

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