[PATCH] D155145: [X86] Add AVX-VNNI-INT16 instructions.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

In D155145#4556157 <https://reviews.llvm.org/D155145#4556157>, @anna wrote:

> In D155145#4554786 <https://reviews.llvm.org/D155145#4554786>, @anna wrote:
>
>>> Can you capture the values of EAX, EBX, ECX, and EDX after the two calls to getX86CpuIDAndInfoEx that have 0x7 as the first argument? Maybe there's a bug in CPUID on Sandy Bridge.
>>
>> Sure, on the original code before the patch you suggested right?
>> The two calls are:
>>
>>    bool HasLeaf7 =
>>           MaxLevel >= 7 && !getX86CpuIDAndInfoEx(0x7, 0x0, &EAX, &EBX, &ECX, &EDX);
>>   +   llvm::errs() << "Before setting fsgsbase the value for EAX: " << EAX
>>   +                     << " EBX: " << EBX << " ECX: " << ECX << "  EDX: " << EDX
>>   +                     << "\n";
>>     
>>       Features["fsgsbase"]   = HasLeaf7 && ((EBX >>  0) & 1);
>>   ....
>>   bool HasLeaf7Subleaf1 =
>>           MaxLevel >= 7 && !getX86CpuIDAndInfoEx(0x7, 0x1, &EAX, &EBX, &ECX, &EDX);
>>   +   llvm::errs() << "Before setting sha512 the value for EAX: " << EAX
>>   +                     << " EBX: " << EBX << " ECX: " << ECX << "  EDX: " << EDX
>>   +                     << "\n";
>>       Features["sha512"]     = HasLeaf7Subleaf1 && ((EAX >> 0) & 1);
>>   ...
>>   we set avxvnniint16 after this
>>
>> Takes a while to get a build on this machine, should have the output soon.
>
> @craig.topper here is the output:
>
>   Before setting fsgsbase the value for EAX: 0 EBX: 0 ECX: 0  EDX: 2617246720 // this is after the HasLeaf7 calculation
>   Before setting sha512 the value for EAX: 0 EBX: 0 ECX: 0  EDX: 2617246720 // this is after the HasLeaf7Subleaf1 calculation
>
> So, with your patch `HasLeaf7Subleaf1` is 0 as EAX is 0. Pls let me know if you need  any additional diagnostics output (we actually lose access to the machine on friday, since it is being retired!).
>
>> The documentation says that invalid subleaves of leaf 7 should return all 0s. So we thought it was safe to check the bits of sub leaf 1 even if eax from subleaf 0 doesn't say subleaf 1 is supported.
>
> This means the CPUID doesn't satisfy the documentation since EDX != 0 for SubLeaf1?

Interestingly all of the bits set in EDX are features that were things that were added in microcode patches in the wake of vulnerabilities like Spectre and Meltdown. Maybe the microcode patch forgot to check the subleaf since there was no subleaf implemented when sandy bridge was originally made.

I think my patch is the correct fix given that information. I'll post a patch for review shortly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155145/new/

https://reviews.llvm.org/D155145