Skip to content

Commit 72f9f06

Browse files
author
Djordje Todorovic
committed
Revert "[LICM] Hoist LOAD without sinking the STORE"
This reverts commit ecb9d8e. I'll reland this as soon as the failing tests are fixed/updated.
1 parent ecb9d8e commit 72f9f06

File tree

8 files changed

+21
-124
lines changed

8 files changed

+21
-124
lines changed

llvm/include/llvm/Transforms/Utils/SSAUpdater.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -169,10 +169,6 @@ class LoadAndStorePromoter {
169169

170170
/// Called to update debug info associated with the instruction.
171171
virtual void updateDebugInfo(Instruction *I) const {}
172-
173-
/// Return false if a sub-class wants to keep one of the loads/stores
174-
/// after the SSA construction.
175-
virtual bool shouldDelete(Instruction *I) const { return true; }
176172
};
177173

178174
} // end namespace llvm

llvm/lib/Transforms/Scalar/LICM.cpp

Lines changed: 9 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1860,7 +1860,6 @@ class LoopPromoter : public LoadAndStorePromoter {
18601860
bool UnorderedAtomic;
18611861
AAMDNodes AATags;
18621862
ICFLoopSafetyInfo &SafetyInfo;
1863-
bool CanInsertStoresInExitBlocks;
18641863

18651864
// We're about to add a use of V in a loop exit block. Insert an LCSSA phi
18661865
// (if legal) if doing so would add an out-of-loop use to an instruction
@@ -1887,13 +1886,12 @@ class LoopPromoter : public LoadAndStorePromoter {
18871886
SmallVectorImpl<MemoryAccess *> &MSSAIP, PredIteratorCache &PIC,
18881887
MemorySSAUpdater *MSSAU, LoopInfo &li, DebugLoc dl,
18891888
Align Alignment, bool UnorderedAtomic, const AAMDNodes &AATags,
1890-
ICFLoopSafetyInfo &SafetyInfo, bool CanInsertStoresInExitBlocks)
1889+
ICFLoopSafetyInfo &SafetyInfo)
18911890
: LoadAndStorePromoter(Insts, S), SomePtr(SP), PointerMustAliases(PMA),
18921891
LoopExitBlocks(LEB), LoopInsertPts(LIP), MSSAInsertPts(MSSAIP),
18931892
PredCache(PIC), MSSAU(MSSAU), LI(li), DL(std::move(dl)),
18941893
Alignment(Alignment), UnorderedAtomic(UnorderedAtomic), AATags(AATags),
1895-
SafetyInfo(SafetyInfo),
1896-
CanInsertStoresInExitBlocks(CanInsertStoresInExitBlocks) {}
1894+
SafetyInfo(SafetyInfo) {}
18971895

