From 4615a505f869f1c9ab3e7016c1ad792b67991cee Mon Sep 17 00:00:00 2001 From: Max Kazantsev Date: Wed, 9 Jan 2019 07:28:13 +0000 Subject: [PATCH] [IPT] Drop cache less eagerly in GVN and LoopSafetyInfo Current strategy of dropping `InstructionPrecedenceTracking` cache is to invalidate the entire basic block whenever we change its contents. In fact, `InstructionPrecedenceTracking` has 2 internal strictures: `OrderedInstructions` that is needed to be invalidated whenever the contents changes, and the map with first special instructions in block. This second map does not need an update if we add/remove a non-special instuction because it cannot affect the contents of this map. This patch changes API of `InstructionPrecedenceTracking` so that it now accounts for reasons under which we invalidate blocks. This should lead to much less recalculations of the map and should save us some compile time because in practice we don't typically add/remove special instructions. Differential Revision: https://reviews.llvm.org/D54462 Reviewed By: efriedma llvm-svn: 350694 --- .../llvm/Analysis/InstructionPrecedenceTracking.h | 10 ++++++++-- llvm/include/llvm/Analysis/MustExecute.h | 6 +++--- llvm/lib/Analysis/InstructionPrecedenceTracking.cpp | 12 ++++++++++-- llvm/lib/Analysis/MustExecute.cpp | 13 ++++++------- llvm/lib/Transforms/Scalar/GVN.cpp | 5 ++--- llvm/lib/Transforms/Scalar/LICM.cpp | 6 +++--- 6 files changed, 32 insertions(+), 20 deletions(-) diff --git a/llvm/include/llvm/Analysis/InstructionPrecedenceTracking.h b/llvm/include/llvm/Analysis/InstructionPrecedenceTracking.h index b754557ecc41d..073e6ec3b7f6e 100644 --- a/llvm/include/llvm/Analysis/InstructionPrecedenceTracking.h +++ b/llvm/include/llvm/Analysis/InstructionPrecedenceTracking.h @@ -75,8 +75,14 @@ class InstructionPrecedenceTracking { virtual ~InstructionPrecedenceTracking() = default; public: - /// Clears cached information about this particular block. - void invalidateBlock(const BasicBlock *BB); + /// Notifies this tracking that we are going to insert a new instruction \p + /// Inst to the basic block \p BB. It makes all necessary updates to internal + /// caches to keep them consistent. + void insertInstructionTo(const Instruction *Inst, const BasicBlock *BB); + + /// Notifies this tracking that we are going to remove the instruction \p Inst + /// It makes all necessary updates to internal caches to keep them consistent. + void removeInstruction(const Instruction *Inst); /// Invalidates all information from this tracking. void clear(); diff --git a/llvm/include/llvm/Analysis/MustExecute.h b/llvm/include/llvm/Analysis/MustExecute.h index b973447ca8221..ad3222c17e62a 100644 --- a/llvm/include/llvm/Analysis/MustExecute.h +++ b/llvm/include/llvm/Analysis/MustExecute.h @@ -151,9 +151,9 @@ class ICFLoopSafetyInfo: public LoopSafetyInfo { const; /// Inform the safety info that we are planning to insert a new instruction - /// into the basic block \p BB. It will make all cache updates to keep it - /// correct after this insertion. - void insertInstructionTo(const BasicBlock *BB); + /// \p Inst into the basic block \p BB. It will make all cache updates to keep + /// it correct after this insertion. + void insertInstructionTo(const Instruction *Inst, const BasicBlock *BB); /// Inform safety info that we are planning to remove the instruction \p Inst /// from its block. It will make all cache updates to keep it correct after diff --git a/llvm/lib/Analysis/InstructionPrecedenceTracking.cpp b/llvm/lib/Analysis/InstructionPrecedenceTracking.cpp index b98975b006a44..816126f407caf 100644 --- a/llvm/lib/Analysis/InstructionPrecedenceTracking.cpp +++ b/llvm/lib/Analysis/InstructionPrecedenceTracking.cpp @@ -99,9 +99,17 @@ void InstructionPrecedenceTracking::validateAll() const { } #endif -void InstructionPrecedenceTracking::invalidateBlock(const BasicBlock *BB) { +void InstructionPrecedenceTracking::insertInstructionTo(const Instruction *Inst, + const BasicBlock *BB) { + if (isSpecialInstruction(Inst)) + FirstSpecialInsts.erase(BB); OI.invalidateBlock(BB); - FirstSpecialInsts.erase(BB); +} + +void InstructionPrecedenceTracking::removeInstruction(const Instruction *Inst) { + if (isSpecialInstruction(Inst)) + FirstSpecialInsts.erase(Inst->getParent()); + OI.invalidateBlock(Inst->getParent()); } void InstructionPrecedenceTracking::clear() { diff --git a/llvm/lib/Analysis/MustExecute.cpp b/llvm/lib/Analysis/MustExecute.cpp index 281b8f5ab987e..180c38ddacc22 100644 --- a/llvm/lib/Analysis/MustExecute.cpp +++ b/llvm/lib/Analysis/MustExecute.cpp @@ -83,16 +83,15 @@ void ICFLoopSafetyInfo::computeLoopSafetyInfo(const Loop *CurLoop) { computeBlockColors(CurLoop); } -void ICFLoopSafetyInfo::insertInstructionTo(const BasicBlock *BB) { - ICF.invalidateBlock(BB); - MW.invalidateBlock(BB); +void ICFLoopSafetyInfo::insertInstructionTo(const Instruction *Inst, + const BasicBlock *BB) { + ICF.insertInstructionTo(Inst, BB); + MW.insertInstructionTo(Inst, BB); } void ICFLoopSafetyInfo::removeInstruction(const Instruction *Inst) { - // TODO: So far we just conservatively drop cache, but maybe we can not do it - // when Inst is not an ICF instruction. Follow-up on that. - ICF.invalidateBlock(Inst->getParent()); - MW.invalidateBlock(Inst->getParent()); + ICF.removeInstruction(Inst); + MW.removeInstruction(Inst); } void LoopSafetyInfo::computeBlockColors(const Loop *CurLoop) { diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp index 04ed914b86c28..9598e595c5ee9 100644 --- a/llvm/lib/Transforms/Scalar/GVN.cpp +++ b/llvm/lib/Transforms/Scalar/GVN.cpp @@ -2079,10 +2079,9 @@ bool GVN::processBlock(BasicBlock *BB) { salvageDebugInfo(*I); if (MD) MD->removeInstruction(I); LLVM_DEBUG(verifyRemoved(I)); + ICF->removeInstruction(I); I->eraseFromParent(); } - - ICF->invalidateBlock(BB); InstrsToErase.clear(); if (AtStart) @@ -2301,7 +2300,7 @@ bool GVN::performScalarPRE(Instruction *CurInst) { LLVM_DEBUG(verifyRemoved(CurInst)); // FIXME: Intended to be markInstructionForDeletion(CurInst), but it causes // some assertion failures. - ICF->invalidateBlock(CurrentBlock); + ICF->removeInstruction(CurInst); CurInst->eraseFromParent(); ++NumGVNInstr; diff --git a/llvm/lib/Transforms/Scalar/LICM.cpp b/llvm/lib/Transforms/Scalar/LICM.cpp index 5ebf035bf510f..3b44ac564d84f 100644 --- a/llvm/lib/Transforms/Scalar/LICM.cpp +++ b/llvm/lib/Transforms/Scalar/LICM.cpp @@ -742,13 +742,13 @@ bool llvm::hoistRegion(DomTreeNode *N, AliasAnalysis *AA, LoopInfo *LI, auto One = llvm::ConstantFP::get(Divisor->getType(), 1.0); auto ReciprocalDivisor = BinaryOperator::CreateFDiv(One, Divisor); ReciprocalDivisor->setFastMathFlags(I.getFastMathFlags()); - SafetyInfo->insertInstructionTo(I.getParent()); + SafetyInfo->insertInstructionTo(ReciprocalDivisor, I.getParent()); ReciprocalDivisor->insertBefore(&I); auto Product = BinaryOperator::CreateFMul(I.getOperand(0), ReciprocalDivisor); Product->setFastMathFlags(I.getFastMathFlags()); - SafetyInfo->insertInstructionTo(I.getParent()); + SafetyInfo->insertInstructionTo(Product, I.getParent()); Product->insertAfter(&I); I.replaceAllUsesWith(Product); eraseInstruction(I, *SafetyInfo, CurAST); @@ -1189,7 +1189,7 @@ static void eraseInstruction(Instruction &I, ICFLoopSafetyInfo &SafetyInfo, static void moveInstructionBefore(Instruction &I, Instruction &Dest, ICFLoopSafetyInfo &SafetyInfo) { SafetyInfo.removeInstruction(&I); - SafetyInfo.insertInstructionTo(Dest.getParent()); + SafetyInfo.insertInstructionTo(&I, Dest.getParent()); I.moveBefore(&Dest); }