[DAG] Narrow vselect mask to vXi1 in foldToMaskedStore#201609
Merged
Conversation
|
@llvm/pr-subscribers-llvm-selectiondag @llvm/pr-subscribers-backend-x86 Author: Feng Zou (fzou1) ChangesOn AVX512F targets without VLX (e.g. KNL), masked stores are custom-lowered by widening to 512-bit operations. A maxnum/minnum store-back is expanded using a legacy packed (CMPP) vector comparison, and the store-back DAGCombine fuses that vector comparison into the masked store. As a result LowerMSTORE receives a mask with vNi32 elements instead of vNi1 and hits an assertion: Assertion `Mask.getSimpleValueType().getScalarType() == MVT::i1 && (reproduces with -mattr=+avx512f or -mcpu=knl; -mcpu=skylake-avx512 and +avx512vl are unaffected because the mask is produced as a k-register.) Accept a wide vNi32 mask in addition to vNi1: widen it in its own element type and truncate to vNi1 before building the masked store. A CMPP result is all-ones/all-zeros per lane, so bit 0 of each lane carries the predicate and the zero-filled widened lanes truncate to 0 (disabled). Full diff: https://github.com/llvm/llvm-project/pull/201609.diff 2 Files Affected:
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 8bb44e55d713f..3c85089f092e5 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -34287,14 +34287,23 @@ static SDValue LowerMSTORE(SDValue Op, const X86Subtarget &Subtarget,
unsigned NumEltsInWideVec = 512/VT.getScalarSizeInBits();
MVT WideDataVT = MVT::getVectorVT(ScalarVT, NumEltsInWideVec);
- // Mask element has to be i1.
- assert(Mask.getSimpleValueType().getScalarType() == MVT::i1 &&
- "Unexpected mask type");
+ // For AVX512F targets without VLX (e.g. KNL), a maxnum/minnum store-back
+ // fuses a legacy CMPP vector comparison into the masked store, so the mask
+ // may arrive as vNi32 instead of vNi1. Accept both and convert to i1 below.
+ MVT MaskEltVT = Mask.getSimpleValueType().getScalarType();
+ assert((MaskEltVT == MVT::i1 || MaskEltVT.getSizeInBits() >= 8) &&
+ "Expected an i1 mask or a wide vector mask from a CMPP comparison");
- MVT WideMaskVT = MVT::getVectorVT(MVT::i1, NumEltsInWideVec);
+ MVT WideMaskVT = MVT::getVectorVT(MaskEltVT, NumEltsInWideVec);
DataToStore = ExtendToType(DataToStore, WideDataVT, DAG);
Mask = ExtendToType(Mask, WideMaskVT, DAG, true);
+ // Truncate a vNi32 mask down to vNi1 for the masked op. A CMPP result is
+ // all-ones/all-zeros per lane, so bit 0 of each lane carries the predicate,
+ // and the zero-filled widened lanes truncate to 0 (disabled).
+ if (MaskEltVT != MVT::i1)
+ Mask = DAG.getNode(ISD::TRUNCATE, dl,
+ MVT::getVectorVT(MVT::i1, NumEltsInWideVec), Mask);
return DAG.getMaskedStore(N->getChain(), dl, DataToStore, N->getBasePtr(),
N->getOffset(), Mask, N->getMemoryVT(),
N->getMemOperand(), N->getAddressingMode(),
diff --git a/llvm/test/CodeGen/X86/avx512-maxnum-minnum-masked-store.ll b/llvm/test/CodeGen/X86/avx512-maxnum-minnum-masked-store.ll
new file mode 100644
index 0000000000000..caeba9a2f1372
--- /dev/null
+++ b/llvm/test/CodeGen/X86/avx512-maxnum-minnum-masked-store.ll
@@ -0,0 +1,149 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mattr=+avx512f | FileCheck %s --check-prefix=AVX512F
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mattr=+avx512f,+avx512vl | FileCheck %s --check-prefix=AVX512VL
+
+; On AVX512F targets without VLX (e.g. KNL), masked stores are widened to 512
+; bits during custom lowering. A maxnum/minnum store-back fuses the legacy CMPP
+; vector comparison into the masked store, so the mask arrives as vNi32 rather
+; than vNi1. The lowering must accept and convert the wide mask, not assert.
+
+define void @maxnum_v4f32_masked_store(<4 x float> %a, ptr %ptr) {
+; AVX512F-LABEL: maxnum_v4f32_masked_store:
+; AVX512F: # %bb.0:
+; AVX512F-NEXT: vmovups (%rdi), %xmm1
+; AVX512F-NEXT: vmaxps %xmm0, %xmm1, %xmm1
+; AVX512F-NEXT: vcmpordps %xmm0, %xmm0, %xmm0
+; AVX512F-NEXT: vptestmd %zmm0, %zmm0, %k1
+; AVX512F-NEXT: vmovups %zmm1, (%rdi) {%k1}
+; AVX512F-NEXT: vzeroupper
+; AVX512F-NEXT: retq
+;
+; AVX512VL-LABEL: maxnum_v4f32_masked_store:
+; AVX512VL: # %bb.0:
+; AVX512VL-NEXT: vmovups (%rdi), %xmm1
+; AVX512VL-NEXT: vmaxps %xmm0, %xmm1, %xmm1
+; AVX512VL-NEXT: vcmpordps %xmm0, %xmm0, %k1
+; AVX512VL-NEXT: vmovups %xmm1, (%rdi) {%k1}
+; AVX512VL-NEXT: retq
+ %b = load <4 x float>, ptr %ptr, align 4
+ %m = call <4 x float> @llvm.maxnum.v4f32(<4 x float> %a, <4 x float> %b)
+ store <4 x float> %m, ptr %ptr, align 4
+ ret void
+}
+
+define void @maxnum_v2f64_masked_store(<2 x double> %a, ptr %ptr) {
+; AVX512F-LABEL: maxnum_v2f64_masked_store:
+; AVX512F: # %bb.0:
+; AVX512F-NEXT: vmovupd (%rdi), %xmm1
+; AVX512F-NEXT: vmaxpd %xmm0, %xmm1, %xmm1
+; AVX512F-NEXT: vcmpordpd %xmm0, %xmm0, %xmm0
+; AVX512F-NEXT: vptestmq %zmm0, %zmm0, %k1
+; AVX512F-NEXT: vmovupd %zmm1, (%rdi) {%k1}
+; AVX512F-NEXT: vzeroupper
+; AVX512F-NEXT: retq
+;
+; AVX512VL-LABEL: maxnum_v2f64_masked_store:
+; AVX512VL: # %bb.0:
+; AVX512VL-NEXT: vmovupd (%rdi), %xmm1
+; AVX512VL-NEXT: vmaxpd %xmm0, %xmm1, %xmm1
+; AVX512VL-NEXT: vcmpordpd %xmm0, %xmm0, %k1
+; AVX512VL-NEXT: vmovupd %xmm1, (%rdi) {%k1}
+; AVX512VL-NEXT: retq
+ %b = load <2 x double>, ptr %ptr, align 8
+ %m = call <2 x double> @llvm.maxnum.v2f64(<2 x double> %a, <2 x double> %b)
+ store <2 x double> %m, ptr %ptr, align 8
+ ret void
+}
+
+define void @minnum_v4f32_masked_store(<4 x float> %a, ptr %ptr) {
+; AVX512F-LABEL: minnum_v4f32_masked_store:
+; AVX512F: # %bb.0:
+; AVX512F-NEXT: vmovups (%rdi), %xmm1
+; AVX512F-NEXT: vminps %xmm0, %xmm1, %xmm1
+; AVX512F-NEXT: vcmpordps %xmm0, %xmm0, %xmm0
+; AVX512F-NEXT: vptestmd %zmm0, %zmm0, %k1
+; AVX512F-NEXT: vmovups %zmm1, (%rdi) {%k1}
+; AVX512F-NEXT: vzeroupper
+; AVX512F-NEXT: retq
+;
+; AVX512VL-LABEL: minnum_v4f32_masked_store:
+; AVX512VL: # %bb.0:
+; AVX512VL-NEXT: vmovups (%rdi), %xmm1
+; AVX512VL-NEXT: vminps %xmm0, %xmm1, %xmm1
+; AVX512VL-NEXT: vcmpordps %xmm0, %xmm0, %k1
+; AVX512VL-NEXT: vmovups %xmm1, (%rdi) {%k1}
+; AVX512VL-NEXT: retq
+ %b = load <4 x float>, ptr %ptr, align 4
+ %m = call <4 x float> @llvm.minnum.v4f32(<4 x float> %a, <4 x float> %b)
+ store <4 x float> %m, ptr %ptr, align 4
+ ret void
+}
+
+define void @minnum_v2f64_masked_store(<2 x double> %a, ptr %ptr) {
+; AVX512F-LABEL: minnum_v2f64_masked_store:
+; AVX512F: # %bb.0:
+; AVX512F-NEXT: vmovupd (%rdi), %xmm1
+; AVX512F-NEXT: vminpd %xmm0, %xmm1, %xmm1
+; AVX512F-NEXT: vcmpordpd %xmm0, %xmm0, %xmm0
+; AVX512F-NEXT: vptestmq %zmm0, %zmm0, %k1
+; AVX512F-NEXT: vmovupd %zmm1, (%rdi) {%k1}
+; AVX512F-NEXT: vzeroupper
+; AVX512F-NEXT: retq
+;
+; AVX512VL-LABEL: minnum_v2f64_masked_store:
+; AVX512VL: # %bb.0:
+; AVX512VL-NEXT: vmovupd (%rdi), %xmm1
+; AVX512VL-NEXT: vminpd %xmm0, %xmm1, %xmm1
+; AVX512VL-NEXT: vcmpordpd %xmm0, %xmm0, %k1
+; AVX512VL-NEXT: vmovupd %xmm1, (%rdi) {%k1}
+; AVX512VL-NEXT: retq
+ %b = load <2 x double>, ptr %ptr, align 8
+ %m = call <2 x double> @llvm.minnum.v2f64(<2 x double> %a, <2 x double> %b)
+ store <2 x double> %m, ptr %ptr, align 8
+ ret void
+}
+
+; Explicit masked store using a setcc mask (reaches LowerMSTORE with an i1 mask).
+define void @explicit_masked_store_setcc(<4 x float> %a, <4 x float> %data, ptr %ptr) {
+; AVX512F-LABEL: explicit_masked_store_setcc:
+; AVX512F: # %bb.0:
+; AVX512F-NEXT: # kill: def $xmm1 killed $xmm1 def $zmm1
+; AVX512F-NEXT: # kill: def $xmm0 killed $xmm0 def $zmm0
+; AVX512F-NEXT: vcmpordps %zmm0, %zmm0, %k0
+; AVX512F-NEXT: kshiftlw $12, %k0, %k0
+; AVX512F-NEXT: kshiftrw $12, %k0, %k1
+; AVX512F-NEXT: vmovups %zmm1, (%rdi) {%k1}
+; AVX512F-NEXT: vzeroupper
+; AVX512F-NEXT: retq
+;
+; AVX512VL-LABEL: explicit_masked_store_setcc:
+; AVX512VL: # %bb.0:
+; AVX512VL-NEXT: vcmpordps %xmm0, %xmm0, %k1
+; AVX512VL-NEXT: vmovups %xmm1, (%rdi) {%k1}
+; AVX512VL-NEXT: retq
+ %cmp = fcmp ord <4 x float> %a, %a
+ call void @llvm.masked.store.v4f32.p0(<4 x float> %data, ptr %ptr, i32 4, <4 x i1> %cmp)
+ ret void
+}
+
+define void @v4i32_masked_store(<4 x i32> %a, <4 x i32> %data, ptr %ptr) {
+; AVX512F-LABEL: v4i32_masked_store:
+; AVX512F: # %bb.0:
+; AVX512F-NEXT: # kill: def $xmm1 killed $xmm1 def $zmm1
+; AVX512F-NEXT: # kill: def $xmm0 killed $xmm0 def $zmm0
+; AVX512F-NEXT: vptestnmd %zmm0, %zmm0, %k0
+; AVX512F-NEXT: kshiftlw $12, %k0, %k0
+; AVX512F-NEXT: kshiftrw $12, %k0, %k1
+; AVX512F-NEXT: vmovdqu32 %zmm1, (%rdi) {%k1}
+; AVX512F-NEXT: vzeroupper
+; AVX512F-NEXT: retq
+;
+; AVX512VL-LABEL: v4i32_masked_store:
+; AVX512VL: # %bb.0:
+; AVX512VL-NEXT: vptestnmd %xmm0, %xmm0, %k1
+; AVX512VL-NEXT: vmovdqu32 %xmm1, (%rdi) {%k1}
+; AVX512VL-NEXT: retq
+ %cmp = icmp eq <4 x i32> %a, zeroinitializer
+ call void @llvm.masked.store.v4i32.p0(<4 x i32> %data, ptr %ptr, i32 4, <4 x i1> %cmp)
+ ret void
+}
|
phoebewang
reviewed
Jun 5, 2026
| } | ||
|
|
||
| ; Explicit masked store using a setcc mask (reaches LowerMSTORE with an i1 mask). | ||
| define void @explicit_masked_store_setcc(<4 x float> %a, <4 x float> %data, ptr %ptr) { |
Contributor
There was a problem hiding this comment.
This passes without the change: https://godbolt.org/z/EYaebqxzq
Contributor
|
It seems it's caused by 1c0ac80d4a9e. Maybe change the code in foldToMaskedStore? We may also need to backport to LLVM22. |
Contributor
Author
You're right. I confirmed that. I'll try to fix in foldToMaskedStore and update PR. Thanks. |
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
arsenm
reviewed
Jun 5, 2026
Comment on lines
+2
to
+5
| ; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mattr=+avx | FileCheck %s --check-prefix=AVX | ||
| ; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mattr=+avx2 | FileCheck %s --check-prefix=AVX | ||
| ; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mattr=+avx512f | FileCheck %s --check-prefix=AVX512F | ||
| ; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mattr=+avx512f,+avx512vl | FileCheck %s --check-prefix=AVX512VL |
Contributor
Author
It's known issue:#201785, unrelated to my changes. |
RKSimon
approved these changes
Jun 5, 2026
…hout VLX
On AVX512F targets without VLX (e.g. KNL), masked stores are custom-lowered by
widening to 512-bit operations. A maxnum/minnum store-back is expanded using a
legacy packed (CMPP) vector comparison, and the store-back DAGCombine fuses that
vector comparison into the masked store. As a result LowerMSTORE receives a mask
with vNi32 elements instead of vNi1 and hits an assertion:
Assertion `Mask.getSimpleValueType().getScalarType() == MVT::i1 &&
"Unexpected mask type"' failed.
(reproduces with -mattr=+avx512f or -mcpu=knl; -mcpu=skylake-avx512 and
+avx512vl are unaffected because the mask is produced as a k-register.)
Accept a wide vNi32 mask in addition to vNi1: widen it in its own element type
and truncate to vNi1 before building the masked store. A CMPP result is
all-ones/all-zeros per lane, so bit 0 of each lane carries the predicate and the
zero-filled widened lanes truncate to 0 (disabled).
foldToMaskedStore (added in 1c0ac80) rewrites store(vselect(cond, x, load(ptr)), ptr) -> masked_store(x, ptr, cond) passing the vselect condition straight through as the store mask. A masked store follows the IR convention of a vXi1 mask, but the condition can be a wider boolean vector. On AVX512F targets without VLX, a maxnum/minnum store-back lowers the NaN test with a legacy packed (CMPP) comparison whose result is a vXi32/vXi64 vector, so the masked store is created with a wide mask and LowerMSTORE asserts: Assertion `Mask.getSimpleValueType().getScalarType() == MVT::i1 && "Unexpected mask type"' failed. When the matching vXi1 type is legal, narrow the mask to it before building the masked store. Targets where vXi1 is illegal (e.g. AVX/AVX2) keep the wide mask and continue to lower it as a blend/vmaskmov, and targets whose vselect condition is already vXi1 (e.g. AArch64 SVE, RISC-V RVV) are unaffected. This fixes the crash at the source and lets the X86 LowerMSTORE keep its invariant of only ever seeing a vXi1 mask (no target-specific workaround). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
155889c to
a9f8d06
Compare
Contributor
|
/cherry-pick e6bd788 |
Member
|
/pull-request #202880 |
dyung
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Jun 10, 2026
foldToMaskedStore (added in llvm@1c0ac80) rewrites store(vselect(cond, x, load(ptr)), ptr) -> masked_store(x, ptr, cond) passing the vselect condition straight through as the store mask. A masked store follows the IR convention of a vXi1 mask, but the condition can be a wider boolean vector. On AVX512F targets without VLX, a maxnum/minnum store-back lowers the NaN test with a legacy packed (CMPP) comparison whose result is a vXi32/vXi64 vector, so the masked store is created with a wide mask and LowerMSTORE asserts: Assertion `Mask.getSimpleValueType().getScalarType() == MVT::i1 && "Unexpected mask type"' failed. When the matching vXi1 type is legal, narrow the mask to it before building the masked store. Targets where vXi1 is illegal (e.g. AVX/AVX2) keep the wide mask and continue to lower it as a blend/vmaskmov, and targets whose vselect condition is already vXi1 (e.g. AArch64 SVE, RISC-V RVV) are unaffected. This fixes the crash at the source and lets the X86 LowerMSTORE keep its invariant of only ever seeing a vXi1 mask (no target-specific workaround). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit e6bd788)
carlobertolli
pushed a commit
to carlobertolli/llvm-project
that referenced
this pull request
Jun 11, 2026
foldToMaskedStore (added in llvm@1c0ac80) rewrites store(vselect(cond, x, load(ptr)), ptr) -> masked_store(x, ptr, cond) passing the vselect condition straight through as the store mask. A masked store follows the IR convention of a vXi1 mask, but the condition can be a wider boolean vector. On AVX512F targets without VLX, a maxnum/minnum store-back lowers the NaN test with a legacy packed (CMPP) comparison whose result is a vXi32/vXi64 vector, so the masked store is created with a wide mask and LowerMSTORE asserts: Assertion `Mask.getSimpleValueType().getScalarType() == MVT::i1 && "Unexpected mask type"' failed. When the matching vXi1 type is legal, narrow the mask to it before building the masked store. Targets where vXi1 is illegal (e.g. AVX/AVX2) keep the wide mask and continue to lower it as a blend/vmaskmov, and targets whose vselect condition is already vXi1 (e.g. AArch64 SVE, RISC-V RVV) are unaffected. This fixes the crash at the source and lets the X86 LowerMSTORE keep its invariant of only ever seeing a vXi1 mask (no target-specific workaround). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
daunabomba
pushed a commit
to daunabomba/llvm-project
that referenced
this pull request
Jun 17, 2026
foldToMaskedStore (added in llvm@1c0ac80) rewrites store(vselect(cond, x, load(ptr)), ptr) -> masked_store(x, ptr, cond) passing the vselect condition straight through as the store mask. A masked store follows the IR convention of a vXi1 mask, but the condition can be a wider boolean vector. On AVX512F targets without VLX, a maxnum/minnum store-back lowers the NaN test with a legacy packed (CMPP) comparison whose result is a vXi32/vXi64 vector, so the masked store is created with a wide mask and LowerMSTORE asserts: Assertion `Mask.getSimpleValueType().getScalarType() == MVT::i1 && "Unexpected mask type"' failed. When the matching vXi1 type is legal, narrow the mask to it before building the masked store. Targets where vXi1 is illegal (e.g. AVX/AVX2) keep the wide mask and continue to lower it as a blend/vmaskmov, and targets whose vselect condition is already vXi1 (e.g. AArch64 SVE, RISC-V RVV) are unaffected. This fixes the crash at the source and lets the X86 LowerMSTORE keep its invariant of only ever seeing a vXi1 mask (no target-specific workaround). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit e6bd788)
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
foldToMaskedStore (added in 1c0ac80) rewrites
store(vselect(cond, x, load(ptr)), ptr) -> masked_store(x, ptr, cond)
passing the vselect condition straight through as the store mask. A masked
store follows the IR convention of a vXi1 mask, but the condition can be a
wider boolean vector. On AVX512F targets without VLX, a maxnum/minnum
store-back lowers the NaN test with a legacy packed (CMPP) comparison whose
result is a vXi32/vXi64 vector, so the masked store is created with a wide
mask and LowerMSTORE asserts:
Assertion `Mask.getSimpleValueType().getScalarType() == MVT::i1 &&
"Unexpected mask type"' failed.
When the matching vXi1 type is legal, narrow the mask to it before building
the masked store. Targets where vXi1 is illegal (e.g. AVX/AVX2) keep the wide
mask and continue to lower it as a blend/vmaskmov, and targets whose vselect
condition is already vXi1 (e.g. AArch64 SVE, RISC-V RVV) are unaffected.
This fixes the crash at the source and lets the X86 LowerMSTORE keep its
invariant of only ever seeing a vXi1 mask (no target-specific workaround).
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com