18981896
bool isInstInList(Instruction *I,
18991897
const SmallVectorImpl<Instruction *> &) const override {
@@ -1905,7 +1903,7 @@ class LoopPromoter : public LoadAndStorePromoter {
19051903
return PointerMustAliases.count(Ptr);
19061904
}
19071905

1908-
void insertStoresInLoopExitBlocks() {
1906+
void doExtraRewritesBeforeFinalDeletion() override {
19091907
// Insert stores after in the loop exit blocks. Each exit block gets a
19101908
// store of the live-out values that feed them. Since we've already told
19111909
// the SSA updater about the defs in the loop and the preheader
@@ -1939,21 +1937,10 @@ class LoopPromoter : public LoadAndStorePromoter {
19391937
}
19401938
}
19411939

1942-
void doExtraRewritesBeforeFinalDeletion() override {
1943-
if (CanInsertStoresInExitBlocks)
1944-
insertStoresInLoopExitBlocks();
1945-
}
1946-
19471940
void instructionDeleted(Instruction *I) const override {
19481941
SafetyInfo.removeInstruction(I);
19491942
MSSAU->removeMemoryAccess(I);
19501943
}
1951-
1952-
bool shouldDelete(Instruction *I) const override {
1953-
if (isa<StoreInst>(I))
1954-
return CanInsertStoresInExitBlocks;
1955-
return true;
1956-
}
19571944
};
19581945

19591946
bool isNotCapturedBeforeOrInLoop(const Value *V, const Loop *L,
@@ -2052,7 +2039,6 @@ bool llvm::promoteLoopAccessesToScalars(
20522039

20532040
bool DereferenceableInPH = false;
20542041
bool SafeToInsertStore = false;
2055-
bool FoundLoadToPromote = false;
20562042

20572043
SmallVector<Instruction *, 64> LoopUses;
20582044

@@ -2100,7 +2086,6 @@ bool llvm::promoteLoopAccessesToScalars(
21002086

21012087
SawUnorderedAtomic |= Load->isAtomic();
21022088
SawNotAtomic |= !Load->isAtomic();
2103-
FoundLoadToPromote = true;
21042089

21052090
Align InstAlignment = Load->getAlign();
21062091

@@ -2212,20 +2197,13 @@ bool llvm::promoteLoopAccessesToScalars(
22122197
}
22132198
}
22142199

2215-
// If we've still failed to prove we can sink the store, hoist the load
2216-
// only, if possible.
2217-
if (!SafeToInsertStore && !FoundLoadToPromote)
2218-
// If we cannot hoist the load either, give up.
2200+
// If we've still failed to prove we can sink the store, give up.
2201+
if (!SafeToInsertStore)
22192202
return false;
22202203

2221-
// Lets do the promotion!
2222-
if (SafeToInsertStore)
2223-
LLVM_DEBUG(dbgs() << "LICM: Promoting load/store of the value: " << *SomePtr
2224-
<< '\n');
2225-
else
2226-
LLVM_DEBUG(dbgs() << "LICM: Promoting load of the value: " << *SomePtr
2227-
<< '\n');
2228-
2204+
// Otherwise, this is safe to promote, lets do it!
2205+
LLVM_DEBUG(dbgs() << "LICM: Promoting value stored to in loop: " << *SomePtr
2206+
<< '\n');
22292207
ORE->emit([&]() {
22302208
return OptimizationRemark(DEBUG_TYPE, "PromoteLoopAccessesToScalar",
22312209
LoopUses[0])
@@ -2244,8 +2222,7 @@ bool llvm::promoteLoopAccessesToScalars(
22442222
SSAUpdater SSA(&NewPHIs);
22452223
LoopPromoter Promoter(SomePtr, LoopUses, SSA, PointerMustAliases, ExitBlocks,
22462224
InsertPts, MSSAInsertPts, PIC, MSSAU, *LI, DL,
2247-
Alignment, SawUnorderedAtomic, AATags, *SafetyInfo,
2248-
SafeToInsertStore);
2225+
Alignment, SawUnorderedAtomic, AATags, *SafetyInfo);
22492226

22502227
// Set up the preheader to have a definition of the value. It is the live-out
22512228
// value from the preheader that uses in the loop will use.

llvm/lib/Transforms/Utils/SSAUpdater.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -446,9 +446,6 @@ void LoadAndStorePromoter::run(const SmallVectorImpl<Instruction *> &Insts) {
446446
// Now that everything is rewritten, delete the old instructions from the
447447
// function. They should all be dead now.
448448
for (Instruction *User : Insts) {
449-
if (!shouldDelete(User))
450-
continue;
451-
452449
// If this is a load that still has uses, then the load must have been added
453450
// as a live value in the SSAUpdate data structure for a block (e.g. because
454451
// the loaded value was stored later). In this case, we need to recursively

llvm/test/Transforms/InstMerge/st_sink_bugfix_22613.ll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@ target triple = "x86_64-unknown-linux-gnu"
55
; RUN: opt -O2 -S < %s | FileCheck %s
66

77
; CHECK-LABEL: main
8-
; CHECK: memset
9-
; CHECK: if.then
10-
; CHECK: store
118
; CHECK: if.end
129
; CHECK: store
10+
; CHECK: memset
11+
; CHECK: if.then
1312
; CHECK: store
13+
; CHECK: memset
1414

1515
@d = common global i32 0, align 4
1616
@b = common global i32 0, align 4

llvm/test/Transforms/LICM/hoist-load-without-store.ll

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

llvm/test/Transforms/LICM/promote-capture.ll

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,19 +111,17 @@ define void @test_captured_before_loop(i32 %len) {
111111
; CHECK-NEXT: [[COUNT:%.*]] = alloca i32, align 4
112112
; CHECK-NEXT: store i32 0, i32* [[COUNT]], align 4
113113
; CHECK-NEXT: call void @capture(i32* [[COUNT]])
114-
; CHECK-NEXT: [[COUNT_PROMOTED:%.*]] = load i32, i32* [[COUNT]], align 4
115114
; CHECK-NEXT: br label [[LOOP:%.*]]
116115
; CHECK: loop:
117-
; CHECK-NEXT: [[C_INC2:%.*]] = phi i32 [ [[COUNT_PROMOTED]], [[ENTRY:%.*]] ], [ [[C_INC1:%.*]], [[LATCH:%.*]] ]
118-
; CHECK-NEXT: [[I:%.*]] = phi i32 [ 0, [[ENTRY]] ], [ [[I_NEXT:%.*]], [[LATCH]] ]
116+
; CHECK-NEXT: [[I:%.*]] = phi i32 [ 0, [[ENTRY:%.*]] ], [ [[I_NEXT:%.*]], [[LATCH:%.*]] ]
119117
; CHECK-NEXT: [[COND:%.*]] = call i1 @cond(i32 [[I]])
120118
; CHECK-NEXT: br i1 [[COND]], label [[IF:%.*]], label [[LATCH]]
121119
; CHECK: if:
122-
; CHECK-NEXT: [[C_INC:%.*]] = add i32 [[C_INC2]], 1
120+
; CHECK-NEXT: [[C:%.*]] = load i32, i32* [[COUNT]], align 4
121+
; CHECK-NEXT: [[C_INC:%.*]] = add i32 [[C]], 1
123122
; CHECK-NEXT: store i32 [[C_INC]], i32* [[COUNT]], align 4
124123
; CHECK-NEXT: br label [[LATCH]]
125124
; CHECK: latch:
126-
; CHECK-NEXT: [[C_INC1]] = phi i32 [ [[C_INC]], [[IF]] ], [ [[C_INC2]], [[LOOP]] ]
127125
; CHECK-NEXT: [[I_NEXT]] = add nuw i32 [[I]], 1
128126
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[I_NEXT]], [[LEN:%.*]]
129127
; CHECK-NEXT: br i1 [[CMP]], label [[EXIT:%.*]], label [[LOOP]]

llvm/test/Transforms/LICM/scalar-promote-memmodel.ll

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,19 @@ define void @bar(i32 %n, i32 %b) nounwind uwtable ssp {
1111
; CHECK-LABEL: @bar(
1212
; CHECK-NEXT: entry:
1313
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i32 [[B:%.*]], 0
14-
; CHECK-NEXT: [[G_PROMOTED:%.*]] = load i32, i32* @g, align 4
1514
; CHECK-NEXT: br label [[FOR_COND:%.*]]
1615
; CHECK: for.cond:
17-
; CHECK-NEXT: [[INC2:%.*]] = phi i32 [ [[G_PROMOTED]], [[ENTRY:%.*]] ], [ [[INC1:%.*]], [[FOR_INC:%.*]] ]
18-
; CHECK-NEXT: [[I_0:%.*]] = phi i32 [ 0, [[ENTRY]] ], [ [[INC5:%.*]], [[FOR_INC]] ]
16+
; CHECK-NEXT: [[I_0:%.*]] = phi i32 [ 0, [[ENTRY:%.*]] ], [ [[INC5:%.*]], [[FOR_INC:%.*]] ]
1917
; CHECK-NEXT: [[CMP:%.*]] = icmp slt i32 [[I_0]], [[N:%.*]]
2018
; CHECK-NEXT: br i1 [[CMP]], label [[FOR_BODY:%.*]], label [[FOR_END:%.*]]
2119
; CHECK: for.body:
2220
; CHECK-NEXT: br i1 [[TOBOOL]], label [[FOR_INC]], label [[IF_THEN:%.*]]
2321
; CHECK: if.then:
24-
; CHECK-NEXT: [[INC:%.*]] = add nsw i32 [[INC2]], 1
22+
; CHECK-NEXT: [[TMP3:%.*]] = load i32, i32* @g, align 4
23+
; CHECK-NEXT: [[INC:%.*]] = add nsw i32 [[TMP3]], 1
2524
; CHECK-NEXT: store i32 [[INC]], i32* @g, align 4
2625
; CHECK-NEXT: br label [[FOR_INC]]
2726
; CHECK: for.inc:
28-
; CHECK-NEXT: [[INC1]] = phi i32 [ [[INC]], [[IF_THEN]] ], [ [[INC2]], [[FOR_BODY]] ]
2927
; CHECK-NEXT: [[INC5]] = add nsw i32 [[I_0]], 1
3028
; CHECK-NEXT: br label [[FOR_COND]]
3129
; CHECK: for.end:

llvm/test/Transforms/LICM/scalar-promote.ll

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -315,19 +315,17 @@ define i32 @test7bad() {
315315
; CHECK-NEXT: entry:
316316
; CHECK-NEXT: [[LOCAL:%.*]] = alloca i32, align 4
317317
; CHECK-NEXT: call void @capture(i32* [[LOCAL]])
318-
; CHECK-NEXT: [[LOCAL_PROMOTED:%.*]] = load i32, i32* [[LOCAL]], align 4
319318
; CHECK-NEXT: br label [[LOOP:%.*]]
320319
; CHECK: loop:
321-
; CHECK-NEXT: [[X22:%.*]] = phi i32 [ [[LOCAL_PROMOTED]], [[ENTRY:%.*]] ], [ [[X21:%.*]], [[ELSE:%.*]] ]
322-
; CHECK-NEXT: [[J:%.*]] = phi i32 [ 0, [[ENTRY]] ], [ [[NEXT:%.*]], [[ELSE]] ]
323-
; CHECK-NEXT: [[X2:%.*]] = call i32 @opaque(i32 [[X22]])
320+
; CHECK-NEXT: [[J:%.*]] = phi i32 [ 0, [[ENTRY:%.*]] ], [ [[NEXT:%.*]], [[ELSE:%.*]] ]
321+
; CHECK-NEXT: [[X:%.*]] = load i32, i32* [[LOCAL]], align 4
322+
; CHECK-NEXT: [[X2:%.*]] = call i32 @opaque(i32 [[X]])
324323
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[X2]], 0
325324
; CHECK-NEXT: br i1 [[CMP]], label [[IF:%.*]], label [[ELSE]]
326325
; CHECK: if:
327326
; CHECK-NEXT: store i32 [[X2]], i32* [[LOCAL]], align 4
328327
; CHECK-NEXT: br label [[ELSE]]
329328
; CHECK: else:
330-
; CHECK-NEXT: [[X21]] = phi i32 [ [[X2]], [[IF]] ], [ [[X22]], [[LOOP]] ]
331329
; CHECK-NEXT: [[NEXT]] = add i32 [[J]], 1
332330
; CHECK-NEXT: [[COND:%.*]] = icmp eq i32 [[NEXT]], 0
333331
; CHECK-NEXT: br i1 [[COND]], label [[EXIT:%.*]], label [[LOOP]]

0 commit comments

Comments
 (0)