diff --git a/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp b/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp index 3ef4a73ff8cd..f6b4ad333795 100644 --- a/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp +++ b/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp @@ -19,6 +19,8 @@ // thinks it safe to do so. This optimization helps with eg. hiding load // latencies, triggering if-conversion, and reducing static code size. // +// NOTE: This code no longer performs load hoisting, it is subsumed by GVNHoist. +// //===----------------------------------------------------------------------===// // // @@ -118,16 +120,6 @@ private: 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); @@ -188,169 +180,6 @@ bool MergedLoadStoreMotion::isDiamondHead(BasicBlock *BB) { return true; } -/// -/// \brief True when instruction is a hoist barrier for a load -/// -/// Whenever an instruction could possibly modify the value -/// 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) { - if (!SafeToLoadUnconditionally) - for (const Instruction &Inst : - make_range(Start.getIterator(), End.getIterator())) - if (!isGuaranteedToTransferExecutionToSuccessor(&Inst)) - return true; - MemoryLocation Loc = MemoryLocation::get(LI); - return AA->canInstructionRangeModRef(Start, End, Loc, MRI_Mod); -} - -/// -/// \brief Decide if a load can be hoisted -/// -/// When there is a load in \p BB to the same address as \p LI -/// and it can be hoisted from \p BB, return that load. -/// Otherwise return Null. -/// -LoadInst *MergedLoadStoreMotion::canHoistFromBlock(BasicBlock *BB1, - LoadInst *Load0) { - BasicBlock *BB0 = Load0->getParent(); - BasicBlock *Head = BB0->getSinglePredecessor(); - bool SafeToLoadUnconditionally = isSafeToLoadUnconditionally( - Load0->getPointerOperand(), Load0->getAlignment(), - Load0->getModule()->getDataLayout(), - /*ScanFrom=*/Head->getTerminator()); - for (BasicBlock::iterator BBI = BB1->begin(), BBE = BB1->end(); BBI != BBE; - ++BBI) { - Instruction *Inst = &*BBI; - - // Only merge and hoist loads when their result in used only in BB - auto *Load1 = dyn_cast<LoadInst>(Inst); - if (!Load1 || Inst->isUsedOutsideOfBlock(BB1)) - continue; - - MemoryLocation Loc0 = MemoryLocation::get(Load0); - MemoryLocation Loc1 = MemoryLocation::get(Load1); - if (Load0->isSameOperationAs(Load1) && AA->isMustAlias(Loc0, Loc1) && - !isLoadHoistBarrierInRange(BB1->front(), *Load1, Load1, - SafeToLoadUnconditionally) && - !isLoadHoistBarrierInRange(BB0->front(), *Load0, Load0, - SafeToLoadUnconditionally)) { - return Load1; - } - } - return nullptr; -} - -/// -/// \brief Merge two equivalent instructions \p HoistCand and \p ElseInst into -/// \p BB -/// -/// BB is the head of a diamond -/// -void MergedLoadStoreMotion::hoistInstruction(BasicBlock *BB, - Instruction *HoistCand, - Instruction *ElseInst) { - 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"); - // Hoist the instruction. - assert(HoistCand->getParent() != BB); - - // Intersect optional metadata. - HoistCand->andIRFlags(ElseInst); - HoistCand->dropUnknownNonDebugMetadata(); - - // Prepend point for instruction insert - Instruction *HoistPt = BB->getTerminator(); - - // Merged instruction - Instruction *HoistedInst = HoistCand->clone(); - - // Hoist instruction. - HoistedInst->insertBefore(HoistPt); - - HoistCand->replaceAllUsesWith(HoistedInst); - removeInstruction(HoistCand); - // Replace the else block instruction. - ElseInst->replaceAllUsesWith(HoistedInst); - removeInstruction(ElseInst); -} - -/// -/// \brief Return true if no operand of \p I is defined in I's parent block -/// -bool MergedLoadStoreMotion::isSafeToHoist(Instruction *I) const { - BasicBlock *Parent = I->getParent(); - for (Use &U : I->operands()) - if (auto *Instr = dyn_cast<Instruction>(&U)) - if (Instr->getParent() == Parent) - return false; - return true; -} - -/// -/// \brief Merge two equivalent loads and GEPs and hoist into diamond head -/// -bool MergedLoadStoreMotion::hoistLoad(BasicBlock *BB, LoadInst *L0, - LoadInst *L1) { - // Only one definition? - auto *A0 = dyn_cast<Instruction>(L0->getPointerOperand()); - auto *A1 = dyn_cast<Instruction>(L1->getPointerOperand()); - if (A0 && A1 && A0->isIdenticalTo(A1) && isSafeToHoist(A0) && - A0->hasOneUse() && (A0->getParent() == L0->getParent()) && - A1->hasOneUse() && (A1->getParent() == L1->getParent()) && - isa<GetElementPtrInst>(A0)) { - 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); - return true; - } - return false; -} - -/// -/// \brief Try to hoist two loads to same address into diamond header -/// -/// 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) { - bool MergedLoads = false; - assert(isDiamondHead(BB)); - BranchInst *BI = cast<BranchInst>(BB->getTerminator()); - BasicBlock *Succ0 = BI->getSuccessor(0); - BasicBlock *Succ1 = BI->getSuccessor(1); - // #Instructions in Succ1 for Compile Time Control - int Size1 = Succ1->size(); - int NLoads = 0; - for (BasicBlock::iterator BBI = Succ0->begin(), BBE = Succ0->end(); - BBI != BBE;) { - Instruction *I = &*BBI; - ++BBI; - - // Don't move non-simple (atomic, volatile) loads. - auto *L0 = dyn_cast<LoadInst>(I); - if (!L0 || !L0->isSimple() || L0->isUsedOutsideOfBlock(Succ0)) - continue; - - ++NLoads; - if (NLoads * Size1 >= MagicCompileTimeControl) - break; - if (LoadInst *L1 = canHoistFromBlock(Succ1, L0)) { - bool Res = hoistLoad(BB, L0, L1); - MergedLoads |= Res; - // Don't attempt to hoist above loads that had not been hoisted. - if (!Res) - break; - } - } - return MergedLoads; -} /// /// \brief True when instruction is a sink barrier for a store @@ -534,7 +363,6 @@ bool MergedLoadStoreMotion::run(Function &F, MemoryDependenceResults *MD, // Hoist equivalent loads and sink stores // outside diamonds when possible if (isDiamondHead(BB)) { - Changed |= mergeLoads(BB); Changed |= mergeStores(getDiamondTail(BB)); } } diff --git a/llvm/test/Transforms/InstMerge/ld_hoist1.ll b/llvm/test/Transforms/GVNHoist/ld_hoist1.ll similarity index 97% rename from llvm/test/Transforms/InstMerge/ld_hoist1.ll rename to llvm/test/Transforms/GVNHoist/ld_hoist1.ll index 74c8900b8ab1..8d4698d87e6f 100644 --- a/llvm/test/Transforms/InstMerge/ld_hoist1.ll +++ b/llvm/test/Transforms/GVNHoist/ld_hoist1.ll @@ -1,5 +1,5 @@ ; Test load hoist -; RUN: opt -basicaa -memdep -mldst-motion -S < %s | FileCheck %s +; RUN: opt -gvn-hoist -S < %s | FileCheck %s target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" target triple = "x86_64-pc_linux" diff --git a/llvm/test/Transforms/InstMerge/ld_hoist_st_sink.ll b/llvm/test/Transforms/GVNHoist/ld_hoist_st_sink.ll similarity index 96% rename from llvm/test/Transforms/InstMerge/ld_hoist_st_sink.ll rename to llvm/test/Transforms/GVNHoist/ld_hoist_st_sink.ll index 1d3f941882e5..c85edc2d8170 100644 --- a/llvm/test/Transforms/InstMerge/ld_hoist_st_sink.ll +++ b/llvm/test/Transforms/GVNHoist/ld_hoist_st_sink.ll @@ -1,6 +1,6 @@ ; Tests to make sure that loads and stores in a diamond get merged ; Loads are hoisted into the header. Stores sunks into the footer. -; RUN: opt -basicaa -memdep -mldst-motion -S < %s | FileCheck %s +; RUN: opt -gvn-hoist -S < %s | FileCheck %s target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128" %struct.node = type { i64, %struct.node*, %struct.node*, %struct.node*, i64, %struct.arc*, i64, i64, i64 } @@ -41,7 +41,7 @@ if.then: ; preds = %while.body %4 = load i64, i64* %p, align 8 %add = add nsw i64 %4, %2 %p1 = getelementptr inbounds %struct.node, %struct.node* %node.020, i64 0, i32 6 -; CHECK-NOT: store i64 +; FIXME: store i64 store i64 %add, i64* %p1, align 8 br label %if.end @@ -61,13 +61,13 @@ if.else: ; preds = %while.body %8 = load i64, i64* %cost5, align 8 %sub = sub nsw i64 %6, %8 %p6 = getelementptr inbounds %struct.node, %struct.node* %node.020, i64 0, i32 6 -; CHECK-NOT: store i64 +; FIXME: store i64 store i64 %sub, i64* %p6, align 8 br label %if.end ; CHECK: if.end if.end: ; preds = %if.else, %if.then -; CHECK: store +; FIXME: store %inc = add nsw i64 %sum.019, 1 %node.0.in = getelementptr inbounds %struct.node, %struct.node* %node.020, i64 0, i32 2 %node.0 = load %struct.node*, %struct.node** %node.0.in, align 8