Skip to content

Commit 36eeb25

Browse files
LU-JOHNLU-JOHN
committed
Extend Sink pass to allow loads to be sunk to non-immediate successor blocks
Signed-off-by: John Lu <[email protected]>
1 parent 41c97af commit 36eeb25

File tree

2 files changed

+204
-24
lines changed

2 files changed

+204
-24
lines changed

‎llvm/lib/Transforms/Scalar/Sink.cpp‎

Lines changed: 52 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -27,43 +27,71 @@ using namespace llvm;
2727
STATISTIC(NumSunk, "Number of instructions sunk");
2828
STATISTIC(NumSinkIter, "Number of sinking iterations");
2929

30-
static bool isSafeToMove(Instruction *Inst, AliasAnalysis &AA,
31-
SmallPtrSetImpl<Instruction *> &Stores) {
32-
33-
if (Inst->mayWriteToMemory()) {
34-
Stores.insert(Inst);
35-
return false;
36-
}
37-
30+
static bool hasStoreConflict(Instruction *Inst, AliasAnalysis &AA,
31+
SmallPtrSetImpl<Instruction *> &Stores) {
3832
if (LoadInst *L = dyn_cast<LoadInst>(Inst)) {
3933
MemoryLocation Loc = MemoryLocation::get(L);
4034
for (Instruction *S : Stores)
4135
if (isModSet(AA.getModRefInfo(S, Loc)))
42-
return false;
36+
return true;
37+
} else if (auto *Call = dyn_cast<CallBase>(Inst)) {
38+
for (Instruction *S : Stores)
39+
if (isModSet(AA.getModRefInfo(S, Call)))
40+
return true;
4341
}
42+
return false;
43+
}
4444

45+
static bool isSafeToMove(Instruction *Inst, AliasAnalysis &AA,
46+
SmallPtrSetImpl<Instruction *> &Stores) {
47+
if (Inst->mayWriteToMemory()) {
48+
Stores.insert(Inst);
49+
return false;
50+
}
4551
if (Inst->isTerminator() || isa<PHINode>(Inst) || Inst->isEHPad() ||
4652
Inst->mayThrow() || !Inst->willReturn())
4753
return false;
48-
49-
if (auto *Call = dyn_cast<CallBase>(Inst)) {
50-
// Convergent operations cannot be made control-dependent on additional
51-
// values.
54+
// Convergent operations cannot be made control-dependent on additional
55+
// values.
56+
if (auto *Call = dyn_cast<CallBase>(Inst))
5257
if (Call->isConvergent())
5358
return false;
59+
if (hasStoreConflict(Inst, AA, Stores))
60+
return false;
61+
return true;
62+
}
5463

55-
for (Instruction *S : Stores)
56-
if (isModSet(AA.getModRefInfo(S, Call)))
57-
return false;
58-
}
64+
typedef SmallPtrSet<BasicBlock *, 8> BlocksSet;
65+
static void findStores(SmallPtrSetImpl<Instruction *> &Stores,
66+
BasicBlock *LoadBB, BasicBlock *BB,
67+
BlocksSet &VisitedBlocksSet) {
68+
if (BB == LoadBB || VisitedBlocksSet.contains(BB))
69+
return;
70+
VisitedBlocksSet.insert(BB);
71+
72+
for (Instruction &Inst : *BB)
73+
if (Inst.mayWriteToMemory())
74+
Stores.insert(&Inst);
75+
for (BasicBlock *Pred : predecessors(BB))
76+
findStores(Stores, LoadBB, Pred, VisitedBlocksSet);
77+
}
5978

60-
return true;
79+
static bool hasConflictingStoreBeforeSuccToSinkTo(AliasAnalysis &AA,
80+
Instruction *ReadMemInst,
81+
BasicBlock *SuccToSinkTo) {
82+
BlocksSet VisitedBlocksSet;
83+
SmallPtrSet<Instruction *, 8> Stores;
84+
BasicBlock *LoadBB = ReadMemInst->getParent();
85+
for (BasicBlock *Pred : predecessors(SuccToSinkTo))
86+
findStores(Stores, LoadBB, Pred, VisitedBlocksSet);
87+
return hasStoreConflict(ReadMemInst, AA, Stores);
6188
}
6289

6390
/// IsAcceptableTarget - Return true if it is possible to sink the instruction
6491
/// in the specified basic block.
65-
static bool IsAcceptableTarget(Instruction *Inst, BasicBlock *SuccToSinkTo,
66-
DominatorTree &DT, LoopInfo &LI) {
92+
static bool IsAcceptableTarget(AliasAnalysis &AA, Instruction *Inst,
93+
BasicBlock *SuccToSinkTo, DominatorTree &DT,
94+
LoopInfo &LI) {
6795
assert(Inst && "Instruction to be sunk is null");
6896
assert(SuccToSinkTo && "Candidate sink target is null");
6997

@@ -76,10 +104,10 @@ static bool IsAcceptableTarget(Instruction *Inst, BasicBlock *SuccToSinkTo,
76104
// just punt.
77105
// FIXME: Split critical edges if not backedges.
78106
if (SuccToSinkTo->getUniquePredecessor() != Inst->getParent()) {
79-
// We cannot sink a load across a critical edge - there may be stores in
80-
// other code paths.
107+
// Ensure that there is no conflicting store on any path to SuccToSinkTo.
81108
if (Inst->mayReadFromMemory() &&
82-
!Inst->hasMetadata(LLVMContext::MD_invariant_load))
109+
!Inst->hasMetadata(LLVMContext::MD_invariant_load) &&
110+
hasConflictingStoreBeforeSuccToSinkTo(AA, Inst, SuccToSinkTo))
83111
return false;
84112

85113
// We don't want to sink across a critical edge if we don't dominate the
@@ -153,7 +181,7 @@ static bool SinkInstruction(Instruction *Inst,
153181
// The nearest common dominator may be in a parent loop of BB, which may not
154182
// be beneficial. Find an ancestor.
155183
while (SuccToSinkTo != BB &&
156-
!IsAcceptableTarget(Inst, SuccToSinkTo, DT, LI))
184+
!IsAcceptableTarget(AA, Inst, SuccToSinkTo, DT, LI))
157185
SuccToSinkTo = DT.getNode(SuccToSinkTo)->getIDom()->getBlock();
158186
if (SuccToSinkTo == BB)
159187
SuccToSinkTo = nullptr;

‎llvm/test/Transforms/Sink/loadsink.ll‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
2+
; RUN: opt -S < %s -passes=sink | FileCheck %s
3+
4+
; Test that loads can be sunk to a non-immediate successor block by analyzing
5+
; paths for conflicting stores.
6+
7+
declare void @readfunc() readonly willreturn
8+
declare void @maywritefunc() willreturn
9+
10+
; Load can be sunk to non-immediate successor
11+
define void @load_can_sink(i1 %condA, i1 %condB, ptr %a, ptr %b) {
12+
; CHECK-LABEL: @load_can_sink(
13+
; CHECK-NEXT: entry:
14+
; CHECK-NEXT: br label [[MERGEA:%.*]]
15+
; CHECK: mergeA:
16+
; CHECK-NEXT: br i1 [[CONDA:%.*]], label [[THENA:%.*]], label [[MERGEB:%.*]]
17+
; CHECK: thenA:
18+
; CHECK-NEXT: call void @readfunc()
19+
; CHECK-NEXT: br label [[MERGEB]]
20+
; CHECK: mergeB:
21+
; CHECK-NEXT: br i1 [[CONDB:%.*]], label [[THENB:%.*]], label [[MERGEC:%.*]]
22+
; CHECK: thenB:
23+
; CHECK-NEXT: [[VALUE:%.*]] = load i32, ptr [[A:%.*]], align 4
24+
; CHECK-NEXT: store i32 [[VALUE]], ptr [[B:%.*]], align 4
25+
; CHECK-NEXT: br label [[MERGEC]]
26+
; CHECK: mergeC:
27+
; CHECK-NEXT: ret void
28+
;
29+
entry:
30+
%value = load i32, ptr %a, align 4
31+
br label %mergeA
32+
mergeA:
33+
br i1 %condA, label %thenA, label %mergeB
34+
thenA:
35+
call void @readfunc()
36+
br label %mergeB
37+
mergeB:
38+
br i1 %condB, label %thenB, label %mergeC
39+
thenB:
40+
store i32 %value, ptr %b
41+
br label %mergeC
42+
mergeC:
43+
ret void
44+
}
45+
46+
; Call may store so load cannot be sunk
47+
define void @load_cannot_sink(i1 %condA, i1 %condB, ptr %a, ptr %b) {
48+
; CHECK-LABEL: @load_cannot_sink(
49+
; CHECK-NEXT: entry:
50+
; CHECK-NEXT: br label [[MERGEA:%.*]]
51+
; CHECK: mergeA:
52+
; CHECK-NEXT: [[VALUE:%.*]] = load i32, ptr [[A:%.*]], align 4
53+
; CHECK-NEXT: br i1 [[CONDA:%.*]], label [[THENA:%.*]], label [[MERGEB:%.*]]
54+
; CHECK: thenA:
55+
; CHECK-NEXT: call void @maywritefunc()
56+
; CHECK-NEXT: br label [[MERGEB]]
57+
; CHECK: mergeB:
58+
; CHECK-NEXT: br i1 [[CONDB:%.*]], label [[THENB:%.*]], label [[MERGEC:%.*]]
59+
; CHECK: thenB:
60+
; CHECK-NEXT: store i32 [[VALUE]], ptr [[B:%.*]], align 4
61+
; CHECK-NEXT: br label [[MERGEC]]
62+
; CHECK: mergeC:
63+
; CHECK-NEXT: ret void
64+
;
65+
entry:
66+
%value = load i32, ptr %a, align 4
67+
br label %mergeA
68+
mergeA:
69+
br i1 %condA, label %thenA, label %mergeB
70+
thenA:
71+
call void @maywritefunc()
72+
br label %mergeB
73+
mergeB:
74+
br i1 %condB, label %thenB, label %mergeC
75+
thenB:
76+
store i32 %value, ptr %b
77+
br label %mergeC
78+
mergeC:
79+
ret void
80+
}
81+
82+
; Load can be sunk to non-immediate successor because load ptr is noalias
83+
define void @load_can_sink_noalias(i1 %condA, i1 %condB, ptr noalias %a, ptr %b) {
84+
; CHECK-LABEL: @load_can_sink_noalias(
85+
; CHECK-NEXT: entry:
86+
; CHECK-NEXT: br label [[MERGEA:%.*]]
87+
; CHECK: mergeA:
88+
; CHECK-NEXT: br i1 [[CONDA:%.*]], label [[THENA:%.*]], label [[MERGEB:%.*]]
89+
; CHECK: thenA:
90+
; CHECK-NEXT: store i32 0, ptr [[B:%.*]], align 4
91+
; CHECK-NEXT: br label [[MERGEB]]
92+
; CHECK: mergeB:
93+
; CHECK-NEXT: br i1 [[CONDB:%.*]], label [[THENB:%.*]], label [[MERGEC:%.*]]
94+
; CHECK: thenB:
95+
; CHECK-NEXT: [[VALUE:%.*]] = load i32, ptr [[A:%.*]], align 4
96+
; CHECK-NEXT: store i32 [[VALUE]], ptr [[B]], align 4
97+
; CHECK-NEXT: br label [[MERGEC]]
98+
; CHECK: mergeC:
99+
; CHECK-NEXT: ret void
100+
;
101+
entry:
102+
%value = load i32, ptr %a, align 4
103+
br label %mergeA
104+
mergeA:
105+
br i1 %condA, label %thenA, label %mergeB
106+
thenA:
107+
store i32 0, ptr %b
108+
br label %mergeB
109+
mergeB:
110+
br i1 %condB, label %thenB, label %mergeC
111+
thenB:
112+
store i32 %value, ptr %b
113+
br label %mergeC
114+
mergeC:
115+
ret void
116+
}
117+
118+
; Load cannot be sunk to non-immediate successor because load ptr may alias
119+
define void @load_cannot_sink_alias(i1 %condA, i1 %condB, ptr %a, ptr %b) {
120+
; CHECK-LABEL: @load_cannot_sink_alias(
121+
; CHECK-NEXT: entry:
122+
; CHECK-NEXT: br label [[MERGEA:%.*]]
123+
; CHECK: mergeA:
124+
; CHECK-NEXT: [[VALUE:%.*]] = load i32, ptr [[A:%.*]], align 4
125+
; CHECK-NEXT: br i1 [[CONDA:%.*]], label [[THENA:%.*]], label [[MERGEB:%.*]]
126+
; CHECK: thenA:
127+
; CHECK-NEXT: store i32 0, ptr [[B:%.*]], align 4
128+
; CHECK-NEXT: br label [[MERGEB]]
129+
; CHECK: mergeB:
130+
; CHECK-NEXT: br i1 [[CONDB:%.*]], label [[THENB:%.*]], label [[MERGEC:%.*]]
131+
; CHECK: thenB:
132+
; CHECK-NEXT: store i32 [[VALUE]], ptr [[B]], align 4
133+
; CHECK-NEXT: br label [[MERGEC]]
134+
; CHECK: mergeC:
135+
; CHECK-NEXT: ret void
136+
;
137+
entry:
138+
%value = load i32, ptr %a, align 4
139+
br label %mergeA
140+
mergeA:
141+
br i1 %condA, label %thenA, label %mergeB
142+
thenA:
143+
store i32 0, ptr %b
144+
br label %mergeB
145+
mergeB:
146+
br i1 %condB, label %thenB, label %mergeC
147+
thenB:
148+
store i32 %value, ptr %b
149+
br label %mergeC
150+
mergeC:
151+
ret void
152+
}

0 commit comments

Comments
 (0)