[llvm] r306919 - [Dominators] Reapply r306892, r306893, r306893.

Jakub Kuderski via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 30 17:23:01 PDT 2017


Author: kuhar
Date: Fri Jun 30 17:23:01 2017
New Revision: 306919

URL: http://llvm.org/viewvc/llvm-project?rev=306919&view=rev
Log:
[Dominators] Reapply r306892, r306893, r306893.

This reverts commit r306907 and reapplies the patches in the title.
The patches used to make one of the
CodeGen/ARM/2011-02-07-AntidepClobber.ll test to fail because of a
missing null check.

Modified:
    llvm/trunk/include/llvm/Analysis/IteratedDominanceFrontier.h
    llvm/trunk/include/llvm/Support/GenericDomTree.h
    llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h
    llvm/trunk/lib/Analysis/IteratedDominanceFrontier.cpp
    llvm/trunk/unittests/IR/DominatorTreeTest.cpp

Modified: llvm/trunk/include/llvm/Analysis/IteratedDominanceFrontier.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/IteratedDominanceFrontier.h?rev=306919&r1=306918&r2=306919&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/IteratedDominanceFrontier.h (original)
+++ llvm/trunk/include/llvm/Analysis/IteratedDominanceFrontier.h Fri Jun 30 17:23:01 2017
@@ -86,7 +86,6 @@ public:
 private:
   DominatorTreeBase<BasicBlock> &DT;
   bool useLiveIn;
-  DenseMap<DomTreeNode *, unsigned> DomLevels;
   const SmallPtrSetImpl<BasicBlock *> *LiveInBlocks;
   const SmallPtrSetImpl<BasicBlock *> *DefBlocks;
 };

Modified: llvm/trunk/include/llvm/Support/GenericDomTree.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/GenericDomTree.h?rev=306919&r1=306918&r2=306919&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/GenericDomTree.h (original)
+++ llvm/trunk/include/llvm/Support/GenericDomTree.h Fri Jun 30 17:23:01 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
@@ -376,7 +407,7 @@ template <class NodeT> class DominatorTr
 
   /// findNearestCommonDominator - Find nearest common dominator basic block
   /// for basic block A and B. If there is no such block then return NULL.
-  NodeT *findNearestCommonDominator(NodeT *A, NodeT *B) {
+  NodeT *findNearestCommonDominator(NodeT *A, NodeT *B) const {
     assert(A->getParent() == B->getParent() &&
            "Two blocks are not in same function");
 
@@ -388,54 +419,24 @@ 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;
-    }
-
-    // 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();
-    }
+    if (!NodeA || !NodeB) return nullptr;
 
-    // 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();
+    // 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);
 
-      IDomB = IDomB->getIDom();
+      NodeA = NodeA->IDom;
     }
 
-    return nullptr;
+    return NodeA ? NodeA->getBlock() : nullptr;
   }
 
