[PATCH] Fix host processor identification for Sandy Bridge

[Aaron Ballman]

Ping?

On Sat, Mar 16, 2013 at 11:21 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
> I did some more digging and found out that Ivy Bridge has the same
> behavior as Sandy Bridge - the Pentium models don't have AVX support
> (they seem to be more similar to the corei7).  So I've revised the
> patch to include Ivy Bridge, and fixed OSHasAVXSupport to return false
> if we don't know how to handle the inline assembly for it.
>
> Btw, as for the < Windows 7 SP1 issues -- that's why we're checking
> for OS support using the xgetbv instruction.
>
> ~Aaron
>
> On Thu, Mar 14, 2013 at 1:31 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>> On Thu, Mar 14, 2013 at 12:20 PM, Benjamin Kramer <benny.kra at gmail.com> wrote:
>>>
>>> On 11.03.2013, at 15:55, Aaron Ballman <aaron at aaronballman.com> wrote:
>>>
>>>> This patch addresses PR15351 by explicitly checking for AVX support
>>>> when getting the host processor information.  Not all Sandy Bridge
>>>> processors support AVX (such as the Pentium-based ones), so I am
>>>> checking explicitly for AVX support when deciding which processor to
>>>> report.
>>>
>>> Do newer CPUs have the same issue. It's certainly an issue if you have e.g. an Ivy Bridge on SP1-less Windows 7.
>>
>> As far as I'm aware, they do not.  But I'm also not an expert on Intel
>> processor models.
>>
>>>> I'm not entirely certain of how to make a lit test for this though, so
>>>> suggestions are welcome.
>>>
>>> Sadly, there isn't really a way to fake a CPU in lit.
>>
>> Oh well.
>>
>>>
>>>> Index: lib/Support/Host.cpp
>>>> ===================================================================
>>>> --- lib/Support/Host.cpp      (revision 176803)
>>>> +++ lib/Support/Host.cpp      (working copy)
>>>> @@ -112,6 +112,16 @@
>>>>  #endif
>>>>  }
>>>>
>>>> +static bool OSHasAVXSupport() {
>>>> +#if defined(__GNUC__)
>>>> +  int rEAX, rEDX;
>>>> +  __asm__ ("xgetbv" : "=a" (rEAX), "=d" (rEDX) : "c" (0));
>>>> +#elif defined(_MSC_VER)
>>>> +  unsigned long long rEAX = _xgetbv(_XCR_XFEATURE_ENABLED_MASK);
>>>> +#endif
>>>> +  return (rEAX & 6) == 6;
>>>
>>> What should happen if neither __GNUC__ nor _MSC_VER is set? return false?
>>
>> I'd say returning false is the safest bet; less likely to cause
>> illegal instruction crashes this way.  Would it make sense to also
>> generate a warning so someone knows to look at the code for their
>> compiler?
>>
>> Thanks!
>>
>> ~Aaron