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

[Nikita Popov via llvm-commits]

https://github.com/nikic created https://github.com/llvm/llvm-project/pull/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.

>From 25f16d19655db76cb5cdab82941103b326928602 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Mon, 23 Jun 2025 10:05:29 +0200
Subject: [PATCH] [EarlyCSE] Fix dead store elimination for unwinding readnone
 calls

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.
---
 llvm/lib/Transforms/Scalar/EarlyCSE.cpp       | 10 ++++++---
 llvm/test/Transforms/EarlyCSE/basic.ll        | 21 ++++++++++++++++---
 .../Transforms/EarlyCSE/readnone-mayunwind.ll | 18 ----------------
 3 files changed, 25 insertions(+), 24 deletions(-)
 delete mode 100644 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
-}