From 44faf7f407072d245fd98bcb92644b77469dbfb2 Mon Sep 17 00:00:00 2001 From: Davide Italiano Date: Mon, 13 Jun 2016 22:27:30 +0000 Subject: [PATCH] [PM/MergeLoadStoreMotion] Convert the logic to static functions. Pass AliasAnalyis and MemoryDepResult around. This is in preparation for porting this pass to the new PM. llvm-svn: 272595 --- .../Scalar/MergedLoadStoreMotion.cpp | 148 ++++++++---------- 1 file changed, 63 insertions(+), 85 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp b/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp index 6010c80cd93a..c9fbec289605 100644 --- a/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp +++ b/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp @@ -103,8 +103,7 @@ class MergedLoadStoreMotion : public FunctionPass { public: static char ID; // Pass identification, replacement for typeid - MergedLoadStoreMotion() - : FunctionPass(ID), MD(nullptr), MagicCompileTimeControl(250) { + MergedLoadStoreMotion() : FunctionPass(ID), MD(nullptr) { initializeMergedLoadStoreMotionPass(*PassRegistry::getPassRegistry()); } @@ -118,43 +117,17 @@ private: AU.addPreserved(); AU.addPreserved(); } - - // Helper routines - - /// - /// \brief Remove instruction from parent and update memory dependence - /// analysis. - /// - void removeInstruction(Instruction *Inst); - BasicBlock *getDiamondTail(BasicBlock *BB); - bool isDiamondHead(BasicBlock *BB); - // Routines for hoisting loads - bool isLoadHoistBarrierInRange(const Instruction &Start, - const Instruction &End, LoadInst *LI, - bool SafeToLoadUnconditionally); - LoadInst *canHoistFromBlock(BasicBlock *BB, LoadInst *LI); - void hoistInstruction(BasicBlock *BB, Instruction *HoistCand, - Instruction *ElseInst); - bool isSafeToHoist(Instruction *I) const; - bool hoistLoad(BasicBlock *BB, LoadInst *HoistCand, LoadInst *ElseInst); - bool mergeLoads(BasicBlock *BB); - // Routines for sinking stores - StoreInst *canSinkFromBlock(BasicBlock *BB, StoreInst *SI); - PHINode *getPHIOperand(BasicBlock *BB, StoreInst *S0, StoreInst *S1); - bool isStoreSinkBarrierInRange(const Instruction &Start, - const Instruction &End, MemoryLocation Loc); - bool sinkStore(BasicBlock *BB, StoreInst *SinkCand, StoreInst *ElseInst); - bool mergeStores(BasicBlock *BB); - // The mergeLoad/Store algorithms could have Size0 * Size1 complexity, - // where Size0 and Size1 are the #instructions on the two sides of - // the diamond. The constant chosen here is arbitrary. Compiler Time - // Control is enforced by the check Size0 * Size1 < MagicCompileTimeControl. - const int MagicCompileTimeControl; }; char MergedLoadStoreMotion::ID = 0; } // anonymous namespace +// The mergeLoad/Store algorithms could have Size0 * Size1 complexity, +// where Size0 and Size1 are the #instructions on the two sides of +// the diamond. The constant chosen here is arbitrary. Compiler Time +// Control is enforced by the check Size0 * Size1 < MagicCompileTimeControl. +const int MagicCompileTimeControl = 250; + /// /// \brief createMergedLoadStoreMotionPass - The public interface to this file. /// @@ -173,7 +146,7 @@ INITIALIZE_PASS_END(MergedLoadStoreMotion, "mldst-motion", /// /// \brief Remove instruction from parent and update memory dependence analysis. /// -void MergedLoadStoreMotion::removeInstruction(Instruction *Inst) { +static void removeInstruction(Instruction *Inst, MemoryDependenceResults *MD) { // Notify the memory dependence analysis. if (MD) { MD->removeInstruction(Inst); @@ -186,18 +159,10 @@ void MergedLoadStoreMotion::removeInstruction(Instruction *Inst) { Inst->eraseFromParent(); } -/// -/// \brief Return tail block of a diamond. -/// -BasicBlock *MergedLoadStoreMotion::getDiamondTail(BasicBlock *BB) { - assert(isDiamondHead(BB) && "Basic block is not head of a diamond"); - return BB->getTerminator()->getSuccessor(0)->getSingleSuccessor(); -} - /// /// \brief True when BB is the head of a diamond (hammock) /// -bool MergedLoadStoreMotion::isDiamondHead(BasicBlock *BB) { +static bool isDiamondHead(BasicBlock *BB) { if (!BB) return false; auto *BI = dyn_cast(BB->getTerminator()); @@ -220,6 +185,14 @@ bool MergedLoadStoreMotion::isDiamondHead(BasicBlock *BB) { return true; } +/// +/// \brief Return tail block of a diamond. +/// +static BasicBlock *getDiamondTail(BasicBlock *BB) { + assert(isDiamondHead(BB) && "Basic block is not head of a diamond"); + return BB->getTerminator()->getSuccessor(0)->getSingleSuccessor(); +} + /// /// \brief True when instruction is a hoist barrier for a load /// @@ -227,9 +200,10 @@ bool MergedLoadStoreMotion::isDiamondHead(BasicBlock *BB) { /// being loaded or protect against the load from happening /// it is considered a hoist barrier. /// -bool MergedLoadStoreMotion::isLoadHoistBarrierInRange( - const Instruction &Start, const Instruction &End, LoadInst *LI, - bool SafeToLoadUnconditionally) { +static bool isLoadHoistBarrierInRange(const Instruction &Start, + const Instruction &End, LoadInst *LI, + bool SafeToLoadUnconditionally, + AliasAnalysis *AA) { if (!SafeToLoadUnconditionally) for (const Instruction &Inst : make_range(Start.getIterator(), End.getIterator())) @@ -246,8 +220,8 @@ bool MergedLoadStoreMotion::isLoadHoistBarrierInRange( /// and it can be hoisted from \p BB, return that load. /// Otherwise return Null. /// -LoadInst *MergedLoadStoreMotion::canHoistFromBlock(BasicBlock *BB1, - LoadInst *Load0) { +static LoadInst *canHoistFromBlock(BasicBlock *BB1, LoadInst *Load0, + AliasAnalysis *AA) { BasicBlock *BB0 = Load0->getParent(); BasicBlock *Head = BB0->getSinglePredecessor(); bool SafeToLoadUnconditionally = isSafeToLoadUnconditionally( @@ -267,9 +241,9 @@ LoadInst *MergedLoadStoreMotion::canHoistFromBlock(BasicBlock *BB1, MemoryLocation Loc1 = MemoryLocation::get(Load1); if (AA->isMustAlias(Loc0, Loc1) && Load0->isSameOperationAs(Load1) && !isLoadHoistBarrierInRange(BB1->front(), *Load1, Load1, - SafeToLoadUnconditionally) && + SafeToLoadUnconditionally, AA) && !isLoadHoistBarrierInRange(BB0->front(), *Load0, Load0, - SafeToLoadUnconditionally)) { + SafeToLoadUnconditionally, AA)) { return Load1; } } @@ -282,9 +256,9 @@ LoadInst *MergedLoadStoreMotion::canHoistFromBlock(BasicBlock *BB1, /// /// BB is the head of a diamond /// -void MergedLoadStoreMotion::hoistInstruction(BasicBlock *BB, - Instruction *HoistCand, - Instruction *ElseInst) { +static void hoistInstruction(BasicBlock *BB, Instruction *HoistCand, + Instruction *ElseInst, + MemoryDependenceResults *MD) { DEBUG(dbgs() << " Hoist Instruction into BB \n"; BB->dump(); dbgs() << "Instruction Left\n"; HoistCand->dump(); dbgs() << "\n"; dbgs() << "Instruction Right\n"; ElseInst->dump(); dbgs() << "\n"); @@ -305,16 +279,16 @@ void MergedLoadStoreMotion::hoistInstruction(BasicBlock *BB, HoistedInst->insertBefore(HoistPt); HoistCand->replaceAllUsesWith(HoistedInst); - removeInstruction(HoistCand); + removeInstruction(HoistCand, MD); // Replace the else block instruction. ElseInst->replaceAllUsesWith(HoistedInst); - removeInstruction(ElseInst); + removeInstruction(ElseInst, MD); } /// /// \brief Return true if no operand of \p I is defined in I's parent block /// -bool MergedLoadStoreMotion::isSafeToHoist(Instruction *I) const { +static bool isSafeToHoist(Instruction *I) { BasicBlock *Parent = I->getParent(); for (Use &U : I->operands()) if (auto *Instr = dyn_cast(&U)) @@ -326,8 +300,8 @@ bool MergedLoadStoreMotion::isSafeToHoist(Instruction *I) const { /// /// \brief Merge two equivalent loads and GEPs and hoist into diamond head /// -bool MergedLoadStoreMotion::hoistLoad(BasicBlock *BB, LoadInst *L0, - LoadInst *L1) { +static bool hoistLoad(BasicBlock *BB, LoadInst *L0, LoadInst *L1, + MemoryDependenceResults *MD) { // Only one definition? auto *A0 = dyn_cast(L0->getPointerOperand()); auto *A1 = dyn_cast(L1->getPointerOperand()); @@ -338,8 +312,8 @@ bool MergedLoadStoreMotion::hoistLoad(BasicBlock *BB, LoadInst *L0, DEBUG(dbgs() << "Hoist Instruction into BB \n"; BB->dump(); dbgs() << "Instruction Left\n"; L0->dump(); dbgs() << "\n"; dbgs() << "Instruction Right\n"; L1->dump(); dbgs() << "\n"); - hoistInstruction(BB, A0, A1); - hoistInstruction(BB, L0, L1); + hoistInstruction(BB, A0, A1, MD); + hoistInstruction(BB, L0, L1, MD); return true; } return false; @@ -351,7 +325,8 @@ bool MergedLoadStoreMotion::hoistLoad(BasicBlock *BB, LoadInst *L0, /// Starting from a diamond head block, iterate over the instructions in one /// successor block and try to match a load in the second successor. /// -bool MergedLoadStoreMotion::mergeLoads(BasicBlock *BB) { +static bool mergeLoads(BasicBlock *BB, AliasAnalysis *AA, + MemoryDependenceResults *MD) { bool MergedLoads = false; assert(isDiamondHead(BB)); BranchInst *BI = cast(BB->getTerminator()); @@ -373,8 +348,8 @@ bool MergedLoadStoreMotion::mergeLoads(BasicBlock *BB) { ++NLoads; if (NLoads * Size1 >= MagicCompileTimeControl) break; - if (LoadInst *L1 = canHoistFromBlock(Succ1, L0)) { - bool Res = hoistLoad(BB, L0, L1); + if (LoadInst *L1 = canHoistFromBlock(Succ1, L0, AA)) { + bool Res = hoistLoad(BB, L0, L1, MD); MergedLoads |= Res; // Don't attempt to hoist above loads that had not been hoisted. if (!Res) @@ -392,9 +367,9 @@ bool MergedLoadStoreMotion::mergeLoads(BasicBlock *BB) { /// value being stored or protect against the store from /// happening it is considered a sink barrier. /// -bool MergedLoadStoreMotion::isStoreSinkBarrierInRange(const Instruction &Start, - const Instruction &End, - MemoryLocation Loc) { +static bool isStoreSinkBarrierInRange(const Instruction &Start, + const Instruction &End, + MemoryLocation Loc, AliasAnalysis *AA) { for (const Instruction &Inst : make_range(Start.getIterator(), End.getIterator())) if (Inst.mayThrow()) @@ -407,8 +382,8 @@ bool MergedLoadStoreMotion::isStoreSinkBarrierInRange(const Instruction &Start, /// /// \return The store in \p when it is safe to sink. Otherwise return Null. /// -StoreInst *MergedLoadStoreMotion::canSinkFromBlock(BasicBlock *BB1, - StoreInst *Store0) { +static StoreInst *canSinkFromBlock(BasicBlock *BB1, StoreInst *Store0, + AliasAnalysis *AA) { DEBUG(dbgs() << "can Sink? : "; Store0->dump(); dbgs() << "\n"); BasicBlock *BB0 = Store0->getParent(); for (BasicBlock::reverse_iterator RBI = BB1->rbegin(), RBE = BB1->rend(); @@ -422,8 +397,10 @@ StoreInst *MergedLoadStoreMotion::canSinkFromBlock(BasicBlock *BB1, MemoryLocation Loc0 = MemoryLocation::get(Store0); MemoryLocation Loc1 = MemoryLocation::get(Store1); if (AA->isMustAlias(Loc0, Loc1) && Store0->isSameOperationAs(Store1) && - !isStoreSinkBarrierInRange(*Store1->getNextNode(), BB1->back(), Loc1) && - !isStoreSinkBarrierInRange(*Store0->getNextNode(), BB0->back(), Loc0)) { + !isStoreSinkBarrierInRange(*Store1->getNextNode(), BB1->back(), Loc1, + AA) && + !isStoreSinkBarrierInRange(*Store0->getNextNode(), BB0->back(), Loc0, + AA)) { return Store1; } } @@ -433,8 +410,8 @@ StoreInst *MergedLoadStoreMotion::canSinkFromBlock(BasicBlock *BB1, /// /// \brief Create a PHI node in BB for the operands of S0 and S1 /// -PHINode *MergedLoadStoreMotion::getPHIOperand(BasicBlock *BB, StoreInst *S0, - StoreInst *S1) { +static PHINode *getPHIOperand(BasicBlock *BB, StoreInst *S0, StoreInst *S1, + MemoryDependenceResults *MD) { // Create a phi if the values mismatch. Value *Opd1 = S0->getValueOperand(); Value *Opd2 = S1->getValueOperand(); @@ -455,8 +432,8 @@ PHINode *MergedLoadStoreMotion::getPHIOperand(BasicBlock *BB, StoreInst *S0, /// /// Also sinks GEP instruction computing the store address /// -bool MergedLoadStoreMotion::sinkStore(BasicBlock *BB, StoreInst *S0, - StoreInst *S1) { +static bool sinkStore(BasicBlock *BB, StoreInst *S0, StoreInst *S1, + MemoryDependenceResults *MD) { // Only one definition? auto *A0 = dyn_cast(S0->getPointerOperand()); auto *A1 = dyn_cast(S1->getPointerOperand()); @@ -482,14 +459,14 @@ bool MergedLoadStoreMotion::sinkStore(BasicBlock *BB, StoreInst *S0, assert(S1->getParent() == A1->getParent()); // New PHI operand? Use it. - if (PHINode *NewPN = getPHIOperand(BB, S0, S1)) + if (PHINode *NewPN = getPHIOperand(BB, S0, S1, MD)) SNew->setOperand(0, NewPN); - removeInstruction(S0); - removeInstruction(S1); + removeInstruction(S0, MD); + removeInstruction(S1, MD); A0->replaceAllUsesWith(ANew); - removeInstruction(A0); + removeInstruction(A0, MD); A1->replaceAllUsesWith(ANew); - removeInstruction(A1); + removeInstruction(A1, MD); return true; } return false; @@ -501,7 +478,8 @@ bool MergedLoadStoreMotion::sinkStore(BasicBlock *BB, StoreInst *S0, /// Starting from a diamond tail block, iterate over the instructions in one /// predecessor block and try to match a store in the second predecessor. /// -bool MergedLoadStoreMotion::mergeStores(BasicBlock *T) { +static bool mergeStores(BasicBlock *T, AliasAnalysis *AA, + MemoryDependenceResults *MD) { bool MergedStores = false; assert(T && "Footer of a diamond cannot be empty"); @@ -536,8 +514,8 @@ bool MergedLoadStoreMotion::mergeStores(BasicBlock *T) { ++NStores; if (NStores * Size1 >= MagicCompileTimeControl) break; - if (StoreInst *S1 = canSinkFromBlock(Pred1, S0)) { - bool Res = sinkStore(T, S0, S1); + if (StoreInst *S1 = canSinkFromBlock(Pred1, S0, AA)) { + bool Res = sinkStore(T, S0, S1, MD); MergedStores |= Res; // Don't attempt to sink below stores that had to stick around // But after removal of a store and some of its feeding @@ -575,8 +553,8 @@ bool MergedLoadStoreMotion::runOnFunction(Function &F) { // Hoist equivalent loads and sink stores // outside diamonds when possible if (isDiamondHead(BB)) { - Changed |= mergeLoads(BB); - Changed |= mergeStores(getDiamondTail(BB)); + Changed |= mergeLoads(BB, AA, MD); + Changed |= mergeStores(getDiamondTail(BB), AA, MD); } } return Changed;