Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/include/llvm/Support/X86TargetParser.def | ||
---|---|---|
220 | what commented out features? |
llvm/include/llvm/Support/X86TargetParser.def | ||
---|---|---|
220 | So this is copy/pasted from: https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/X86Target.def#L70 (where it will probably be removed in a future patch?). That comment came from here: https://reviews.llvm.org/D47474 If you look at the 'diff' here: https://reviews.llvm.org/D47474?vs=148894&id=156484#toc (1st patch vs last) You can see that the original used a bitmask to create the values rather than the string list (as suggested by @craig.topper in the review). That version had some commented out in the bitmasks. However, I never removed the comment! So this comment likely should just be deleted. |
I'm not opposed either.
llvm/include/llvm/Support/X86TargetParser.def | ||
---|---|---|
220 | I'm not sure the commented out features made it over. For example, FEATURE_TSX was one of the commented out values, but +tsx doesn't appear in these strings. |
There's nothing later than CannonLake here - does Intel need to at least reference up to Tiger/Rocketlake?
llvm/include/llvm/Support/X86TargetParser.def | ||
---|---|---|
220 |
Not sure what you mean - the whole file is being removed as part of this change. |
llvm/include/llvm/Support/X86TargetParser.def | ||
---|---|---|
220 |
Ah! Missed that! Phab seems to have changed how it displays deleted files! |
Thanks for reminding. We've supported -march=${CPU}, but forgot to update this table. We will update it.
Thanks for reminding. We've supported -march=${CPU}, but forgot to update this table. We will update it.
Shall we get this patch committed first before making any changes?
Yes, committing the patch first looks good to me.
This should be autogenerated from the main X86.td, otherwise this is, unfortunately only partially, duplicates that.
E.g. there isn't a single AMD CPU in there.
The intent of this list was to duplicate the list used for the cpu_dispatch feature from the Intel Compiler, which for obvious reasons didn't include AMD names. OUR implementation at least uses feature lists, directly, so the equivalent AMD cpus would work here. At the time, I contacted a few people from AMD via email and invited them to improve this list if they'd like to, but I don't believe I ever saw a patch to do so.
what commented out features?