Skip to content

[X86][EVEX512] Restrict attaching EVEX512 for default CPU only, NFCI #65920

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 11, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 17 additions & 12 deletions llvm/lib/Target/X86/X86Subtarget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -268,18 +268,23 @@ void X86Subtarget::initSubtargetFeatures(StringRef CPU, StringRef TuneCPU,
if (!FS.empty())
FullFS = (Twine(FullFS) + "," + FS).str();

// Attach EVEX512 feature when we have AVX512 features and EVEX512 is not set.
size_t posNoEVEX512 = FS.rfind("-evex512");
// Make sure we won't be cheated by "-avx512fp16".
size_t posNoAVX512F = FS.endswith("-avx512f") ? FS.size() - 8
: FS.rfind("-avx512f,");
size_t posEVEX512 = FS.rfind("+evex512");
size_t posAVX512F = FS.rfind("+avx512"); // Any AVX512XXX will enable AVX512F.

if (posAVX512F != StringRef::npos &&
(posNoAVX512F == StringRef::npos || posNoAVX512F < posAVX512F))
if (posEVEX512 == StringRef::npos && posNoEVEX512 == StringRef::npos)
FullFS += ",+evex512";
// Attach EVEX512 feature when we have AVX512 features with a default CPU.
// "pentium4" is default CPU for 32-bit targets.
// "x86-64" is default CPU for 64-bit targets.
if (CPU == "generic" || CPU == "pentium4" || CPU == "x86-64") {
size_t posNoEVEX512 = FS.rfind("-evex512");
// Make sure we won't be cheated by "-avx512fp16".
size_t posNoAVX512F = FS.endswith("-avx512f") ? FS.size() - 8
: FS.rfind("-avx512f,");
size_t posEVEX512 = FS.rfind("+evex512");
// Any AVX512XXX will enable AVX512F.
size_t posAVX512F = FS.rfind("+avx512");

if (posAVX512F != StringRef::npos &&
(posNoAVX512F == StringRef::npos || posNoAVX512F < posAVX512F))
if (posEVEX512 == StringRef::npos && posNoEVEX512 == StringRef::npos)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Combine two if to one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think keeping it separate is friendly to understand. This outer logic is for AVX512 and inner is for EVEX512.

FullFS += ",+evex512";
}

// Parse features string and set the CPU.
ParseSubtargetFeatures(CPU, TuneCPU, FullFS);
Expand Down