From 35bc0a33c893c6199cf513a19eb7ce1d4064482f Mon Sep 17 00:00:00 2001 From: XChy Date: Sat, 16 Mar 2024 15:41:46 +0800 Subject: [PATCH 1/2] [DFAJumpThreading] Avoid exploring the paths that never come back --- .../Transforms/Scalar/DFAJumpThreading.cpp | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp b/llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp index 1caed93b1b668..80889a590726b 100644 --- a/llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp +++ b/llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp @@ -131,7 +131,7 @@ class SelectInstToUnfold { explicit operator bool() const { return SI && SIUse; } }; -void unfold(DomTreeUpdater *DTU, SelectInstToUnfold SIToUnfold, +void unfold(DomTreeUpdater *DTU, LoopInfo *LI, SelectInstToUnfold SIToUnfold, std::vector *NewSIsToUnfold, std::vector *NewBBs); @@ -157,7 +157,7 @@ class DFAJumpThreading { std::vector NewSIsToUnfold; std::vector NewBBs; - unfold(&DTU, SIToUnfold, &NewSIsToUnfold, &NewBBs); + unfold(&DTU, LI, SIToUnfold, &NewSIsToUnfold, &NewBBs); // Put newly discovered select instructions into the work list. for (const SelectInstToUnfold &NewSIToUnfold : NewSIsToUnfold) @@ -201,7 +201,7 @@ void createBasicBlockAndSinkSelectInst( /// created basic blocks into \p NewBBs. /// /// TODO: merge it with CodeGenPrepare::optimizeSelectInst() if possible. -void unfold(DomTreeUpdater *DTU, SelectInstToUnfold SIToUnfold, +void unfold(DomTreeUpdater *DTU, LoopInfo *LI, SelectInstToUnfold SIToUnfold, std::vector *NewSIsToUnfold, std::vector *NewBBs) { SelectInst *SI = SIToUnfold.getInst(); @@ -307,6 +307,12 @@ void unfold(DomTreeUpdater *DTU, SelectInstToUnfold SIToUnfold, DTU->applyUpdates({{DominatorTree::Insert, StartBlock, TT}, {DominatorTree::Insert, StartBlock, FT}}); + // Preserve loop info + if (Loop *L = LI->getLoopFor(SI->getParent())) { + for (BasicBlock *NewBB : *NewBBs) + L->addBasicBlockToLoop(NewBB, *LI); + } + // The select is now dead. assert(SI->use_empty() && "Select must be dead now"); SI->eraseFromParent(); @@ -522,9 +528,10 @@ struct MainSwitch { }; struct AllSwitchPaths { - AllSwitchPaths(const MainSwitch *MSwitch, OptimizationRemarkEmitter *ORE) - : Switch(MSwitch->getInstr()), SwitchBlock(Switch->getParent()), - ORE(ORE) {} + AllSwitchPaths(const MainSwitch *MSwitch, OptimizationRemarkEmitter *ORE, + LoopInfo *LI) + : Switch(MSwitch->getInstr()), SwitchBlock(Switch->getParent()), ORE(ORE), + LI(LI) {} std::vector &getThreadingPaths() { return TPaths; } unsigned getNumThreadingPaths() { return TPaths.size(); } @@ -596,6 +603,12 @@ struct AllSwitchPaths { Visited.insert(BB); + // Stop if we have reached the BB out of loop, since its successors have no + // impact on the DFA. + // TODO: Do we need to stop exploring if BB is the outer loop of the switch? + if (!LI->getLoopFor(BB)) + return Res; + // Some blocks have multiple edges to the same successor, and this set // is used to prevent a duplicate path from being generated SmallSet Successors; @@ -737,6 +750,7 @@ struct AllSwitchPaths { BasicBlock *SwitchBlock; OptimizationRemarkEmitter *ORE; std::vector TPaths; + LoopInfo *LI; }; struct TransformDFA { @@ -1304,7 +1318,7 @@ bool DFAJumpThreading::run(Function &F) { if (!Switch.getSelectInsts().empty()) MadeChanges = true; - AllSwitchPaths SwitchPaths(&Switch, ORE); + AllSwitchPaths SwitchPaths(&Switch, ORE, LI); SwitchPaths.run(); if (SwitchPaths.getNumThreadingPaths() > 0) { From 94cbd8918f0826eaf9f3e84d0992486f2a239300 Mon Sep 17 00:00:00 2001 From: XChy Date: Sat, 20 Apr 2024 23:32:12 +0800 Subject: [PATCH 2/2] [DFAJumpThreading] Add checks about LoopInfo --- llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp b/llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp index 80889a590726b..14cdcc956aa5d 100644 --- a/llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp +++ b/llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp @@ -142,6 +142,7 @@ class DFAJumpThreading { : AC(AC), DT(DT), LI(LI), TTI(TTI), ORE(ORE) {} bool run(Function &F); + bool LoopInfoBroken; private: void @@ -1297,6 +1298,7 @@ bool DFAJumpThreading::run(Function &F) { SmallVector ThreadableLoops; bool MadeChanges = false; + LoopInfoBroken = false; for (BasicBlock &BB : F) { auto *SI = dyn_cast(BB.getTerminator()); @@ -1329,10 +1331,15 @@ bool DFAJumpThreading::run(Function &F) { // strict requirement but it can cause buggy behavior if there is an // overlap of blocks in different opportunities. There is a lot of room to // experiment with catching more opportunities here. + // NOTE: To release this contraint, we must handle LoopInfo invalidation break; } } +#ifdef NDEBUG + LI->verify(*DT); +#endif + SmallPtrSet EphValues; if (ThreadableLoops.size() > 0) CodeMetrics::collectEphemeralValues(&F, AC, EphValues); @@ -1341,6 +1348,7 @@ bool DFAJumpThreading::run(Function &F) { TransformDFA Transform(&SwitchPaths, DT, AC, TTI, ORE, EphValues); Transform.run(); MadeChanges = true; + LoopInfoBroken = true; } #ifdef EXPENSIVE_CHECKS @@ -1361,11 +1369,13 @@ PreservedAnalyses DFAJumpThreadingPass::run(Function &F, LoopInfo &LI = AM.getResult(F); TargetTransformInfo &TTI = AM.getResult(F); OptimizationRemarkEmitter ORE(&F); - - if (!DFAJumpThreading(&AC, &DT, &LI, &TTI, &ORE).run(F)) + DFAJumpThreading ThreadImpl(&AC, &DT, &LI, &TTI, &ORE); + if (!ThreadImpl.run(F)) return PreservedAnalyses::all(); PreservedAnalyses PA; PA.preserve(); + if (!ThreadImpl.LoopInfoBroken) + PA.preserve(); return PA; }