[llvm] 0291f49 - [EarlyCSE] Correcting assertion for DSE with invariant loads (#141495)

[via llvm-commits]

Author: zGoldthorpe
Date: 2025-05-28T12:29:34+02:00
New Revision: 0291f495dbea64231212a8d51ecef653e10aed33

URL: https://github.com/llvm/llvm-project/commit/0291f495dbea64231212a8d51ecef653e10aed33
DIFF: https://github.com/llvm/llvm-project/commit/0291f495dbea64231212a8d51ecef653e10aed33.diff

LOG: [EarlyCSE] Correcting assertion for DSE with invariant loads (#141495)

This patch corrects an assertion to handle an edge case where there is a
dead store into an `!invariant.load`'s pointer, but there is an
interleaving store to a different (non-aliasing) address.

In this case we know that the interleaved store cannot modify the
address even without MemorySSA, so the assert does not hold.

This bug was found through the AMD fuzzing project.

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/EarlyCSE.cpp
    llvm/test/Transforms/EarlyCSE/invariant-loads.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
index 1e378369166c0..09cb2f4cb0104 100644
--- a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
+++ b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
@@ -1698,16 +1698,8 @@ bool EarlyCSE::processNode(DomTreeNode *Node) {
     if (MemInst.isValid() && MemInst.isStore()) {
       LoadValue InVal = AvailableLoads.lookup(MemInst.getPointerOperand());
       if (InVal.DefInst &&
-          InVal.DefInst == getMatchingValue(InVal, MemInst, CurrentGeneration)) {
-        // It is okay to have a LastStore to a 
diff erent pointer here if MemorySSA
-        // tells us that the load and store are from the same memory generation.
-        // In that case, LastStore should keep its present value since we're
-        // removing the current store.
-        assert((!LastStore ||
-                ParseMemoryInst(LastStore, TTI).getPointerOperand() ==
-                    MemInst.getPointerOperand() ||
-                MSSA) &&
-               "can't have an intervening store if not using MemorySSA!");
+          InVal.DefInst ==
+              getMatchingValue(InVal, MemInst, CurrentGeneration)) {
         LLVM_DEBUG(dbgs() << "EarlyCSE DSE (writeback): " << Inst << '\n');
         if (!DebugCounter::shouldExecute(CSECounter)) {
           LLVM_DEBUG(dbgs() << "Skipping due to debug counter\n");

diff  --git a/llvm/test/Transforms/EarlyCSE/invariant-loads.ll b/llvm/test/Transforms/EarlyCSE/invariant-loads.ll
index 88454660711ed..96a8fc3ddc00a 100644
--- a/llvm/test/Transforms/EarlyCSE/invariant-loads.ll
+++ b/llvm/test/Transforms/EarlyCSE/invariant-loads.ll
@@ -4,6 +4,7 @@
 ; RUN: opt -S -passes='early-cse<memssa>' --enable-knowledge-retention < %s | FileCheck %s --check-prefixes=CHECK,USE_ASSUME
 
 declare void @clobber_and_use(i32)
+declare void @clobber_and_combine(i32, i32)
 
 define void @f_0(ptr %ptr) {
 ; NO_ASSUME-LABEL: @f_0(
@@ -164,6 +165,32 @@ define void @test_dse1(ptr %p) {
   ret void
 }
 
+; Extending @test_dse1, the call can't change the contents of p.invariant.
+; Moreover, the contents of p.invariant cannot change from the store to p.
+define void @test_dse2(ptr noalias %p, ptr noalias %p.invariant) {
+; NO_ASSUME-LABEL: @test_dse2(
+; NO_ASSUME-NEXT:    [[V:%.*]] = load i32, ptr [[P:%.*]], align 4
+; NO_ASSUME-NEXT:    [[V_INVARIANT:%.*]] = load i32, ptr [[P_INVARIANT:%.*]], align 4, !invariant.load !0
+; NO_ASSUME-NEXT:    [[V_COMB:%.*]] = call i32 @clobber_and_combine(i32 [[V]], i32 [[V_INVARIANT]])
+; NO_ASSUME-NEXT:    store i32 [[V_COMB]], ptr [[P]], align 4
+; NO_ASSUME-NEXT:    ret void
+;
+; USE_ASSUME-LABEL: @test_dse2(
+; USE_ASSUME-NEXT:    [[V:%.*]] = load i32, ptr [[P:%.*]], align 4
+; USE_ASSUME-NEXT:    [[V_INVARIANT:%.*]] = load i32, ptr [[P_INVARIANT:%.*]], align 4, !invariant.load !0
+; USE_ASSUME-NEXT:    [[V_COMB:%.*]] = call i32 @clobber_and_combine(i32 [[V]], i32 [[V_INVARIANT]])
+; USE_ASSUME-NEXT:    store i32 [[V_COMB]], ptr [[P]], align 4
+; USE_ASSUME-NEXT:    call void @llvm.assume(i1 true) [ "dereferenceable"(ptr [[P_INVARIANT]], i64 4), "nonnull"(ptr [[P_INVARIANT]]), "align"(ptr [[P_INVARIANT]], i64 4) ]
+; USE_ASSUME-NEXT:    ret void
+;
+  %v = load i32, ptr %p
+  %v.invariant = load i32, ptr %p.invariant, !invariant.load !{}
+  %v.comb = call i32 @clobber_and_combine(i32 %v, i32 %v.invariant)
+  store i32 %v.comb, ptr %p
+  store i32 %v.invariant, ptr %p.invariant
+  ret void
+}
+
 ; By assumption, v1 must equal v2 (TODO)
 define void @test_false_negative_dse2(ptr %p, i32 %v2) {
 ; CHECK-LABEL: @test_false_negative_dse2(