[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.

Author: nikic

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;
 

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]]
 ;