pengfei added a comment. In D155145#4556178 <https://reviews.llvm.org/D155145#4556178>, @craig.topper wrote:
> 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. Thanks Craig! That makes sense to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155145/new/ https://reviews.llvm.org/D155145 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
