Skip to content

Commit c229f76

Browse files
authored
[DFAJumpThreading] Avoid exploring the paths that never come back (#85505)
This patch does: - Preserve loop info when unfolding selects. - Reduce the search space for loop paths.
1 parent 7152194 commit c229f76

File tree

1 file changed

+33
-9
lines changed

1 file changed

+33
-9
lines changed

llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ class SelectInstToUnfold {
131131
explicit operator bool() const { return SI && SIUse; }
132132
};
133133

134-
void unfold(DomTreeUpdater *DTU, SelectInstToUnfold SIToUnfold,
134+
void unfold(DomTreeUpdater *DTU, LoopInfo *LI, SelectInstToUnfold SIToUnfold,
135135
std::vector<SelectInstToUnfold> *NewSIsToUnfold,
136136
std::vector<BasicBlock *> *NewBBs);
137137

@@ -142,6 +142,7 @@ class DFAJumpThreading {
142142
: AC(AC), DT(DT), LI(LI), TTI(TTI), ORE(ORE) {}
143143

144144
bool run(Function &F);
145+
bool LoopInfoBroken;
145146

146147
private:
147148
void
@@ -157,7 +158,7 @@ class DFAJumpThreading {
157158

158159
std::vector<SelectInstToUnfold> NewSIsToUnfold;
159160
std::vector<BasicBlock *> NewBBs;
160-
unfold(&DTU, SIToUnfold, &NewSIsToUnfold, &NewBBs);
161+
unfold(&DTU, LI, SIToUnfold, &NewSIsToUnfold, &NewBBs);
161162

162163
// Put newly discovered select instructions into the work list.
163164
for (const SelectInstToUnfold &NewSIToUnfold : NewSIsToUnfold)
@@ -201,7 +202,7 @@ void createBasicBlockAndSinkSelectInst(
201202
/// created basic blocks into \p NewBBs.
202203
///
203204
/// TODO: merge it with CodeGenPrepare::optimizeSelectInst() if possible.
204-
void unfold(DomTreeUpdater *DTU, SelectInstToUnfold SIToUnfold,
205+
void unfold(DomTreeUpdater *DTU, LoopInfo *LI, SelectInstToUnfold SIToUnfold,
205206
std::vector<SelectInstToUnfold> *NewSIsToUnfold,
206207
std::vector<BasicBlock *> *NewBBs) {
207208
SelectInst *SI = SIToUnfold.getInst();
@@ -307,6 +308,12 @@ void unfold(DomTreeUpdater *DTU, SelectInstToUnfold SIToUnfold,
307308
DTU->applyUpdates({{DominatorTree::Insert, StartBlock, TT},
308309
{DominatorTree::Insert, StartBlock, FT}});
309310

311+
// Preserve loop info
312+
if (Loop *L = LI->getLoopFor(SI->getParent())) {
313+
for (BasicBlock *NewBB : *NewBBs)
314+
L->addBasicBlockToLoop(NewBB, *LI);
315+
}
316+
310317
// The select is now dead.
311318
assert(SI->use_empty() && "Select must be dead now");
312319
SI->eraseFromParent();
@@ -522,9 +529,10 @@ struct MainSwitch {
522529
};
523530

524531
struct AllSwitchPaths {
525-
AllSwitchPaths(const MainSwitch *MSwitch, OptimizationRemarkEmitter *ORE)
526-
: Switch(MSwitch->getInstr()), SwitchBlock(Switch->getParent()),
527-
ORE(ORE) {}
532+
AllSwitchPaths(const MainSwitch *MSwitch, OptimizationRemarkEmitter *ORE,
533+
LoopInfo *LI)
534+
: Switch(MSwitch->getInstr()), SwitchBlock(Switch->getParent()), ORE(ORE),
535+
LI(LI) {}
528536

529537
std::vector<ThreadingPath> &getThreadingPaths() { return TPaths; }
530538
unsigned getNumThreadingPaths() { return TPaths.size(); }
@@ -596,6 +604,12 @@ struct AllSwitchPaths {
596604

597605
Visited.insert(BB);
598606

607+
// Stop if we have reached the BB out of loop, since its successors have no
608+
// impact on the DFA.
609+
// TODO: Do we need to stop exploring if BB is the outer loop of the switch?
610+
if (!LI->getLoopFor(BB))
611+
return Res;
612+
599613
// Some blocks have multiple edges to the same successor, and this set
600614
// is used to prevent a duplicate path from being generated
601615
SmallSet<BasicBlock *, 4> Successors;
@@ -737,6 +751,7 @@ struct AllSwitchPaths {
737751
BasicBlock *SwitchBlock;
738752
OptimizationRemarkEmitter *ORE;
739753
std::vector<ThreadingPath> TPaths;
754+
LoopInfo *LI;
740755
};
741756

742757
struct TransformDFA {
@@ -1283,6 +1298,7 @@ bool DFAJumpThreading::run(Function &F) {
12831298

12841299
SmallVector<AllSwitchPaths, 2> ThreadableLoops;
12851300
bool MadeChanges = false;
1301+
LoopInfoBroken = false;
12861302

12871303
for (BasicBlock &BB : F) {
12881304
auto *SI = dyn_cast<SwitchInst>(BB.getTerminator());
@@ -1304,7 +1320,7 @@ bool DFAJumpThreading::run(Function &F) {
13041320
if (!Switch.getSelectInsts().empty())
13051321
MadeChanges = true;
13061322

1307-
AllSwitchPaths SwitchPaths(&Switch, ORE);
1323+
AllSwitchPaths SwitchPaths(&Switch, ORE, LI);
13081324
SwitchPaths.run();
13091325

13101326
if (SwitchPaths.getNumThreadingPaths() > 0) {
@@ -1315,10 +1331,15 @@ bool DFAJumpThreading::run(Function &F) {
13151331
// strict requirement but it can cause buggy behavior if there is an
13161332
// overlap of blocks in different opportunities. There is a lot of room to
13171333
// experiment with catching more opportunities here.
1334+
// NOTE: To release this contraint, we must handle LoopInfo invalidation
13181335
break;
13191336
}
13201337
}
13211338

1339+
#ifdef NDEBUG
1340+
LI->verify(*DT);
1341+
#endif
1342+
13221343
SmallPtrSet<const Value *, 32> EphValues;
13231344
if (ThreadableLoops.size() > 0)
13241345
CodeMetrics::collectEphemeralValues(&F, AC, EphValues);
@@ -1327,6 +1348,7 @@ bool DFAJumpThreading::run(Function &F) {
13271348
TransformDFA Transform(&SwitchPaths, DT, AC, TTI, ORE, EphValues);
13281349
Transform.run();
13291350
MadeChanges = true;
1351+
LoopInfoBroken = true;
13301352
}
13311353

13321354
#ifdef EXPENSIVE_CHECKS
@@ -1347,11 +1369,13 @@ PreservedAnalyses DFAJumpThreadingPass::run(Function &F,
13471369
LoopInfo &LI = AM.getResult<LoopAnalysis>(F);
13481370
TargetTransformInfo &TTI = AM.getResult<TargetIRAnalysis>(F);
13491371
OptimizationRemarkEmitter ORE(&F);
1350-
1351-
if (!DFAJumpThreading(&AC, &DT, &LI, &TTI, &ORE).run(F))
1372+
DFAJumpThreading ThreadImpl(&AC, &DT, &LI, &TTI, &ORE);
1373+
if (!ThreadImpl.run(F))
13521374
return PreservedAnalyses::all();
13531375

13541376
PreservedAnalyses PA;
13551377
PA.preserve<DominatorTreeAnalysis>();
1378+
if (!ThreadImpl.LoopInfoBroken)
1379+
PA.preserve<LoopAnalysis>();
13561380
return PA;
13571381
}

0 commit comments

Comments
 (0)