Page MenuHomePhabricator

[X86] Support -march=tigerlake
ClosedPublic

Authored by xiangzhangllvm on Aug 6 2019, 6:08 PM.

Details

Summary

Support -march=tigerlake for x86.
Compare with Icelake Client, It include 4 more new features ,they are avx512vp2intersect, movdiri, movdir64b, shstk.

Diff Detail

Repository
rL LLVM

Event Timeline

xiangzhangllvm created this revision.Aug 6 2019, 6:08 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 6 2019, 6:08 PM
xiangzhangllvm retitled this revision from Tigerlake to [X86] Support -march=tigerlake.Aug 6 2019, 6:21 PM
xiangzhangllvm edited the summary of this revision. (Show Details)
RKSimon added inline comments.Aug 8 2019, 1:16 PM
clang/lib/Basic/Targets/X86.cpp
167 ↗(On Diff #213769)

Are we ok with introducing this kind of thing again or should we use goto?

clang/test/Preprocessor/predefined-arch-macros.c
1569 ↗(On Diff #213769)

You test for no-wbnoinvd - do we need to do the same for pconfig?

llvm/lib/Support/Host.cpp
749 ↗(On Diff #213769)

Check for tigerlake with avx512vp2intersect cpuid bit? @craig.topper any suggestions?

xiangzhangllvm marked 2 inline comments as done.Aug 8 2019, 7:39 PM
xiangzhangllvm added inline comments.
clang/lib/Basic/Targets/X86.cpp
167 ↗(On Diff #213769)

Sorry, not quite understand, How to introducing it again?

clang/test/Preprocessor/predefined-arch-macros.c
1569 ↗(On Diff #213769)

I am not much sure about the -wbnoinvd, The tigerlake succeed to icelake, So I write the check follow the icelake.

craig.topper added inline comments.Aug 8 2019, 7:59 PM
clang/lib/Basic/Targets/X86.cpp
167 ↗(On Diff #213769)

We had a bunch of ifs like this in the switch before and turned into a maintenance issue because multiple ifs needed to be updated everytime someone added something to the stop of the switch. They were removed in https://reviews.llvm.org/D63018

clang/test/Preprocessor/predefined-arch-macros.c
1535 ↗(On Diff #213769)

Missing a check for AVX512VP2INTERSECT

1569 ↗(On Diff #213769)

I think icelake ended up like that because we had a bug once due to the ifs in the switch. Go ahead and add a NOT for PCONFIG

llvm/lib/Support/Host.cpp
749 ↗(On Diff #213769)

Sounds good to me.

xiangzhangllvm marked 7 inline comments as done and an inline comment as not done.

All changed, Thank you very much!

craig.topper added inline comments.Aug 9 2019, 11:04 PM
clang/test/Preprocessor/predefined-arch-macros.c
1571 ↗(On Diff #214337)

This is only effective if its alphabetically where PCONFIG should be.

llvm/include/llvm/Support/X86TargetParser.def
164 ↗(On Diff #214337)

The COMPAT macro is for things implemented in libgcc. I don't know if this one is yet. So it should be at position 70 instead.

llvm/lib/Support/Host.cpp
750 ↗(On Diff #214337)

There 3 variables Features, Features2, and Feature3. Each are 32 bits. The current bit position you have would put it in Features2, but when you move it to 70 it will be Features3.

xiangzhangllvm marked 3 inline comments as done.Aug 9 2019, 11:58 PM
xiangzhangllvm added inline comments.
clang/test/Preprocessor/predefined-arch-macros.c
1571 ↗(On Diff #214337)

Yes!

llvm/include/llvm/Support/X86TargetParser.def
164 ↗(On Diff #214337)

Make sense!

llvm/lib/Support/Host.cpp
750 ↗(On Diff #214337)

Oh! Yes! I ignored it! Thank you very much!!

xiangzhangllvm marked an inline comment as done.Aug 10 2019, 12:08 AM

Done again, thank you again!!

craig.topper added inline comments.Aug 10 2019, 12:48 AM
llvm/lib/Support/Host.cpp
1088 ↗(On Diff #214506)

This also needs HasAVX512Save. Sorry I didn't catch that earlier.

xiangzhangllvm marked an inline comment as done.Aug 10 2019, 1:18 AM
xiangzhangllvm added inline comments.
llvm/lib/Support/Host.cpp
1088 ↗(On Diff #214506)

OK, seems all AVX512 features AND the HasAVX512Save, but I check the SPEC of avxvp2intersect, it just show CPUID.7.0.EDX[8].

craig.topper added inline comments.Aug 10 2019, 1:35 AM
llvm/lib/Support/Host.cpp
1088 ↗(On Diff #214506)

It's in the E4NF table where it says "State requirement, Table 2-37 not met"

This revision is now accepted and ready to land.Aug 10 2019, 1:35 AM
This revision was automatically updated to reflect the committed changes.