-  const NodeT *findNearestCommonDominator(const NodeT *A, const NodeT *B) {
+  const NodeT *findNearestCommonDominator(const NodeT *A,
+                                          const NodeT *B) const {
     // Cast away the const qualifiers here. This is ok since
     // const is re-introduced on the return type.
     return findNearestCommonDominator(const_cast<NodeT *>(A),
@@ -481,8 +482,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=306919&r1=306918&r2=306919&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h (original)
+++ llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h Fri Jun 30 17:23:01 2017
@@ -280,6 +280,7 @@ struct SemiNCAInfo {
   }
 
   void doFullDFSWalk(const DomTreeT &DT) {
+    NumToNode.push_back(nullptr);
     unsigned Num = 0;
     for (auto *Root : DT.Roots)
       if (!DT.isPostDominator())
@@ -317,6 +318,78 @@ 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 for every edge From -> To in the graph
+  //     NCD(From, To) == IDom(To) or To.
+  bool verifyNCD(const DomTreeT &DT) {
+    clear();
+    doFullDFSWalk(DT);
+
+    for (auto &BlockToInfo : NodeToInfo) {
+      auto &Info = BlockToInfo.second;
+
+      const NodePtr From = NumToNode[Info.Parent];
+      if (!From) continue;
+
+      const NodePtr To = BlockToInfo.first;
+      const TreeNodePtr ToTN = DT.getNode(To);
+      assert(ToTN);
+
+      const NodePtr NCD = DT.findNearestCommonDominator(From, To);
+      const TreeNodePtr NCDTN = NCD ? DT.getNode(NCD) : nullptr;
+      const TreeNodePtr ToIDom = ToTN->getIDom();
+      if (NCDTN != ToTN && NCDTN != ToIDom) {
+        errs() << "NearestCommonDominator verification failed:\n\tNCD(From:";
+        PrintBlockOrNullptr(errs(), From);
+        errs() << ", To:";
+        PrintBlockOrNullptr(errs(), To);
+        errs() << ") = ";
+        PrintBlockOrNullptr(errs(), NCD);
+        errs() << ",\t (should be To or IDom[To]: ";
+        PrintBlockOrNullptr(errs(), ToIDom ? ToIDom->getBlock() : nullptr);
+        errs() << ")\n";
+        errs().flush();
+
+        return false;
+      }
+    }
+
+    return true;
+  }
+
   // The below routines verify the correctness of the dominator tree relative to
   // the CFG it's coming from.  A tree is a dominator tree iff it has two
   // properties, called the parent property and the sibling property.  Tarjan
@@ -441,9 +514,9 @@ 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) &&
+  return SNCA.verifyReachability(DT) && SNCA.VerifyLevels(DT) &&
+         SNCA.verifyNCD(DT) && SNCA.verifyParentProperty(DT) &&
          SNCA.verifySiblingProperty(DT);
-
 }
 
 }  // namespace DomTreeBuilder

Modified: llvm/trunk/lib/Analysis/IteratedDominanceFrontier.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/IteratedDominanceFrontier.cpp?rev=306919&r1=306918&r2=306919&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/IteratedDominanceFrontier.cpp (original)
+++ llvm/trunk/lib/Analysis/IteratedDominanceFrontier.cpp Fri Jun 30 17:23:01 2017
@@ -20,14 +20,6 @@ namespace llvm {
 template <class NodeTy>
 void IDFCalculator<NodeTy>::calculate(
     SmallVectorImpl<BasicBlock *> &PHIBlocks) {
-  // If we haven't computed dominator tree levels, do so now.
-  if (DomLevels.empty()) {
-    for (auto DFI = df_begin(DT.getRootNode()), DFE = df_end(DT.getRootNode());
-         DFI != DFE; ++DFI) {
-      DomLevels[*DFI] = DFI.getPathLength() - 1;
-    }
-  }
-
   // Use a priority queue keyed on dominator tree level so that inserted nodes
   // are handled from the bottom of the dominator tree upwards.
   typedef std::pair<DomTreeNode *, unsigned> DomTreeNodePair;
@@ -37,7 +29,7 @@ void IDFCalculator<NodeTy>::calculate(
 
   for (BasicBlock *BB : *DefBlocks) {
     if (DomTreeNode *Node = DT.getNode(BB))
-      PQ.push(std::make_pair(Node, DomLevels.lookup(Node)));
+      PQ.push({Node, Node->getLevel()});
   }
 
   SmallVector<DomTreeNode *, 32> Worklist;
@@ -72,7 +64,7 @@ void IDFCalculator<NodeTy>::calculate(
         if (SuccNode->getIDom() == Node)
           continue;
 
-        unsigned SuccLevel = DomLevels.lookup(SuccNode);
+        const unsigned SuccLevel = SuccNode->getLevel();
         if (SuccLevel > RootLevel)
           continue;
 

Modified: llvm/trunk/unittests/IR/DominatorTreeTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/IR/DominatorTreeTest.cpp?rev=306919&r1=306918&r2=306919&view=diff
==============================================================================
--- llvm/trunk/unittests/IR/DominatorTreeTest.cpp (original)
+++ llvm/trunk/unittests/IR/DominatorTreeTest.cpp Fri Jun 30 17:23:01 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 =




More information about the llvm-commits mailing list