[Dominators] Move root-finding out of DomTreeBase and simplify it

Summary:
This patch moves root-finding logic from DominatorTreeBase to GenericDomTreeConstruction.h.
It makes the behavior simpler and more consistent by always adding a virtual root to PostDominatorTrees.

Reviewers: dberlin, davide, grosser, sanjoy

Reviewed By: dberlin

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D35597

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@309146 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/IR/Dominators.h b/include/llvm/IR/Dominators.h
index 5b21a2c83e4a8a48700a5201a5a24f8e91979a2f..66e89341978cd43b16f2cfb60975bc4c17c2bf86 100644 (file)
--- a/include/llvm/IR/Dominators.h
+++ b/include/llvm/IR/Dominators.h
@@ -41,9 +41,8 @@ namespace DomTreeBuilder {
 using BBDomTree = DomTreeBase<BasicBlock>;
 using BBPostDomTree = PostDomTreeBase<BasicBlock>;
 
-extern template void Calculate<BBDomTree, Function>(BBDomTree &DT, Function &F);
-extern template void Calculate<BBPostDomTree, Function>(BBPostDomTree &DT,
-                                                        Function &F);
+extern template void Calculate<BBDomTree>(BBDomTree &DT);
+extern template void Calculate<BBPostDomTree>(BBPostDomTree &DT);
 
 extern template void InsertEdge<BBDomTree>(BBDomTree &DT, BasicBlock *From,
                                            BasicBlock *To);

diff --git a/include/llvm/Support/GenericDomTree.h b/include/llvm/Support/GenericDomTree.h
index 706320fed9a727622990e4d9f1c5e8bf5b73acb9..b94310fd9162f1b0d1d9d5b0eecf7ba17335f28f 100644 (file)
--- a/include/llvm/Support/GenericDomTree.h
+++ b/include/llvm/Support/GenericDomTree.h
@@ -14,8 +14,8 @@
 /// graph types.
 ///
 /// Unlike ADT/* graph algorithms, generic dominator tree has more requirements
-/// on the graph's NodeRef. The NodeRef should be a pointer and, depending on
-/// the implementation, e.g. NodeRef->getParent() return the parent node.
+/// on the graph's NodeRef. The NodeRef should be a pointer and,
+/// NodeRef->getParent() must return the parent node that is also a pointer.
 ///
 /// FIXME: Maybe GenericDomTree needs a TreeTraits, instead of GraphTraits.
 ///
@@ -187,8 +187,8 @@ void PrintDomTree(const DomTreeNodeBase<NodeT> *N, raw_ostream &O,
 
 namespace DomTreeBuilder {
 // The routines below are provided in a separate header but referenced here.
-template <typename DomTreeT, typename FuncT>
-void Calculate(DomTreeT &DT, FuncT &F);
+template <typename DomTreeT>
+void Calculate(DomTreeT &DT);
 
 template <class DomTreeT>
 void InsertEdge(DomTreeT &DT, typename DomTreeT::NodePtr From,
@@ -208,6 +208,17 @@ bool Verify(const DomTreeT &DT);
 /// various graphs in the LLVM IR or in the code generator.
 template <typename NodeT, bool IsPostDom>
 class DominatorTreeBase {
+ public:
+  static_assert(std::is_pointer<typename GraphTraits<NodeT *>::NodeRef>::value,
+                "Currently DominatorTreeBase supports only pointer nodes");
+  using NodeType = NodeT;
+  using NodePtr = NodeT *;
+  using ParentPtr = decltype(std::declval<NodeT *>()->getParent());
+  static_assert(std::is_pointer<ParentPtr>::value,
+                "Currently NodeT's parent must be a pointer type");
+  using ParentType = typename std::remove_pointer<ParentPtr>::type;
+  static constexpr bool IsPostDominator = IsPostDom;
+
  protected:
   std::vector<NodeT *> Roots;
 
@@ -215,7 +226,6 @@ class DominatorTreeBase {
      DenseMap<NodeT *, std::unique_ptr<DomTreeNodeBase<NodeT>>>;
   DomTreeNodeMapType DomTreeNodes;
   DomTreeNodeBase<NodeT> *RootNode;
-  using ParentPtr = decltype(std::declval<NodeT *>()->getParent());
   ParentPtr Parent = nullptr;
 
   mutable bool DFSInfoValid = false;
@@ -224,12 +234,6 @@ class DominatorTreeBase {
   friend struct DomTreeBuilder::SemiNCAInfo<DominatorTreeBase>;
 
  public:
-  static_assert(std::is_pointer<typename GraphTraits<NodeT *>::NodeRef>::value,
-                "Currently DominatorTreeBase supports only pointer nodes");
-  using NodeType = NodeT;
-  using NodePtr = NodeT *;
-  static constexpr bool IsPostDominator = IsPostDom;
-
   DominatorTreeBase() {}
 
   DominatorTreeBase(DominatorTreeBase &&Arg)
@@ -650,23 +654,10 @@ public:
   }
 
   /// recalculate - compute a dominator tree for the given function
-  template <class FT> void recalculate(FT &F) {
-    using TraitsTy = GraphTraits<FT *>;
+  void recalculate(ParentType &Func) {
     reset();
-    Parent = &F;
-
-    if (!IsPostDominator) {
-      // Initialize root
-      NodeT *entry = TraitsTy::getEntryNode(&F);
-      addRoot(entry);
-    } else {
-      // Initialize the roots list
-      for (auto *Node : nodes(&F))
-        if (TraitsTy::child_begin(Node) == TraitsTy::child_end(Node))
-          addRoot(Node);
-    }
-
-    DomTreeBuilder::Calculate(*this, F);
+    Parent = &Func;
+    DomTreeBuilder::Calculate(*this);
   }
 
   /// verify - check parent and sibling property

diff --git a/include/llvm/Support/GenericDomTreeConstruction.h b/include/llvm/Support/GenericDomTreeConstruction.h
index be90afa4c3c8e25fbdf2c757eff25e9f85db7d00..6fd182b15c046a0efb6a50f1b89ddd6df29bb9cd 100644 (file)
--- a/include/llvm/Support/GenericDomTreeConstruction.h
+++ b/include/llvm/Support/GenericDomTreeConstruction.h
@@ -131,7 +131,7 @@ struct SemiNCAInfo {
   // Custom DFS implementation which can skip nodes based on a provided
   // predicate. It also collects ReverseChildren so that we don't have to spend
   // time getting predecessors in SemiNCA.
-  template <bool Inverse, typename DescendCondition>
+  template <typename DescendCondition>
   unsigned runDFS(NodePtr V, unsigned LastNum, DescendCondition Condition,
                   unsigned AttachToNum) {
     assert(V);
@@ -148,7 +148,7 @@ struct SemiNCAInfo {
       BBInfo.Label = BB;
       NumToNode.push_back(BB);
 
-      for (const NodePtr Succ : ChildrenGetter<NodePtr, Inverse>::Get(BB)) {
+      for (const NodePtr Succ : ChildrenGetter<NodePtr, IsPostDom>::Get(BB)) {
         const auto SIT = NodeToInfo.find(Succ);
         // Don't visit nodes more than once but remember to collect
         // RerverseChildren.
@@ -260,7 +260,8 @@ struct SemiNCAInfo {
   unsigned doFullDFSWalk(const DomTreeT &DT, DescendCondition DC) {
     unsigned Num = 0;
 
-    if (DT.Roots.size() > 1) {
+    // If the DT is a PostDomTree, always add a virtual root.
+    if (IsPostDom) {
       auto &BBInfo = NodeToInfo[nullptr];
       BBInfo.DFSNum = BBInfo.Semi = ++Num;
       BBInfo.Label = nullptr;
@@ -268,34 +269,42 @@ struct SemiNCAInfo {
       NumToNode.push_back(nullptr);  // NumToNode[n] = V;
     }
 
-    if (DT.isPostDominator()) {
-      for (auto *Root : DT.Roots) Num = runDFS<true>(Root, Num, DC, 1);
-    } else {
-      assert(DT.Roots.size() == 1);
-      Num = runDFS<false>(DT.Roots[0], Num, DC, Num);
-    }
+    const unsigned InitialNum = Num;
+    for (auto *Root : DT.Roots) Num = runDFS(Root, Num, DC, InitialNum);
 
     return Num;
   }
 
-  void calculateFromScratch(DomTreeT &DT, const unsigned NumBlocks) {
+  static void FindAndAddRoots(DomTreeT &DT) {
+    assert(DT.Parent && "Parent pointer is not set");
+    using TraitsTy = GraphTraits<typename DomTreeT::ParentPtr>;
+
+    if (!IsPostDom) {
+      // Dominators have a single root that is the function's entry.
+      NodeT *entry = TraitsTy::getEntryNode(DT.Parent);
+      DT.addRoot(entry);
+    } else {
+      // Initialize the roots list for PostDominators.
+      for (auto *Node : nodes(DT.Parent))
+        if (TraitsTy::child_begin(Node) == TraitsTy::child_end(Node))
+          DT.addRoot(Node);
+    }
+  }
+
+  void calculateFromScratch(DomTreeT &DT) {
     // Step #0: Number blocks in depth-first order and initialize variables used
     // in later stages of the algorithm.
-    const unsigned LastDFSNum = doFullDFSWalk(DT, AlwaysDescend);
+    FindAndAddRoots(DT);
+    doFullDFSWalk(DT, AlwaysDescend);
 
     runSemiNCA(DT);
 
     if (DT.Roots.empty()) return;
 
-    // Add a node for the root.  This node might be the actual root, if there is
-    // one exit block, or it may be the virtual exit (denoted by
-    // (BasicBlock *)0) which postdominates all real exits if there are multiple
-    // exit blocks, or an infinite loop.
-    // It might be that some blocks did not get a DFS number (e.g., blocks of
-    // infinite loops). In these cases an artificial exit node is required.
-    const bool MultipleRoots = DT.Roots.size() > 1 || (DT.isPostDominator() &&
-                                                       LastDFSNum != NumBlocks);
-    NodePtr Root = !MultipleRoots ? DT.Roots[0] : nullptr;
+    // Add a node for the root. If the tree is a PostDominatorTree it will be
+    // the virtual exit (denoted by (BasicBlock *) nullptr) which postdominates
+    // all real exits (including multiple exit blocks, infinite loops).
+    NodePtr Root = IsPostDom ? nullptr : DT.Roots[0];
 
     DT.RootNode = (DT.DomTreeNodes[Root] =
                        llvm::make_unique<DomTreeNodeBase<NodeT>>(Root, nullptr))
@@ -523,7 +532,7 @@ struct SemiNCAInfo {
     };
 
     SemiNCAInfo SNCA;
-    SNCA.runDFS<IsPostDom>(Root, 0, UnreachableDescender, 0);
+    SNCA.runDFS(Root, 0, UnreachableDescender, 0);
     SNCA.runSemiNCA(DT);
     SNCA.attachNewSubtree(DT, Incoming);
 
@@ -638,7 +647,7 @@ struct SemiNCAInfo {
     DEBUG(dbgs() << "\tTop of subtree: " << BlockNamePrinter(ToIDomTN) << "\n");
 
     SemiNCAInfo SNCA;
-    SNCA.runDFS<IsPostDom>(ToIDom, 0, DescendBelow, 0);
+    SNCA.runDFS(ToIDom, 0, DescendBelow, 0);
     DEBUG(dbgs() << "\tRunning Semi-NCA\n");
     SNCA.runSemiNCA(DT, Level);
     SNCA.reattachExistingSubtree(DT, PrevIDomSubTree);
@@ -692,7 +701,7 @@ struct SemiNCAInfo {
 
     SemiNCAInfo SNCA;
     unsigned LastDFSNum =
-        SNCA.runDFS<IsPostDom>(ToTN->getBlock(), 0, DescendAndCollect, 0);
+        SNCA.runDFS(ToTN->getBlock(), 0, DescendAndCollect, 0);
 
     TreeNodePtr MinNode = ToTN;
 
@@ -744,7 +753,7 @@ struct SemiNCAInfo {
       const TreeNodePtr ToTN = DT.getNode(To);
       return ToTN && ToTN->getLevel() > MinLevel;
     };
-    SNCA.runDFS<IsPostDom>(MinNode->getBlock(), 0, DescendBelow, 0);
+    SNCA.runDFS(MinNode->getBlock(), 0, DescendBelow, 0);
 
     DEBUG(dbgs() << "Previous IDom(MinNode) = " << BlockNamePrinter(PrevIDom)
                  << "\nRunning Semi-NCA\n");
@@ -945,11 +954,10 @@ struct SemiNCAInfo {
   }
 };
 
-
-template <class DomTreeT, class FuncT>
-void Calculate(DomTreeT &DT, FuncT &F) {
+template <class DomTreeT>
+void Calculate(DomTreeT &DT) {
   SemiNCAInfo<DomTreeT> SNCA;
-  SNCA.calculateFromScratch(DT, GraphTraits<FuncT *>::size(&F));
+  SNCA.calculateFromScratch(DT);
 }
 
 template <class DomTreeT>

diff --git a/lib/IR/Dominators.cpp b/lib/IR/Dominators.cpp
index 4d7e3040ecd7f93ab31f35f00e98c686f436749a..993ef1d7c22cd98373d0761c221ad54939c1eff4 100644 (file)
--- a/lib/IR/Dominators.cpp
+++ b/lib/IR/Dominators.cpp
@@ -64,12 +64,10 @@ template class llvm::DomTreeNodeBase<BasicBlock>;
 template class llvm::DominatorTreeBase<BasicBlock, false>; // DomTreeBase
 template class llvm::DominatorTreeBase<BasicBlock, true>; // PostDomTreeBase
 
-template void
-llvm::DomTreeBuilder::Calculate<DomTreeBuilder::BBDomTree, Function>(
-    DomTreeBuilder::BBDomTree &DT, Function &F);
-template void
-llvm::DomTreeBuilder::Calculate<DomTreeBuilder::BBPostDomTree, Function>(
-    DomTreeBuilder::BBPostDomTree &DT, Function &F);
+template void llvm::DomTreeBuilder::Calculate<DomTreeBuilder::BBDomTree>(
+    DomTreeBuilder::BBDomTree &DT);
+template void llvm::DomTreeBuilder::Calculate<DomTreeBuilder::BBPostDomTree>(
+    DomTreeBuilder::BBPostDomTree &DT);
 
 template void llvm::DomTreeBuilder::InsertEdge<DomTreeBuilder::BBDomTree>(
     DomTreeBuilder::BBDomTree &DT, BasicBlock *From, BasicBlock *To);