Skip to content

Commit 0291f49

Browse files
authored
[EarlyCSE] Correcting assertion for DSE with invariant loads (llvm#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.
1 parent c4d0d95 commit 0291f49

File tree

2 files changed

+29
-10
lines changed

2 files changed

+29
-10
lines changed

llvm/lib/Transforms/Scalar/EarlyCSE.cpp

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1698,16 +1698,8 @@ bool EarlyCSE::processNode(DomTreeNode *Node) {
16981698
if (MemInst.isValid() && MemInst.isStore()) {
16991699
LoadValue InVal = AvailableLoads.lookup(MemInst.getPointerOperand());
17001700
if (InVal.DefInst &&
1701-
InVal.DefInst == getMatchingValue(InVal, MemInst, CurrentGeneration)) {
1702-
// It is okay to have a LastStore to a different pointer here if MemorySSA
1703-
// tells us that the load and store are from the same memory generation.
1704-
// In that case, LastStore should keep its present value since we're
1705-
// removing the current store.
1706-
assert((!LastStore ||
1707-
ParseMemoryInst(LastStore, TTI).getPointerOperand() ==
1708-
MemInst.getPointerOperand() ||
1709-
MSSA) &&
1710-
"can't have an intervening store if not using MemorySSA!");
1701+
InVal.DefInst ==
1702+
getMatchingValue(InVal, MemInst, CurrentGeneration)) {
17111703
LLVM_DEBUG(dbgs() << "EarlyCSE DSE (writeback): " << Inst << '\n');
17121704
if (!DebugCounter::shouldExecute(CSECounter)) {
17131705
LLVM_DEBUG(dbgs() << "Skipping due to debug counter\n");

llvm/test/Transforms/EarlyCSE/invariant-loads.ll

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
; RUN: opt -S -passes='early-cse<memssa>' --enable-knowledge-retention < %s | FileCheck %s --check-prefixes=CHECK,USE_ASSUME
55

66
declare void @clobber_and_use(i32)
7+
declare void @clobber_and_combine(i32, i32)
78

89
define void @f_0(ptr %ptr) {
910
; NO_ASSUME-LABEL: @f_0(
@@ -164,6 +165,32 @@ define void @test_dse1(ptr %p) {
164165
ret void
165166
}
166167

168+
; Extending @test_dse1, the call can't change the contents of p.invariant.
169+
; Moreover, the contents of p.invariant cannot change from the store to p.
170+
define void @test_dse2(ptr noalias %p, ptr noalias %p.invariant) {
171+
; NO_ASSUME-LABEL: @test_dse2(
172+
; NO_ASSUME-NEXT: [[V:%.*]] = load i32, ptr [[P:%.*]], align 4
173+
; NO_ASSUME-NEXT: [[V_INVARIANT:%.*]] = load i32, ptr [[P_INVARIANT:%.*]], align 4, !invariant.load !0
174+
; NO_ASSUME-NEXT: [[V_COMB:%.*]] = call i32 @clobber_and_combine(i32 [[V]], i32 [[V_INVARIANT]])
175+
; NO_ASSUME-NEXT: store i32 [[V_COMB]], ptr [[P]], align 4
176+
; NO_ASSUME-NEXT: ret void
177+
;
178+
; USE_ASSUME-LABEL: @test_dse2(
179+
; USE_ASSUME-NEXT: [[V:%.*]] = load i32, ptr [[P:%.*]], align 4
180+
; USE_ASSUME-NEXT: [[V_INVARIANT:%.*]] = load i32, ptr [[P_INVARIANT:%.*]], align 4, !invariant.load !0
181+
; USE_ASSUME-NEXT: [[V_COMB:%.*]] = call i32 @clobber_and_combine(i32 [[V]], i32 [[V_INVARIANT]])
182+
; USE_ASSUME-NEXT: store i32 [[V_COMB]], ptr [[P]], align 4
183+
; 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) ]
184+
; USE_ASSUME-NEXT: ret void
185+
;
186+
%v = load i32, ptr %p
187+
%v.invariant = load i32, ptr %p.invariant, !invariant.load !{}
188+
%v.comb = call i32 @clobber_and_combine(i32 %v, i32 %v.invariant)
189+
store i32 %v.comb, ptr %p
190+
store i32 %v.invariant, ptr %p.invariant
191+
ret void
192+
}
193+
167194
; By assumption, v1 must equal v2 (TODO)
168195
define void @test_false_negative_dse2(ptr %p, i32 %v2) {
169196
; CHECK-LABEL: @test_false_negative_dse2(

0 commit comments

Comments
 (0)