[llvm] [InstCombine] Fix miscompile when folding a select into a masked load (PR #216730)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 18 01:49:19 PDT 2026
https://github.com/Chennesxu updated https://github.com/llvm/llvm-project/pull/216730
>From 7d7c2aea9f09c1471cd66e2c51d949763b9effac Mon Sep 17 00:00:00 2001
From: Chennes <xuchen359 at gmail.com>
Date: Mon, 17 Aug 2026 20:31:28 +0800
Subject: [PATCH 1/3] [InstCombine][NFC] Precommit tests for
select-to-masked-load folding
Add baseline coverage for the fold introduced in eb8589987267. The checks
show that the replacement load is currently created at the select, moving
it past an aliasing store, and that the fold also fires when the new
passthrough is only available at the select.
Also cover dropping call-site attributes that no longer apply after
changing the passthrough. This is already handled correctly because the
fold creates a fresh call.
---
.../InstCombine/select-masked_load.ll | 42 +++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/llvm/test/Transforms/InstCombine/select-masked_load.ll b/llvm/test/Transforms/InstCombine/select-masked_load.ll
index cc6c48b29bf28..fcb25b8dd0640 100644
--- a/llvm/test/Transforms/InstCombine/select-masked_load.ll
+++ b/llvm/test/Transforms/InstCombine/select-masked_load.ll
@@ -169,6 +169,47 @@ define <vscale x 4 x i32> @fold_sel_into_masked_load_drop_metadata(ptr %loc, <vs
ret <vscale x 4 x i32> %sel
}
+; FIXME: The replacement load is created at the select, below the aliasing
+; store, so it reads the stored value.
+define <4 x float> @fold_sel_into_masked_load_aliasing_store(ptr %ptr, <4 x i1> %mask, <4 x float> %passthrough) {
+; CHECK-LABEL: @fold_sel_into_masked_load_aliasing_store(
+; CHECK-NEXT: store <4 x float> [[PASSTHROUGH:%.*]], ptr [[PTR:%.*]], align 16
+; CHECK-NEXT: [[SEL:%.*]] = call <4 x float> @llvm.masked.load.v4f32.p0(ptr nonnull align 4 [[PTR]], <4 x i1> [[MASK:%.*]], <4 x float> [[PASSTHROUGH]])
+; CHECK-NEXT: ret <4 x float> [[SEL]]
+;
+ %load = call <4 x float> @llvm.masked.load.v4f32.p0(ptr %ptr, i32 4, <4 x i1> %mask, <4 x float> zeroinitializer)
+ store <4 x float> %passthrough, ptr %ptr, align 16
+ %sel = select <4 x i1> %mask, <4 x float> %load, <4 x float> %passthrough
+ ret <4 x float> %sel
+}
+
+; The fold currently creates the replacement load at the select, where the
+; passthrough is available.
+define <4 x float> @neg_fold_sel_into_masked_load_passthrough_after_load(ptr %ptr, <4 x i1> %mask, <4 x float> %a) {
+; CHECK-LABEL: @neg_fold_sel_into_masked_load_passthrough_after_load(
+; CHECK-NEXT: [[PASSTHROUGH:%.*]] = fadd <4 x float> [[A:%.*]], [[A]]
+; CHECK-NEXT: [[SEL:%.*]] = call <4 x float> @llvm.masked.load.v4f32.p0(ptr align 4 [[PTR:%.*]], <4 x i1> [[MASK:%.*]], <4 x float> [[PASSTHROUGH]])
+; CHECK-NEXT: ret <4 x float> [[SEL]]
+;
+ %load = call <4 x float> @llvm.masked.load.v4f32.p0(ptr %ptr, i32 4, <4 x i1> %mask, <4 x float> zeroinitializer)
+ %passthrough = fadd <4 x float> %a, %a
+ %sel = select <4 x i1> %mask, <4 x float> %load, <4 x float> %passthrough
+ ret <4 x float> %sel
+}
+
+; Do not copy result or passthrough attributes (range/noundef) to the new load.
+; Use the current intrinsic form because auto-upgrading the legacy form drops
+; these attributes before InstCombine.
+define <8 x i16> @fold_sel_into_masked_load_drop_attrs(ptr %ptr, <8 x i1> %mask, <8 x i16> %passthrough) {
+; CHECK-LABEL: @fold_sel_into_masked_load_drop_attrs(
+; CHECK-NEXT: [[SEL:%.*]] = call <8 x i16> @llvm.masked.load.v8i16.p0(ptr align 2 [[PTR:%.*]], <8 x i1> [[MASK:%.*]], <8 x i16> [[PASSTHROUGH:%.*]])
+; CHECK-NEXT: ret <8 x i16> [[SEL]]
+;
+ %load = call range(i16 0, 2) <8 x i16> @llvm.masked.load.v8i16.p0(ptr align 2 %ptr, <8 x i1> %mask, <8 x i16> noundef zeroinitializer)
+ %sel = select <8 x i1> %mask, <8 x i16> %load, <8 x i16> %passthrough
+ ret <8 x i16> %sel
+}
+
!0 = !{!1, !1, i64 0}
!1 = !{!"int", !2, i64 0}
!2 = !{!"omnipotent char", !8, i64 0}
@@ -184,3 +225,4 @@ define <vscale x 4 x i32> @fold_sel_into_masked_load_drop_metadata(ptr %loc, <vs
declare <8 x float> @llvm.masked.load.v8f32.p0(ptr, i32 immarg, <8 x i1>, <8 x float>)
declare <4 x i32> @llvm.masked.load.v4i32.p0(ptr, i32 immarg, <4 x i1>, <4 x i32>)
declare <4 x float> @llvm.masked.load.v4f32.p0(ptr, i32 immarg, <4 x i1>, <4 x float>)
+declare <8 x i16> @llvm.masked.load.v8i16.p0(ptr, <8 x i1>, <8 x i16>)
>From f91312cd9c872804ac305ff30d94665f395c2f4a Mon Sep 17 00:00:00 2001
From: Chennes <xuchen359 at gmail.com>
Date: Mon, 17 Aug 2026 20:33:55 +0800
Subject: [PATCH 2/3] [InstCombine] Fix miscompile when folding a select into a
masked load
visitSelectInst folds select(mask, masked.load(ptr, mask, PT), FV) into
masked.load(ptr, mask, FV). The replacement load was created at the select,
effectively moving the memory access past any intervening instructions.
If one of them writes the loaded memory, the new load reads the updated
value instead of the original one. This was also reported downstream as
ispc/ispc#3891.
Create the replacement load at the original load's position and require
FV to be available there. Otherwise, leave the select unchanged.
Changing the insertion point picks up the original load's debug location,
so explicitly restore the location of the select being replaced.
Fixes #215453
---
.../InstCombine/InstCombineSelect.cpp | 20 ++++++++++++++-----
.../InstCombine/select-masked_load.ll | 13 ++++++------
2 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
index 558ad2ebccc37..75346989cb27c 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -5379,11 +5379,21 @@ Instruction *InstCombinerImpl::visitSelectInst(SelectInst &SI) {
if (match(TrueVal, m_OneUse(m_MaskedLoad(m_Value(MaskedLoadPtr),
m_Specific(CondVal), m_Value())))) {
auto *LoadInst = cast<IntrinsicInst>(TrueVal);
- Instruction *In = Builder.CreateMaskedLoad(
- TrueVal->getType(), MaskedLoadPtr,
- LoadInst->getParamAlign(0).valueOrOne(), CondVal, FalseVal);
- In->setAAMetadata(LoadInst->getAAMetadata());
- return replaceInstUsesWith(SI, In);
+ // Keep the load at its original position to avoid crossing writes. The new
+ // passthrough must therefore be available there.
+ // TODO: Sink the load when the passthrough is unavailable but no
+ // intervening instruction can modify memory.
+ if (DT.dominates(FalseVal, LoadInst)) {
+ Builder.SetInsertPoint(LoadInst);
+ // SetInsertPoint() took the debug location from the old load, but the new
+ // load replaces the select, so restore the select's location.
+ Builder.SetCurrentDebugLocation(SI.getDebugLoc());
+ Instruction *In = Builder.CreateMaskedLoad(
+ TrueVal->getType(), MaskedLoadPtr,
+ LoadInst->getParamAlign(0).valueOrOne(), CondVal, FalseVal);
+ In->setAAMetadata(LoadInst->getAAMetadata());
+ return replaceInstUsesWith(SI, In);
+ }
}
// Canonicalize sign function ashr pattern: select (icmp slt X, 1), ashr X,
diff --git a/llvm/test/Transforms/InstCombine/select-masked_load.ll b/llvm/test/Transforms/InstCombine/select-masked_load.ll
index fcb25b8dd0640..f0c2e8c8845cf 100644
--- a/llvm/test/Transforms/InstCombine/select-masked_load.ll
+++ b/llvm/test/Transforms/InstCombine/select-masked_load.ll
@@ -169,12 +169,11 @@ define <vscale x 4 x i32> @fold_sel_into_masked_load_drop_metadata(ptr %loc, <vs
ret <vscale x 4 x i32> %sel
}
-; FIXME: The replacement load is created at the select, below the aliasing
-; store, so it reads the stored value.
+; Keep the folded load before an intervening aliasing store.
define <4 x float> @fold_sel_into_masked_load_aliasing_store(ptr %ptr, <4 x i1> %mask, <4 x float> %passthrough) {
; CHECK-LABEL: @fold_sel_into_masked_load_aliasing_store(
-; CHECK-NEXT: store <4 x float> [[PASSTHROUGH:%.*]], ptr [[PTR:%.*]], align 16
-; CHECK-NEXT: [[SEL:%.*]] = call <4 x float> @llvm.masked.load.v4f32.p0(ptr nonnull align 4 [[PTR]], <4 x i1> [[MASK:%.*]], <4 x float> [[PASSTHROUGH]])
+; CHECK-NEXT: [[SEL:%.*]] = call <4 x float> @llvm.masked.load.v4f32.p0(ptr align 4 [[PTR:%.*]], <4 x i1> [[MASK:%.*]], <4 x float> [[PASSTHROUGH:%.*]])
+; CHECK-NEXT: store <4 x float> [[PASSTHROUGH]], ptr [[PTR]], align 16
; CHECK-NEXT: ret <4 x float> [[SEL]]
;
%load = call <4 x float> @llvm.masked.load.v4f32.p0(ptr %ptr, i32 4, <4 x i1> %mask, <4 x float> zeroinitializer)
@@ -183,12 +182,12 @@ define <4 x float> @fold_sel_into_masked_load_aliasing_store(ptr %ptr, <4 x i1>
ret <4 x float> %sel
}
-; The fold currently creates the replacement load at the select, where the
-; passthrough is available.
+; Do not fold when the new passthrough is unavailable at the old load.
define <4 x float> @neg_fold_sel_into_masked_load_passthrough_after_load(ptr %ptr, <4 x i1> %mask, <4 x float> %a) {
; CHECK-LABEL: @neg_fold_sel_into_masked_load_passthrough_after_load(
+; CHECK-NEXT: [[LOAD:%.*]] = call <4 x float> @llvm.masked.load.v4f32.p0(ptr align 4 [[PTR:%.*]], <4 x i1> [[MASK:%.*]], <4 x float> zeroinitializer)
; CHECK-NEXT: [[PASSTHROUGH:%.*]] = fadd <4 x float> [[A:%.*]], [[A]]
-; CHECK-NEXT: [[SEL:%.*]] = call <4 x float> @llvm.masked.load.v4f32.p0(ptr align 4 [[PTR:%.*]], <4 x i1> [[MASK:%.*]], <4 x float> [[PASSTHROUGH]])
+; CHECK-NEXT: [[SEL:%.*]] = select <4 x i1> [[MASK]], <4 x float> [[LOAD]], <4 x float> [[PASSTHROUGH]]
; CHECK-NEXT: ret <4 x float> [[SEL]]
;
%load = call <4 x float> @llvm.masked.load.v4f32.p0(ptr %ptr, i32 4, <4 x i1> %mask, <4 x float> zeroinitializer)
>From c5dc25a28ab5c8eba863f8fe43eead8ed87e2ab3 Mon Sep 17 00:00:00 2001
From: Chennes <xuchen359 at gmail.com>
Date: Tue, 18 Aug 2026 16:42:34 +0800
Subject: [PATCH 3/3] [InstCombine] Keep the original load's debug location
SetInsertPoint() already takes the debug location from the original load, so
do not restore the select's location. Also remove the speculative TODO.
---
llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp | 5 -----
1 file changed, 5 deletions(-)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
index 75346989cb27c..090abfaea28ec 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -5381,13 +5381,8 @@ Instruction *InstCombinerImpl::visitSelectInst(SelectInst &SI) {
auto *LoadInst = cast<IntrinsicInst>(TrueVal);
// Keep the load at its original position to avoid crossing writes. The new
// passthrough must therefore be available there.
- // TODO: Sink the load when the passthrough is unavailable but no
- // intervening instruction can modify memory.
if (DT.dominates(FalseVal, LoadInst)) {
Builder.SetInsertPoint(LoadInst);
- // SetInsertPoint() took the debug location from the old load, but the new
- // load replaces the select, so restore the select's location.
- Builder.SetCurrentDebugLocation(SI.getDebugLoc());
Instruction *In = Builder.CreateMaskedLoad(
TrueVal->getType(), MaskedLoadPtr,
LoadInst->getParamAlign(0).valueOrOne(), CondVal, FalseVal);
More information about the llvm-commits
mailing list