From 49128636b627847b223fa3bcc29b85e6deb87c09 Mon Sep 17 00:00:00 2001 From: Quentin Colombet Date: Wed, 13 Aug 2014 21:00:07 +0000 Subject: [PATCH] [MachineDominatorTree] Provide a method to inform a MachineDominatorTree that a critical edge has been split. The MachineDominatorTree will when lazy update the underlying dominance properties when require. ** Context ** This is a follow-up of r215410. Each time a critical edge is split this invalidates the dominator tree information. Thus, subsequent queries of that interface will be slow until the underlying information is actually recomputed (costly). ** Problem ** Prior to this patch, splitting a critical edge needed to query the dominator tree to update the dominator information. Therefore, splitting a bunch of critical edges will likely produce poor performance as each query to the dominator tree will use the slow query path. This happens a lot in passes like MachineSink and PHIElimination. ** Proposed Solution ** Splitting a critical edge is a local modification of the CFG. Moreover, as soon as a critical edge is split, it is not critical anymore and thus cannot be a candidate for critical edge splitting anymore. In other words, the predecessor and successor of a basic block inserted on a critical edge cannot be inserted by critical edge splitting. Using these observations, we can pile up the splitting of critical edge and apply then at once before updating the DT information. The core of this patch moves the update of the MachineDominatorTree information from MachineBasicBlock::SplitCriticalEdge to a lazy MachineDominatorTree. ** Performance ** Thanks to this patch, the motivating example compiles in 4- minutes instead of 6+ minutes. No test case added as the motivating example as nothing special but being huge! The binaries are strictly identical for all the llvm test-suite + SPECs with and without this patch for both Os and O3. Regarding compile time, I observed only noise, although on average I saw a small improvement. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@215576 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/CodeGen/MachineDominators.h | 142 ++++++++++++++++++++++- lib/CodeGen/MachineBasicBlock.cpp | 27 +---- lib/CodeGen/MachineDominators.cpp | 2 + 3 files changed, 145 insertions(+), 26 deletions(-) diff --git a/include/llvm/CodeGen/MachineDominators.h b/include/llvm/CodeGen/MachineDominators.h index f1ae0bf5f9c..879ae861510 100644 --- a/include/llvm/CodeGen/MachineDominators.h +++ b/include/llvm/CodeGen/MachineDominators.h @@ -15,6 +15,7 @@ #ifndef LLVM_CODEGEN_MACHINEDOMINATORS_H #define LLVM_CODEGEN_MACHINEDOMINATORS_H +#include "llvm/ADT/SmallSet.h" #include "llvm/CodeGen/MachineBasicBlock.h" #include "llvm/CodeGen/MachineFunction.h" #include "llvm/CodeGen/MachineFunctionPass.h" @@ -38,6 +39,103 @@ typedef DomTreeNodeBase MachineDomTreeNode; /// compute a normal dominator tree. /// class MachineDominatorTree : public MachineFunctionPass { + /// \brief Helper structure used to hold all the basic blocks + /// involved in the split of a critical edge. + struct CriticalEdge { + MachineBasicBlock *FromBB; + MachineBasicBlock *ToBB; + MachineBasicBlock *NewBB; + CriticalEdge(MachineBasicBlock *FromBB, MachineBasicBlock *ToBB, + MachineBasicBlock *NewBB) + : FromBB(FromBB), ToBB(ToBB), NewBB(NewBB) {} + }; + + /// \brief Pile up all the critical edges to be split. + /// The splitting of a critical edge is local and thus, it is possible + /// to apply several of those changes at the same time. + mutable SmallVector CriticalEdgesToSplit; + /// \brief Remember all the basic blocks that are inserted during + /// edge splitting. + /// Invariant: NewBBs == all the basic blocks contained in the NewBB + /// field of all the elements of CriticalEdgesToSplit. + /// I.e., forall elt in CriticalEdgesToSplit, it exists BB in NewBBs + /// such as BB == elt.NewBB. + mutable SmallSet NewBBs; + + /// \brief Apply all the recorded critical edges to the DT. + /// This updates the underlying DT information in a way that uses + /// the fast query path of DT as much as possible. + /// + /// \post CriticalEdgesToSplit.empty(). + void applySplitCriticalEdges() const { + // Bail out early if there is nothing to do. + if (CriticalEdgesToSplit.empty()) + return; + + // For each element in CriticalEdgesToSplit, remember whether or + // not element is the new immediate domminator of its successor. + // The mapping is done by index, i.e., the information for the ith + // element of CriticalEdgesToSplit is the ith element of IsNewIDom. + SmallVector IsNewIDom; + IsNewIDom.resize(CriticalEdgesToSplit.size()); + size_t Idx = 0; + + // Collect all the dominance properties info, before invalidating + // the underlying DT. + for (CriticalEdge &Edge : CriticalEdgesToSplit) { + // Update dominator information. + MachineBasicBlock *Succ = Edge.ToBB; + MachineDomTreeNode *SucccDTNode = DT->getNode(Succ); + + IsNewIDom[Idx] = true; + for (MachineBasicBlock *PredBB : Succ->predecessors()) { + if (PredBB == Edge.NewBB) + continue; + // If we are in this situation: + // FromBB1 FromBB2 + // + + + // + + + + + // + + + + + // ... Split1 Split2 ... + // + + + // + + + // + + // Succ + // Instead of checking the domiance property with Split2, we + // check it with FromBB2 since Split2 is still unknown of the + // underlying DT structure. + if (NewBBs.count(PredBB)) { + assert(PredBB->pred_size() == 1 && "A basic block resulting from a " + "critical edge split has more " + "than one predecessor!"); + PredBB = *PredBB->pred_begin(); + } + if (!DT->dominates(SucccDTNode, DT->getNode(PredBB))) { + IsNewIDom[Idx] = false; + break; + } + } + ++Idx; + } + + // Now, update DT with the collected dominance properties info. + Idx = 0; + for (CriticalEdge &Edge : CriticalEdgesToSplit) { + // We know FromBB dominates NewBB. + MachineDomTreeNode *NewDTNode = DT->addNewBlock(Edge.NewBB, Edge.FromBB); + MachineDomTreeNode *SucccDTNode = DT->getNode(Edge.ToBB); + + // If all the other predecessors of "Succ" are dominated by "Succ" itself + // then the new block is the new immediate dominator of "Succ". Otherwise, + // the new block doesn't dominate anything. + if (IsNewIDom[Idx]) + DT->changeImmediateDominator(SucccDTNode, NewDTNode); + ++Idx; + } + NewBBs.clear(); + CriticalEdgesToSplit.clear(); + } + public: static char ID; // Pass ID, replacement for typeid DominatorTreeBase* DT; @@ -46,7 +144,10 @@ public: ~MachineDominatorTree(); - DominatorTreeBase& getBase() { return *DT; } + DominatorTreeBase &getBase() { + applySplitCriticalEdges(); + return *DT; + } void getAnalysisUsage(AnalysisUsage &AU) const override; @@ -55,14 +156,17 @@ public: /// dominators, this will always be a single block (the entry node). /// inline const std::vector &getRoots() const { + applySplitCriticalEdges(); return DT->getRoots(); } inline MachineBasicBlock *getRoot() const { + applySplitCriticalEdges(); return DT->getRoot(); } inline MachineDomTreeNode *getRootNode() const { + applySplitCriticalEdges(); return DT->getRootNode(); } @@ -70,17 +174,20 @@ public: inline bool dominates(const MachineDomTreeNode* A, const MachineDomTreeNode* B) const { + applySplitCriticalEdges(); return DT->dominates(A, B); } inline bool dominates(const MachineBasicBlock* A, const MachineBasicBlock* B) const { + applySplitCriticalEdges(); return DT->dominates(A, B); } // dominates - Return true if A dominates B. This performs the // special checks necessary if A and B are in the same basic block. bool dominates(const MachineInstr *A, const MachineInstr *B) const { + applySplitCriticalEdges(); const MachineBasicBlock *BBA = A->getParent(), *BBB = B->getParent(); if (BBA != BBB) return DT->dominates(BBA, BBB); @@ -100,11 +207,13 @@ public: inline bool properlyDominates(const MachineDomTreeNode* A, const MachineDomTreeNode* B) const { + applySplitCriticalEdges(); return DT->properlyDominates(A, B); } inline bool properlyDominates(const MachineBasicBlock* A, const MachineBasicBlock* B) const { + applySplitCriticalEdges(); return DT->properlyDominates(A, B); } @@ -112,10 +221,12 @@ public: /// for basic block A and B. If there is no such block then return NULL. inline MachineBasicBlock *findNearestCommonDominator(MachineBasicBlock *A, MachineBasicBlock *B) { + applySplitCriticalEdges(); return DT->findNearestCommonDominator(A, B); } inline MachineDomTreeNode *operator[](MachineBasicBlock *BB) const { + applySplitCriticalEdges(); return DT->getNode(BB); } @@ -123,6 +234,7 @@ public: /// block. This is the same as using operator[] on this class. /// inline MachineDomTreeNode *getNode(MachineBasicBlock *BB) const { + applySplitCriticalEdges(); return DT->getNode(BB); } @@ -131,6 +243,7 @@ public: /// the children list of the immediate dominator. inline MachineDomTreeNode *addNewBlock(MachineBasicBlock *BB, MachineBasicBlock *DomBB) { + applySplitCriticalEdges(); return DT->addNewBlock(BB, DomBB); } @@ -139,11 +252,13 @@ public: /// inline void changeImmediateDominator(MachineBasicBlock *N, MachineBasicBlock* NewIDom) { + applySplitCriticalEdges(); DT->changeImmediateDominator(N, NewIDom); } inline void changeImmediateDominator(MachineDomTreeNode *N, MachineDomTreeNode* NewIDom) { + applySplitCriticalEdges(); DT->changeImmediateDominator(N, NewIDom); } @@ -151,24 +266,49 @@ public: /// dominate any other blocks. Removes node from its immediate dominator's /// children list. Deletes dominator node associated with basic block BB. inline void eraseNode(MachineBasicBlock *BB) { + applySplitCriticalEdges(); DT->eraseNode(BB); } /// splitBlock - BB is split and now it has one successor. Update dominator /// tree to reflect this change. inline void splitBlock(MachineBasicBlock* NewBB) { + applySplitCriticalEdges(); DT->splitBlock(NewBB); } /// isReachableFromEntry - Return true if A is dominated by the entry /// block of the function containing it. bool isReachableFromEntry(const MachineBasicBlock *A) { + applySplitCriticalEdges(); return DT->isReachableFromEntry(A); } void releaseMemory() override; void print(raw_ostream &OS, const Module*) const override; + + /// \brief Record that the critical edge (FromBB, ToBB) has been + /// split with NewBB. + /// This is best to use this method instead of directly update the + /// underlying information, because this helps mitigating the + /// number of time the DT information is invalidated. + /// + /// \note Do not use this method with regular edges. + /// + /// \note To benefit from the compile time improvement incurred by this + /// method, the users of this method have to limit the queries to the DT + /// interface between two edges splitting. In other words, they have to + /// pack the splitting of critical edges as much as possible. + void recordSplitCriticalEdge(MachineBasicBlock *FromBB, + MachineBasicBlock *ToBB, + MachineBasicBlock *NewBB) { + bool Inserted = NewBBs.insert(NewBB); + (void)Inserted; + assert(Inserted && + "A basic block inserted via edge splitting cannot appear twice"); + CriticalEdgesToSplit.push_back(CriticalEdge(FromBB, ToBB, NewBB)); + } }; //===------------------------------------- diff --git a/lib/CodeGen/MachineBasicBlock.cpp b/lib/CodeGen/MachineBasicBlock.cpp index ebf3be80257..18d034aeaa9 100644 --- a/lib/CodeGen/MachineBasicBlock.cpp +++ b/lib/CodeGen/MachineBasicBlock.cpp @@ -906,31 +906,8 @@ MachineBasicBlock::SplitCriticalEdge(MachineBasicBlock *Succ, Pass *P) { } if (MachineDominatorTree *MDT = - P->getAnalysisIfAvailable()) { - // Update dominator information. - MachineDomTreeNode *SucccDTNode = MDT->getNode(Succ); - - bool IsNewIDom = true; - for (const_pred_iterator PI = Succ->pred_begin(), E = Succ->pred_end(); - PI != E; ++PI) { - MachineBasicBlock *PredBB = *PI; - if (PredBB == NMBB) - continue; - if (!MDT->dominates(SucccDTNode, MDT->getNode(PredBB))) { - IsNewIDom = false; - break; - } - } - - // We know "this" dominates the newly created basic block. - MachineDomTreeNode *NewDTNode = MDT->addNewBlock(NMBB, this); - - // If all the other predecessors of "Succ" are dominated by "Succ" itself - // then the new block is the new immediate dominator of "Succ". Otherwise, - // the new block doesn't dominate anything. - if (IsNewIDom) - MDT->changeImmediateDominator(SucccDTNode, NewDTNode); - } + P->getAnalysisIfAvailable()) + MDT->recordSplitCriticalEdge(this, Succ, NMBB); if (MachineLoopInfo *MLI = P->getAnalysisIfAvailable()) if (MachineLoop *TIL = MLI->getLoopFor(this)) { diff --git a/lib/CodeGen/MachineDominators.cpp b/lib/CodeGen/MachineDominators.cpp index 04c8ecbf9bd..df60cf34b61 100644 --- a/lib/CodeGen/MachineDominators.cpp +++ b/lib/CodeGen/MachineDominators.cpp @@ -35,6 +35,8 @@ void MachineDominatorTree::getAnalysisUsage(AnalysisUsage &AU) const { } bool MachineDominatorTree::runOnMachineFunction(MachineFunction &F) { + CriticalEdgesToSplit.clear(); + NewBBs.clear(); DT->recalculate(F); return false; -- 2.34.1