[llvm] cfcb788 - [EarlyCSE] Fix dead store elimination for unwinding readnone calls (#145287)

[via llvm-commits]

Author: Nikita Popov
Date: 2025-06-23T12:44:14+02:00
New Revision: cfcb7888c71c7b5468e3fc55b6de0804403dc3fe

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

LOG: [EarlyCSE] Fix dead store elimination for unwinding readnone calls (#145287)

EarlyCSE already resets LastStore when it hits an potentially unwinding
instruction, as the memory state may be observed by the caller after the
unwind.

There also was a test specifically making sure that this works even for
unwinding readnone calls -- however, the call in that test did not
participate in EarlyCSE in the first place, because it returns void
(relaxing that is how I got here), so it was not actually testing the
right thing.

Move the check for unwinding instructions earlier, so it also handles
the readnone case.

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/EarlyCSE.cpp
    llvm/test/Transforms/EarlyCSE/basic.ll

Removed: 
    llvm/test/Transforms/EarlyCSE/readnone-mayunwind.ll


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
index 5c62a2cf526e9..e1e283f171d38 100644
--- a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
+++ b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
@@ -1525,6 +1525,11 @@ bool EarlyCSE::processNode(DomTreeNode *Node) {
       }
     }
 
+    // Make sure stores prior to a potential unwind are not removed, as the
+    // caller may read the memory.
+    if (Inst.mayThrow())
+      LastStore = nullptr;
+
     // If this is a simple instruction that we can value number, process it.
     if (SimpleValue::canHandle(&Inst)) {
       if ([[maybe_unused]] auto *CI = dyn_cast<ConstrainedFPIntrinsic>(&Inst)) {
@@ -1616,13 +1621,12 @@ bool EarlyCSE::processNode(DomTreeNode *Node) {
       continue;
     }
 
-    // If this instruction may read from memory or throw (and potentially read
-    // from memory in the exception handler), forget LastStore.  Load/store
+    // If this instruction may read from memory, forget LastStore.  Load/store
     // intrinsics will indicate both a read and a write to memory.  The target
     // may override this (e.g. so that a store intrinsic does not read from
     // memory, and thus will be treated the same as a regular store for
     // commoning purposes).
-    if ((Inst.mayReadFromMemory() || Inst.mayThrow()) &&
+    if (Inst.mayReadFromMemory() &&
         !(MemInst.isValid() && !MemInst.mayReadFromMemory()))
       LastStore = nullptr;
 

diff  --git a/llvm/test/Transforms/EarlyCSE/basic.ll b/llvm/test/Transforms/EarlyCSE/basic.ll
index c6b746026c94d..2c6b2a9613924 100644
--- a/llvm/test/Transforms/EarlyCSE/basic.ll
+++ b/llvm/test/Transforms/EarlyCSE/basic.ll
@@ -137,7 +137,7 @@ declare i32 @func(ptr%P) readonly
 ;; Simple call CSE'ing.
 define i32 @test5(ptr%P) {
 ; CHECK-LABEL: @test5(
-; CHECK-NEXT:    [[V1:%.*]] = call i32 @func(ptr [[P:%.*]]), !prof !0
+; CHECK-NEXT:    [[V1:%.*]] = call i32 @func(ptr [[P:%.*]]), !prof [[PROF0:![0-9]+]]
 ; CHECK-NEXT:    ret i32 0
 ;
   %V1 = call i32 @func(ptr %P), !prof !0
@@ -212,10 +212,25 @@ define i32 @test9(ptr%P) {
   ret i32 %V1
 }
 
-;; Trivial DSE can be performed across a readnone call.
+;; Trivial DSE can be performed across a readnone nounwind call.
 define i32 @test10(ptr%P) {
 ; CHECK-LABEL: @test10(
-; CHECK-NEXT:    [[V1:%.*]] = call i32 @func(ptr [[P:%.*]]) #[[ATTR2]]
+; CHECK-NEXT:    [[V1:%.*]] = call i32 @func(ptr [[P:%.*]]) #[[ATTR3:[0-9]+]]
+; CHECK-NEXT:    store i32 5, ptr [[P]], align 4
+; CHECK-NEXT:    ret i32 [[V1]]
+;
+  store i32 4, ptr %P
+  %V1 = call i32 @func(ptr %P) readnone nounwind
+  store i32 5, ptr %P
+  ret i32 %V1
+}
+
+; Trivial DSE can't be performed across a potentially unwinding readnone
+; call, as the caller may read the memory on unwind.
+define i32 @test_readnone_missing_nounwind(ptr %P) {
+; CHECK-LABEL: @test_readnone_missing_nounwind(
+; CHECK-NEXT:    store i32 4, ptr [[P:%.*]], align 4
+; CHECK-NEXT:    [[V1:%.*]] = call i32 @func(ptr [[P]]) #[[ATTR2]]
 ; CHECK-NEXT:    store i32 5, ptr [[P]], align 4
 ; CHECK-NEXT:    ret i32 [[V1]]
 ;

diff  --git a/llvm/test/Transforms/EarlyCSE/readnone-mayunwind.ll b/llvm/test/Transforms/EarlyCSE/readnone-mayunwind.ll
deleted file mode 100644
index e4d31f31d9ff7..0000000000000
--- a/llvm/test/Transforms/EarlyCSE/readnone-mayunwind.ll
+++ /dev/null
@@ -1,18 +0,0 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -S -passes=early-cse -earlycse-debug-hash < %s | FileCheck %s
-
-declare void @readnone_may_unwind() readnone
-
-define void @f(ptr %ptr) {
-; CHECK-LABEL: @f(
-; CHECK-NEXT:    store i32 100, ptr [[PTR:%.*]], align 4
-; CHECK-NEXT:    call void @readnone_may_unwind()
-; CHECK-NEXT:    store i32 200, ptr [[PTR]], align 4
-; CHECK-NEXT:    ret void
-;
-
-  store i32 100, ptr %ptr
-  call void @readnone_may_unwind()
-  store i32 200, ptr %ptr
-  ret void
-}