Skip to content

Commit 25f16d1

Browse files
committed
[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.
1 parent 4795b2b commit 25f16d1

File tree

3 files changed

+25
-24
lines changed

3 files changed

+25
-24
lines changed

llvm/lib/Transforms/Scalar/EarlyCSE.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1525,6 +1525,11 @@ bool EarlyCSE::processNode(DomTreeNode *Node) {
15251525
}
15261526
}
15271527

1528+
// Make sure stores prior to a potential unwind are not removed, as the
1529+
// caller may read the memory.
1530+
if (Inst.mayThrow())
1531+
LastStore = nullptr;
1532+
15281533
// If this is a simple instruction that we can value number, process it.
15291534
if (SimpleValue::canHandle(&Inst)) {
15301535
if ([[maybe_unused]] auto *CI = dyn_cast<ConstrainedFPIntrinsic>(&Inst)) {
@@ -1616,13 +1621,12 @@ bool EarlyCSE::processNode(DomTreeNode *Node) {
16161621
continue;
16171622
}
16181623

1619-
// If this instruction may read from memory or throw (and potentially read
1620-
// from memory in the exception handler), forget LastStore. Load/store
1624+
// If this instruction may read from memory, forget LastStore. Load/store
16211625
// intrinsics will indicate both a read and a write to memory. The target
16221626
// may override this (e.g. so that a store intrinsic does not read from
16231627
// memory, and thus will be treated the same as a regular store for
16241628
// commoning purposes).
1625-
if ((Inst.mayReadFromMemory() || Inst.mayThrow()) &&
1629+
if (Inst.mayReadFromMemory() &&
16261630
!(MemInst.isValid() && !MemInst.mayReadFromMemory()))
16271631
LastStore = nullptr;
16281632

llvm/test/Transforms/EarlyCSE/basic.ll

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ declare i32 @func(ptr%P) readonly
137137
;; Simple call CSE'ing.
138138
define i32 @test5(ptr%P) {
139139
; CHECK-LABEL: @test5(
140-
; CHECK-NEXT: [[V1:%.*]] = call i32 @func(ptr [[P:%.*]]), !prof !0
140+
; CHECK-NEXT: [[V1:%.*]] = call i32 @func(ptr [[P:%.*]]), !prof [[PROF0:![0-9]+]]
141141
; CHECK-NEXT: ret i32 0
142142
;
143143
%V1 = call i32 @func(ptr %P), !prof !0
@@ -212,10 +212,25 @@ define i32 @test9(ptr%P) {
212212
ret i32 %V1
213213
}
214214

215-
;; Trivial DSE can be performed across a readnone call.
215+
;; Trivial DSE can be performed across a readnone nounwind call.
216216
define i32 @test10(ptr%P) {
217217
; CHECK-LABEL: @test10(
218-
; CHECK-NEXT: [[V1:%.*]] = call i32 @func(ptr [[P:%.*]]) #[[ATTR2]]
218+
; CHECK-NEXT: [[V1:%.*]] = call i32 @func(ptr [[P:%.*]]) #[[ATTR3:[0-9]+]]
219+
; CHECK-NEXT: store i32 5, ptr [[P]], align 4
220+
; CHECK-NEXT: ret i32 [[V1]]
221+
;
222+
store i32 4, ptr %P
223+
%V1 = call i32 @func(ptr %P) readnone nounwind
224+
store i32 5, ptr %P
225+
ret i32 %V1
226+
}
227+
228+
; Trivial DSE can't be performed across a potentially unwinding readnone
229+
; call, as the caller may read the memory on unwind.
230+
define i32 @test_readnone_missing_nounwind(ptr %P) {
231+
; CHECK-LABEL: @test_readnone_missing_nounwind(
232+
; CHECK-NEXT: store i32 4, ptr [[P:%.*]], align 4
233+
; CHECK-NEXT: [[V1:%.*]] = call i32 @func(ptr [[P]]) #[[ATTR2]]
219234
; CHECK-NEXT: store i32 5, ptr [[P]], align 4
220235
; CHECK-NEXT: ret i32 [[V1]]
221236
;

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

Lines changed: 0 additions & 18 deletions
This file was deleted.

0 commit comments

Comments
 (0)