This is an archive of the discontinued LLVM Phabricator instance.

[clang] Add missing CPUID feature bit masks
Needs ReviewPublic

Authored by adamdb5 on Nov 20 2021, 9:39 AM.

Details

Summary

Whilst working on a project which uses the x86 CPUID instruction, I noticed the header provided by clang (cpuid.h), is missing some bit masks for the feature registers.

I've implemented the missing masks, and also noticed that the previous mask
for PKU was shifted one bit too far to the right. It was 0b100, when it
should have been 0b1000.

Sources:
https://www.scss.tcd.ie/~jones/CS4021/processor-identification-cpuid-instruction-note.pdf
https://www.felixcloutier.com/x86/cpuid

This is my first commit, so please let me know if this is worth considering,
and any feedback is appreciated.

Diff Detail

Event Timeline

adamdb5 created this revision.Nov 20 2021, 9:39 AM
adamdb5 retitled this revision from Whilst working on a project which uses the x86 CPUID instruction, I noticed the header provided by clang (cpuid.h), is missing some bit masks for the feature registers. to Add missing CPUID bit masks..Nov 20 2021, 9:41 AM
adamdb5 edited the summary of this revision. (Show Details)
adamdb5 retitled this revision from Add missing CPUID bit masks. to Add missing CPUID feature bit masks..
adamdb5 retitled this revision from Add missing CPUID feature bit masks. to Add missing CPUID feature bit masks.
adamdb5 retitled this revision from Add missing CPUID feature bit masks to [clang] Add missing CPUID feature bit masks.Nov 20 2021, 10:03 AM
adamdb5 added a project: Restricted Project.
adamdb5 updated this revision to Diff 388718.Nov 20 2021, 10:37 AM

ran clang-format

adamdb5 published this revision for review.Nov 20 2021, 12:56 PM

[clang] Add missing CPUID feature bit masks

craig.topper added inline comments.
clang/lib/Headers/cpuid.h
101

Please don't change the formatting despite for the constants despite what the lint check says. Having then in columns is more readable.

adamdb5 updated this revision to Diff 388744.Nov 21 2021, 2:08 AM

undo clang-format

ran clang-format

I think we'd better to keep the existing format. It's in good readability.

clang/lib/Headers/cpuid.h
116

Where's this from. The latest SDM doesn't define this bit. I didn't find it in your link either.

147

Is there any other source using the same name? The same below.

152

SDM calls it "FDP_EXCPTN_ONLY". Don't know where's "EXCPTN" from.

173

This is reserved in latest SDM.

214

This is reserved in latest SDM.

222

Why not "AVX512VP2INT"?

adamdb5 updated this revision to Diff 388746.EditedNov 21 2021, 3:32 AM

Corrected issues.

Thanks for the feedback, please see the following comments:

bit_HYPERVISOR: This seems to be standardized. See the following links:

Let me know if you think this shouldn't be added.

bit_IA64: I'm not entirely sure where I found this. I think it might have been wikipedia or something. I have removed it.

bit_TSCADJUST: Changed to match Intel SDM (IA32_TSC_ADJUST).

bit_FPUEXCEPT: Changed to match Intel SDM (FDP_EXCPTN_ONLY).

bit_LA57: Appears to be defined in June 2021 Intel SDM at Vol. 2A 3-219 (PDF page 794).

bit_AVX512VPCINT: Typo, changed to match Intel SDM (AVX512VP2INT).

bit_HYPERVISOR: This seems to be standardized. See the following links:

I'm not sure what are these bits defined according to. From PV of HW, I think this shouldn't be added. This bit is set by hypervisor rather than CPU like others. And I found this bit is still 0 in my VM.

bit_IA57: Appears to be defined in June 2021 Intel SDM at Vol. 2A 3-219 (PDF page 794).

Oh, I missed that. Thanks.

GCC has the same (but fewer) definitions, https://github.com/gcc-mirror/gcc/blob/master/gcc/config/i386/cpuid.h
+@hjl.tools to have a look.

clang/lib/Headers/cpuid.h
159

DEPR_FPU_CSDS ?

adamdb5 updated this revision to Diff 388751.Nov 21 2021, 5:07 AM

Renamed bit_DEPRFPUCSDS to bit_DEPR_FPU_CSDS