[X86][EVEX512] Restrict attaching EVEX512 for default CPU only, NFCI #65920
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Attaching EVEX512 is used to provide backward compatibility for legacy LLVM IR files, which didn't set EVEX512 feature explicitly. AVX512 and AVX10 targets have set or unset EVEX512 properly through X86.td. However, it's not feasible to list all AVX512 and AVX10 targets or their complementary set here to skip/restrict such code. Instead, we can restrict it for default CPU only. "generic" is used when "target-cpu" is not specified in IR, while "pentium4" and "x86-64" is the default CPU if "-march" is not specified in Clang for 32-bit and 64-bit targets respectively. This patch is no functional change intended, though it might affect scenarios like "-march=broadwell -mavx512dw", which looks like a misuse of "-march" and can be solved by changing to "-mtune=broadwell -mavx512dw".
|
-mavx512dw -> -mavx512bw |
KanRobert
reviewed
Sep 11, 2023
|
|
||
| if (posAVX512F != StringRef::npos && | ||
| (posNoAVX512F == StringRef::npos || posNoAVX512F < posAVX512F)) | ||
| if (posEVEX512 == StringRef::npos && posNoEVEX512 == StringRef::npos) |
There was a problem hiding this comment.
I think keeping it separate is friendly to understand. This outer logic is for AVX512 and inner is for EVEX512.
KanRobert
approved these changes
Sep 11, 2023
Where's the second comment? |
Oh, just saw it. Modified! |
ZijunZhaoCCK
pushed a commit
to ZijunZhaoCCK/llvm-project
that referenced
this pull request
Sep 19, 2023
…lvm#65920) Attaching EVEX512 is used to provide backward compatibility for legacy LLVM IR files, which didn't set EVEX512 feature explicitly. AVX512 and AVX10 targets have set or unset EVEX512 properly through X86.td. However, it's not feasible to list all AVX512 and AVX10 targets or their complementary set here to skip/restrict such code. Instead, we can restrict it for default CPU only. "generic" is used when "target-cpu" is not specified in IR, while "pentium4" and "x86-64" is the default CPU if "-march" is not specified in Clang for 32-bit and 64-bit targets respectively. This patch is no functional change intended, though it might affect scenarios like "-march=broadwell -mavx512bw", which looks like a misuse of "-march" and can be solved by changing to "-mtune=broadwell -mavx512bw".
|
sorry for the late problem, seems our qmcpack app may have made a recent change that exposed this issue: llc: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:7536: llvm::SDValue getMemcpyLoadsAndStores(llvm::SelectionDAG&, const llvm::SDLoc&, llvm::SDValue, llvm::SDValue, llvm::SDValue, uint64_t, llvm::Align, bool, bool, llvm::MachinePointerInfo, llvm::MachinePointerInfo, const llvm::AAMDNodes&, llvm::AAResults*): Assertion `NVT.bitsGE(VT)' failed. |
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Oct 27, 2023
…y, NFCI (llvm#65920)" breaks qmcpack This reverts commit 8a58407. Change-Id: Ife1e7e0c5e9ab5a2349b57dde856cae023579b7a
phoebewang
added a commit
to phoebewang/llvm-project
that referenced
this pull request
Oct 27, 2023
Solves crash mentioned in llvm#65920.
|
@ronlieb Thanks for reporting this issue! This exposed a bug not for this patch but a related one 2419409, which should be solved by #70420. |
phoebewang
added a commit
that referenced
this pull request
Oct 27, 2023
Solves crash mentioned in #65920.
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Nov 4, 2023
…lvm#65920) Attaching EVEX512 is used to provide backward compatibility for legacy LLVM IR files, which didn't set EVEX512 feature explicitly. AVX512 and AVX10 targets have set or unset EVEX512 properly through X86.td. However, it's not feasible to list all AVX512 and AVX10 targets or their complementary set here to skip/restrict such code. Instead, we can restrict it for default CPU only. "generic" is used when "target-cpu" is not specified in IR, while "pentium4" and "x86-64" is the default CPU if "-march" is not specified in Clang for 32-bit and 64-bit targets respectively. This patch is no functional change intended, though it might affect scenarios like "-march=broadwell -mavx512bw", which looks like a misuse of "-march" and can be solved by changing to "-mtune=broadwell -mavx512bw". Change-Id: Ia9ffa97056923b45f01e663a0df6c0c01fa69fe5
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Attaching EVEX512 is used to provide backward compatibility for legacy
LLVM IR files, which didn't set EVEX512 feature explicitly.
AVX512 and AVX10 targets have set or unset EVEX512 properly through
X86.td.
However, it's not feasible to list all AVX512 and AVX10 targets or their
complementary set here to skip/restrict such code.
Instead, we can restrict it for default CPU only. "generic" is used when
"target-cpu" is not specified in IR, while "pentium4" and "x86-64" is
the default CPU if "-march" is not specified in Clang for 32-bit and
64-bit targets respectively.
This patch is no functional change intended, though it might affect
scenarios like "-march=broadwell -mavx512bw", which looks like a misuse
of "-march" and can be solved by changing to "-mtune=broadwell -mavx512bw".