From 983d6c3f18908532b28887ca96ac0da6ad921e7f Mon Sep 17 00:00:00 2001 From: Bjorn Steinbrink Date: Fri, 23 Feb 2018 10:41:57 +0000 Subject: Mark MergedLoadStoreMotion as not preserving MemDep results Summary: MemDep caches results that signify that a dependence is non-local, and there is currently no way to invalidate such cache entries. Unfortunately, when MLSM sinks a store that can result in a non-local dependence becoming a local one, and then MemDep gives wrong answers. The easiest way out here is to just say that MLSM does indeed not preserve MemDep results. Reviewers: davide, Gerolf Subscribers: llvm-commits Differential Revision: https://reviews.llvm.org/D43177 llvm-svn: 325880 --- .../Transforms/Scalar/MergedLoadStoreMotion.cpp | 49 ++++------------------ 1 file changed, 8 insertions(+), 41 deletions(-) (limited to 'llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp') diff --git a/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp b/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp index f2f615cb9b0..058da52dd84 100644 --- a/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp +++ b/llvm/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp @@ -80,7 +80,6 @@ #include "llvm/Analysis/CFG.h" #include "llvm/Analysis/GlobalsModRef.h" #include "llvm/Analysis/Loads.h" -#include "llvm/Analysis/MemoryDependenceAnalysis.h" #include "llvm/Analysis/ValueTracking.h" #include "llvm/IR/Metadata.h" #include "llvm/Support/Debug.h" @@ -97,7 +96,6 @@ namespace { // MergedLoadStoreMotion Pass //===----------------------------------------------------------------------===// class MergedLoadStoreMotion { - MemoryDependenceResults *MD = nullptr; AliasAnalysis *AA = nullptr; // The mergeLoad/Store algorithms could have Size0 * Size1 complexity, @@ -107,14 +105,9 @@ class MergedLoadStoreMotion { const int MagicCompileTimeControl = 250; public: - bool run(Function &F, MemoryDependenceResults *MD, AliasAnalysis &AA); + bool run(Function &F, AliasAnalysis &AA); private: - /// - /// \brief Remove instruction from parent and update memory dependence - /// analysis. - /// - void removeInstruction(Instruction *Inst); BasicBlock *getDiamondTail(BasicBlock *BB); bool isDiamondHead(BasicBlock *BB); // Routines for sinking stores @@ -127,22 +120,6 @@ private: }; } // end anonymous namespace -/// -/// \brief Remove instruction from parent and update memory dependence analysis. -/// -void MergedLoadStoreMotion::removeInstruction(Instruction *Inst) { - // Notify the memory dependence analysis. - if (MD) { - MD->removeInstruction(Inst); - if (auto *LI = dyn_cast(Inst)) - MD->invalidateCachedPointerInfo(LI->getPointerOperand()); - if (Inst->getType()->isPtrOrPtrVectorTy()) { - MD->invalidateCachedPointerInfo(Inst); - } - } - Inst->eraseFromParent(); -} - /// /// \brief Return tail block of a diamond. /// @@ -236,8 +213,6 @@ PHINode *MergedLoadStoreMotion::getPHIOperand(BasicBlock *BB, StoreInst *S0, &BB->front()); NewPN->addIncoming(Opd1, S0->getParent()); NewPN->addIncoming(Opd2, S1->getParent()); - if (MD && NewPN->getType()->isPtrOrPtrVectorTy()) - MD->invalidateCachedPointerInfo(NewPN); return NewPN; } @@ -275,12 +250,12 @@ bool MergedLoadStoreMotion::sinkStore(BasicBlock *BB, StoreInst *S0, // New PHI operand? Use it. if (PHINode *NewPN = getPHIOperand(BB, S0, S1)) SNew->setOperand(0, NewPN); - removeInstruction(S0); - removeInstruction(S1); + S0->eraseFromParent(); + S1->eraseFromParent(); A0->replaceAllUsesWith(ANew); - removeInstruction(A0); + A0->eraseFromParent(); A1->replaceAllUsesWith(ANew); - removeInstruction(A1); + A1->eraseFromParent(); return true; } return false; @@ -344,9 +319,7 @@ bool MergedLoadStoreMotion::mergeStores(BasicBlock *T) { return MergedStores; } -bool MergedLoadStoreMotion::run(Function &F, MemoryDependenceResults *MD, - AliasAnalysis &AA) { - this->MD = MD; +bool MergedLoadStoreMotion::run(Function &F, AliasAnalysis &AA) { this->AA = &AA; bool Changed = false; @@ -382,9 +355,7 @@ public: if (skipFunction(F)) return false; MergedLoadStoreMotion Impl; - auto *MDWP = getAnalysisIfAvailable(); - return Impl.run(F, MDWP ? &MDWP->getMemDep() : nullptr, - getAnalysis().getAAResults()); + return Impl.run(F, getAnalysis().getAAResults()); } private: @@ -392,7 +363,6 @@ private: AU.setPreservesCFG(); AU.addRequired(); AU.addPreserved(); - AU.addPreserved(); } }; @@ -408,7 +378,6 @@ FunctionPass *llvm::createMergedLoadStoreMotionPass() { INITIALIZE_PASS_BEGIN(MergedLoadStoreMotionLegacyPass, "mldst-motion", "MergedLoadStoreMotion", false, false) -INITIALIZE_PASS_DEPENDENCY(MemoryDependenceWrapperPass) INITIALIZE_PASS_DEPENDENCY(AAResultsWrapperPass) INITIALIZE_PASS_END(MergedLoadStoreMotionLegacyPass, "mldst-motion", "MergedLoadStoreMotion", false, false) @@ -416,14 +385,12 @@ INITIALIZE_PASS_END(MergedLoadStoreMotionLegacyPass, "mldst-motion", PreservedAnalyses MergedLoadStoreMotionPass::run(Function &F, FunctionAnalysisManager &AM) { MergedLoadStoreMotion Impl; - auto *MD = AM.getCachedResult(F); auto &AA = AM.getResult(F); - if (!Impl.run(F, MD, AA)) + if (!Impl.run(F, AA)) return PreservedAnalyses::all(); PreservedAnalyses PA; PA.preserveSet(); PA.preserve(); - PA.preserve(); return PA; } -- cgit v1.2.3