[llvm] r306892 - [Dominators] Keep tree level in DomTreeNode and use it to find NCD and answer dominance queries
Jakub (Kuba) Kuderski via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 30 15:57:44 PDT 2017
Reverted in r306907.
On Fri, Jun 30, 2017 at 3:52 PM, Jakub (Kuba) Kuderski <
kubakuderski at gmail.com> wrote:
> Reverting now.
>
> On Fri, Jun 30, 2017 at 3:40 PM, Rafael Avila de Espindola <
> rafael.espindola at gmail.com> wrote:
>
>> 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
>>
>
>
>
> --
> Jakub Kuderski
>
--
Jakub Kuderski
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170630/4824f6e9/attachment.html>
More information about the llvm-commits
mailing list