From 6ef94a5c91bef01b11d8eba1cc29a3a63a81e246 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Steinbrink?= <bjoern.steinbrink@saltation.com>
Date: Tue, 23 Jan 2018 20:31:09 +0100
Subject: [PATCH] Mark MergedLoadStoreMotion as not preserving MemDep results
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.
---
.../Scalar/MergedLoadStoreMotion.cpp | 53 ++++---------------
1 file changed, 10 insertions(+), 43 deletions(-)
diff --git a/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp b/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp
index 6a64c6b3619c..8ef0f9637163 100644
--- a/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp
+++ b/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp
@@ -79,7 +79,6 @@
#include "llvm/Analysis/GlobalsModRef.h"
#include "llvm/Analysis/Loads.h"
#include "llvm/Analysis/MemoryBuiltins.h"
-#include "llvm/Analysis/MemoryDependenceAnalysis.h"
#include "llvm/Analysis/ValueTracking.h"
#include "llvm/IR/Metadata.h"
#include "llvm/IR/PatternMatch.h"
@@ -98,7 +97,6 @@ namespace {
// MergedLoadStoreMotion Pass
//===----------------------------------------------------------------------===//
class MergedLoadStoreMotion {
- MemoryDependenceResults *MD = nullptr;
AliasAnalysis *AA = nullptr;
// The mergeLoad/Store algorithms could have Size0 * Size1 complexity,
@@ -108,14 +106,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 hoisting loads
@@ -138,22 +131,6 @@ class MergedLoadStoreMotion {
};
} // 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<LoadInst>(Inst))
- MD->invalidateCachedPointerInfo(LI->getPointerOperand());
- if (Inst->getType()->isPtrOrPtrVectorTy()) {
- MD->invalidateCachedPointerInfo(Inst);
- }
- }
- Inst->eraseFromParent();
-}
-
///
/// \brief Return tail block of a diamond.
///
@@ -273,10 +250,10 @@ void MergedLoadStoreMotion::hoistInstruction(BasicBlock *BB,
HoistedInst->insertBefore(HoistPt);
HoistCand->replaceAllUsesWith(HoistedInst);
- removeInstruction(HoistCand);
+ HoistCand->eraseFromParent();
// Replace the else block instruction.
ElseInst->replaceAllUsesWith(HoistedInst);
- removeInstruction(ElseInst);
+ ElseInst->eraseFromParent();
}
///
@@ -410,8 +387,6 @@ PHINode *MergedLoadStoreMotion::getPHIOperand(BasicBlock *BB, StoreInst *S0,
&BB->front());
NewPN->addIncoming(Opd1, S0->getParent());
NewPN->addIncoming(Opd2, S1->getParent());
- if (MD && NewPN->getType()->getScalarType()->isPointerTy())
- MD->invalidateCachedPointerInfo(NewPN);
return NewPN;
}
@@ -449,12 +424,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;
@@ -518,9 +493,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;
@@ -557,9 +530,7 @@ class MergedLoadStoreMotionLegacyPass : public FunctionPass {
if (skipFunction(F))
return false;
MergedLoadStoreMotion Impl;
- auto *MDWP = getAnalysisIfAvailable<MemoryDependenceWrapperPass>();
- return Impl.run(F, MDWP ? &MDWP->getMemDep() : nullptr,
- getAnalysis<AAResultsWrapperPass>().getAAResults());
+ return Impl.run(F, getAnalysis<AAResultsWrapperPass>().getAAResults());
}
private:
@@ -567,7 +538,6 @@ class MergedLoadStoreMotionLegacyPass : public FunctionPass {
AU.setPreservesCFG();
AU.addRequired<AAResultsWrapperPass>();
AU.addPreserved<GlobalsAAWrapperPass>();
- AU.addPreserved<MemoryDependenceWrapperPass>();
}
};
@@ -583,7 +553,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)
@@ -591,14 +560,12 @@ INITIALIZE_PASS_END(MergedLoadStoreMotionLegacyPass, "mldst-motion",
PreservedAnalyses
MergedLoadStoreMotionPass::run(Function &F, FunctionAnalysisManager &AM) {
MergedLoadStoreMotion Impl;
- auto *MD = AM.getCachedResult<MemoryDependenceAnalysis>(F);
auto &AA = AM.getResult<AAManager>(F);
- if (!Impl.run(F, MD, AA))
+ if (!Impl.run(F, AA))
return PreservedAnalyses::all();
// FIXME: This should also 'preserve the CFG'.
PreservedAnalyses PA;
PA.preserve<GlobalsAA>();
- PA.preserve<MemoryDependenceAnalysis>();
return PA;
}