[llvm-branch-commits] [llvm] release/23.x: [InstCombine] Fix miscompile when folding a select into a masked load (#216730) (PR #216989)

[via llvm-branch-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: Nikita Popov (nikic)

<details>
<summary>Changes</summary>

Backport fix for #<!-- -->215453.

(cherry picked from commit fec2cbad15395d9d971e5a81e660e5ab604da4e8)

---
Full diff: https://github.com/llvm/llvm-project/pull/216989.diff


2 Files Affected:

- (modified) llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp (+11-6) 
- (modified) llvm/test/Transforms/InstCombine/select-masked_load.ll (+41) 


``````````diff
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
index f107c15304d9f..da691791c0fb3 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -5306,12 +5306,17 @@ Instruction *InstCombinerImpl::visitSelectInst(SelectInst &SI) {
 
   Value *MaskedLoadPtr;
   if (match(TrueVal, m_OneUse(m_MaskedLoad(m_Value(MaskedLoadPtr),
-                                           m_Specific(CondVal), m_Value()))))
-    return replaceInstUsesWith(
-        SI, Builder.CreateMaskedLoad(
-                TrueVal->getType(), MaskedLoadPtr,
-                cast<IntrinsicInst>(TrueVal)->getParamAlign(0).valueOrOne(),
-                CondVal, FalseVal));
+    auto *LoadInst = cast<IntrinsicInst>(TrueVal);
+    // Keep the load at its original position to avoid crossing writes. The new
+    // passthrough must therefore be available there.
+    if (DT.dominates(FalseVal, LoadInst)) {
+      Builder.SetInsertPoint(LoadInst);
+      Instruction *In = Builder.CreateMaskedLoad(
+          TrueVal->getType(), MaskedLoadPtr,
+          LoadInst->getParamAlign(0).valueOrOne(), CondVal, FalseVal);
+      return replaceInstUsesWith(SI, In);
+    }
+  }
 
   // Canonicalize sign function ashr pattern: select (icmp slt X, 1), ashr X,
   // bitwidth-1, 1 -> scmp(X, 0)
diff --git a/llvm/test/Transforms/InstCombine/select-masked_load.ll b/llvm/test/Transforms/InstCombine/select-masked_load.ll
index f7584c41ce641..a196c56bcac5e 100644
--- a/llvm/test/Transforms/InstCombine/select-masked_load.ll
+++ b/llvm/test/Transforms/InstCombine/select-masked_load.ll
@@ -149,6 +149,47 @@ define <vscale x 4 x float> @fold_sel_into_masked_load_scalable_one_use_check(pt
   ret <vscale x 4 x float> %sel
 }
 
+; 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:    [[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)
+  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
+}
+
+; 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:%.*]] = 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)
+  %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
+}
+
 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>)

``````````

</details>


https://github.com/llvm/llvm-project/pull/216989