[llvm] r306892 - [Dominators] Keep tree level in DomTreeNode and use it to find NCD and answer dominance queries
Rafael Avila de Espindola via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 30 15:40:55 PDT 2017
This is crashing on
CodeGen/ARM/2011-02-07-AntidepClobber.ll
Cheers,
Rafael
Jakub Kuderski via llvm-commits <llvm-commits at lists.llvm.org> writes:
> Author: kuhar
> Date: Fri Jun 30 14:51:40 2017
> New Revision: 306892
>
> URL: http://llvm.org/viewvc/llvm-project?rev=306892&view=rev
> Log:
> [Dominators] Keep tree level in DomTreeNode and use it to find NCD and answer dominance queries
>
> Summary:
> This patch makes DomTreeNodes keep their level (depth) in the DomTree. By having this information always available, it is possible to speedup and simplify findNearestCommonDominator and certain dominance queries.
>
> In the future, level information will be also needed to perform incremental updates.
>
> My testing doesn't show any noticeable performance differences after applying this patch. There may be some improvements when other passes are thought to use the level information.
>
> Reviewers: dberlin, sanjoy, chandlerc, grosser
>
> Reviewed By: dberlin
>
> Subscribers: llvm-commits
>
> Differential Revision: https://reviews.llvm.org/D34548
>
> Modified:
> llvm/trunk/include/llvm/Support/GenericDomTree.h
> llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h
> llvm/trunk/unittests/IR/DominatorTreeTest.cpp
>
> Modified: llvm/trunk/include/llvm/Support/GenericDomTree.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/GenericDomTree.h?rev=306892&r1=306891&r2=306892&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Support/GenericDomTree.h (original)
> +++ llvm/trunk/include/llvm/Support/GenericDomTree.h Fri Jun 30 14:51:40 2017
> @@ -65,12 +65,14 @@ template <class NodeT> class DomTreeNode
>
> NodeT *TheBB;
> DomTreeNodeBase *IDom;
> + unsigned Level;
> std::vector<DomTreeNodeBase *> Children;
> mutable unsigned DFSNumIn = ~0;
> mutable unsigned DFSNumOut = ~0;
>
> public:
> - DomTreeNodeBase(NodeT *BB, DomTreeNodeBase *iDom) : TheBB(BB), IDom(iDom) {}
> + DomTreeNodeBase(NodeT *BB, DomTreeNodeBase *iDom)
> + : TheBB(BB), IDom(iDom), Level(IDom ? IDom->Level + 1 : 0) {}
>
> using iterator = typename std::vector<DomTreeNodeBase *>::iterator;
> using const_iterator =
> @@ -83,6 +85,7 @@ template <class NodeT> class DomTreeNode
>
> NodeT *getBlock() const { return TheBB; }
> DomTreeNodeBase *getIDom() const { return IDom; }
> + unsigned getLevel() const { return Level; }
>
> const std::vector<DomTreeNodeBase *> &getChildren() const { return Children; }
>
> @@ -100,6 +103,8 @@ template <class NodeT> class DomTreeNode
> if (getNumChildren() != Other->getNumChildren())
> return true;
>
> + if (Level != Other->Level) return true;
> +
> SmallPtrSet<const NodeT *, 4> OtherChildren;
> for (const DomTreeNodeBase *I : *Other) {
> const NodeT *Nd = I->getBlock();
> @@ -116,18 +121,19 @@ template <class NodeT> class DomTreeNode
>
> void setIDom(DomTreeNodeBase *NewIDom) {
> assert(IDom && "No immediate dominator?");
> - if (IDom != NewIDom) {
> - typename std::vector<DomTreeNodeBase *>::iterator I =
> - find(IDom->Children, this);
> - assert(I != IDom->Children.end() &&
> - "Not in immediate dominator children set!");
> - // I am no longer your child...
> - IDom->Children.erase(I);
> -
> - // Switch to new dominator
> - IDom = NewIDom;
> - IDom->Children.push_back(this);
> - }
> + if (IDom == NewIDom) return;
> +
> + auto I = find(IDom->Children, this);
> + assert(I != IDom->Children.end() &&
> + "Not in immediate dominator children set!");
> + // I am no longer your child...
> + IDom->Children.erase(I);
> +
> + // Switch to new dominator
> + IDom = NewIDom;
> + IDom->Children.push_back(this);
> +
> + UpdateLevel();
> }
>
> /// getDFSNumIn/getDFSNumOut - These return the DFS visitation order for nodes
> @@ -143,6 +149,23 @@ private:
> return this->DFSNumIn >= other->DFSNumIn &&
> this->DFSNumOut <= other->DFSNumOut;
> }
> +
> + void UpdateLevel() {
> + assert(IDom);
> + if (Level == IDom->Level + 1) return;
> +
> + SmallVector<DomTreeNodeBase *, 64> WorkStack = {this};
> +
> + while (!WorkStack.empty()) {
> + DomTreeNodeBase *Current = WorkStack.pop_back_val();
> + Current->Level = Current->IDom->Level + 1;
> +
> + for (DomTreeNodeBase *C : *Current) {
> + assert(C->IDom);
> + if (C->Level != C->IDom->Level + 1) WorkStack.push_back(C);
> + }
> + }
> + }
> };
>
> template <class NodeT>
> @@ -152,9 +175,10 @@ raw_ostream &operator<<(raw_ostream &O,
> else
> O << " <<exit node>>";
>
> - O << " {" << Node->getDFSNumIn() << "," << Node->getDFSNumOut() << "}";
> + O << " {" << Node->getDFSNumIn() << "," << Node->getDFSNumOut() << "} ["
> + << Node->getLevel() << "]\n";
>
> - return O << "\n";
> + return O;
> }
>
> template <class NodeT>
> @@ -345,6 +369,13 @@ template <class NodeT> class DominatorTr
> if (!isReachableFromEntry(A))
> return false;
>
> + if (B->getIDom() == A) return true;
> +
> + if (A->getIDom() == B) return false;
> +
> + // A can only dominate B if it is higher in the tree.
> + if (A->getLevel() >= B->getLevel()) return false;
> +
> // Compare the result of the tree walk and the dfs numbers, if expensive
> // checks are enabled.
> #ifdef EXPENSIVE_CHECKS
> @@ -388,51 +419,18 @@ template <class NodeT> class DominatorTr
> return &Entry;
> }
>
> - // If B dominates A then B is nearest common dominator.
> - if (dominates(B, A))
> - return B;
> -
> - // If A dominates B then A is nearest common dominator.
> - if (dominates(A, B))
> - return A;
> -
> DomTreeNodeBase<NodeT> *NodeA = getNode(A);
> DomTreeNodeBase<NodeT> *NodeB = getNode(B);
>
> - // If we have DFS info, then we can avoid all allocations by just querying
> - // it from each IDom. Note that because we call 'dominates' twice above, we
> - // expect to call through this code at most 16 times in a row without
> - // building valid DFS information. This is important as below is a *very*
> - // slow tree walk.
> - if (DFSInfoValid) {
> - DomTreeNodeBase<NodeT> *IDomA = NodeA->getIDom();
> - while (IDomA) {
> - if (NodeB->DominatedBy(IDomA))
> - return IDomA->getBlock();
> - IDomA = IDomA->getIDom();
> - }
> - return nullptr;
> - }
> + // Use level information to go up the tree until the levels match. Then
> + // continue going up til we arrive at the same node.
> + while (NodeA && NodeA != NodeB) {
> + if (NodeA->getLevel() < NodeB->getLevel()) std::swap(NodeA, NodeB);
>
> - // Collect NodeA dominators set.
> - SmallPtrSet<DomTreeNodeBase<NodeT> *, 16> NodeADoms;
> - NodeADoms.insert(NodeA);
> - DomTreeNodeBase<NodeT> *IDomA = NodeA->getIDom();
> - while (IDomA) {
> - NodeADoms.insert(IDomA);
> - IDomA = IDomA->getIDom();
> + NodeA = NodeA->IDom;
> }
>
> - // Walk NodeB immediate dominators chain and find common dominator node.
> - DomTreeNodeBase<NodeT> *IDomB = NodeB->getIDom();
> - while (IDomB) {
> - if (NodeADoms.count(IDomB) != 0)
> - return IDomB->getBlock();
> -
> - IDomB = IDomB->getIDom();
> - }
> -
> - return nullptr;
> + return NodeA ? NodeA->getBlock() : nullptr;
> }
>
> const NodeT *findNearestCommonDominator(const NodeT *A, const NodeT *B) {
> @@ -481,8 +479,10 @@ template <class NodeT> class DominatorTr
> } else {
> assert(Roots.size() == 1);
> NodeT *OldRoot = Roots.front();
> - DomTreeNodes[OldRoot] =
> - NewNode->addChild(std::move(DomTreeNodes[OldRoot]));
> + auto &OldNode = DomTreeNodes[OldRoot];
> + OldNode = NewNode->addChild(std::move(DomTreeNodes[OldRoot]));
> + OldNode->IDom = NewNode;
> + OldNode->UpdateLevel();
> Roots[0] = BB;
> }
> return RootNode = NewNode;
>
> Modified: llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h?rev=306892&r1=306891&r2=306892&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h (original)
> +++ llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h Fri Jun 30 14:51:40 2017
> @@ -318,6 +318,39 @@ struct SemiNCAInfo {
> return true;
> }
>
> + // Check if for every parent with a level L in the tree all of its children
> + // have level L + 1.
> + static bool VerifyLevels(const DomTreeT &DT) {
> + for (auto &NodeToTN : DT.DomTreeNodes) {
> + const TreeNodePtr TN = NodeToTN.second.get();
> + const NodePtr BB = TN->getBlock();
> + if (!BB) continue;
> +
> + const TreeNodePtr IDom = TN->getIDom();
> + if (!IDom && TN->getLevel() != 0) {
> + errs() << "Node without an IDom ";
> + PrintBlockOrNullptr(errs(), BB);
> + errs() << " has a nonzero level " << TN->getLevel() << "!\n";
> + errs().flush();
> +
> + return false;
> + }
> +
> + if (IDom && TN->getLevel() != IDom->getLevel() + 1) {
> + errs() << "Node ";
> + PrintBlockOrNullptr(errs(), BB);
> + errs() << " has level " << TN->getLevel() << " while it's IDom ";
> + PrintBlockOrNullptr(errs(), IDom->getBlock());
> + errs() << " has level " << IDom->getLevel() << "!\n";
> + errs().flush();
> +
> + return false;
> + }
> + }
> +
> + return true;
> + }
> +
> // Checks if the tree has the parent property: if for all edges from V to W in
> // the input graph, such that V is reachable, the parent of W in the tree is
> // an ancestor of V in the tree.
> @@ -405,9 +438,8 @@ bool Verify(const DominatorTreeBaseByGra
> "NodePtr should be a pointer type");
> SemiNCAInfo<typename std::remove_pointer<NodePtr>::type> SNCA;
>
> - return SNCA.verifyReachability(DT) && SNCA.verifyParentProperty(DT) &&
> - SNCA.verifySiblingProperty(DT);
> -
> + return SNCA.verifyReachability(DT) && SNCA.VerifyLevels(DT) &&
> + SNCA.verifyParentProperty(DT) && SNCA.verifySiblingProperty(DT);
> }
>
> } // namespace DomTreeBuilder
>
> Modified: llvm/trunk/unittests/IR/DominatorTreeTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/IR/DominatorTreeTest.cpp?rev=306892&r1=306891&r2=306892&view=diff
> ==============================================================================
> --- llvm/trunk/unittests/IR/DominatorTreeTest.cpp (original)
> +++ llvm/trunk/unittests/IR/DominatorTreeTest.cpp Fri Jun 30 14:51:40 2017
> @@ -230,6 +230,12 @@ TEST(DominatorTree, Unreachable) {
> EXPECT_EQ(DT->getNode(BB4)->getDFSNumIn(), 3UL);
> EXPECT_EQ(DT->getNode(BB4)->getDFSNumOut(), 4UL);
>
> + // Check levels before
> + EXPECT_EQ(DT->getNode(BB0)->getLevel(), 0U);
> + EXPECT_EQ(DT->getNode(BB1)->getLevel(), 1U);
> + EXPECT_EQ(DT->getNode(BB2)->getLevel(), 1U);
> + EXPECT_EQ(DT->getNode(BB4)->getLevel(), 1U);
> +
> // Reattach block 3 to block 1 and recalculate
> BB1->getTerminator()->eraseFromParent();
> BranchInst::Create(BB4, BB3, ConstantInt::getTrue(F.getContext()), BB1);
> @@ -248,6 +254,13 @@ TEST(DominatorTree, Unreachable) {
> EXPECT_EQ(DT->getNode(BB4)->getDFSNumIn(), 5UL);
> EXPECT_EQ(DT->getNode(BB4)->getDFSNumOut(), 6UL);
>
> + // Check levels after
> + EXPECT_EQ(DT->getNode(BB0)->getLevel(), 0U);
> + EXPECT_EQ(DT->getNode(BB1)->getLevel(), 1U);
> + EXPECT_EQ(DT->getNode(BB2)->getLevel(), 1U);
> + EXPECT_EQ(DT->getNode(BB3)->getLevel(), 2U);
> + EXPECT_EQ(DT->getNode(BB4)->getLevel(), 1U);
> +
> // Change root node
> DT->verifyDomTree();
> BasicBlock *NewEntry =
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list