[GenericDomTree] Change GenericDomTree to use NodeRef in GraphTraits. NFC.

timshen91 · timshen91 · commit 1dca602a3731 · 2016-08-17T20:01:58.000Z
Summary: Looking at the implementation, GenericDomTree has more specific requirements on NodeRef, e.g. NodeRefObject->getParent() should compile, and NodeRef should be a pointer. We can remove the pointer requirement, but it seems to have little gain, given the limited use cases. Also changed GraphTraits<Inverse<Inverse<T>> to be more accurate. Reviewers: dblaikie, chandlerc Subscribers: llvm-commits Differential Revision: https://reviews.llvm.org/D23593 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@278961 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/llvm/ADT/GraphTraits.h b/include/llvm/ADT/GraphTraits.h
@@ -88,23 +88,7 @@ struct Inverse {
 
 // Provide a partial specialization of GraphTraits so that the inverse of an
 // inverse falls back to the original graph.
-template<class T>
-struct GraphTraits<Inverse<Inverse<T> > > {
-  typedef typename GraphTraits<T>::NodeType NodeType;
-  typedef typename GraphTraits<T>::ChildIteratorType ChildIteratorType;
-
-  static NodeType *getEntryNode(Inverse<Inverse<T> > *G) {
-    return GraphTraits<T>::getEntryNode(G->Graph.Graph);
-  }
-
-  static ChildIteratorType child_begin(NodeType* N) {
-    return GraphTraits<T>::child_begin(N);
-  }
-
-  static ChildIteratorType child_end(NodeType* N) {
-    return GraphTraits<T>::child_end(N);
-  }
-};
+template <class T> struct GraphTraits<Inverse<Inverse<T>>> : GraphTraits<T> {};
 
 } // End llvm namespace
 
diff --git a/include/llvm/IR/Dominators.h b/include/llvm/IR/Dominators.h
@@ -33,9 +33,9 @@ extern template class DomTreeNodeBase<BasicBlock>;
 extern template class DominatorTreeBase<BasicBlock>;
 
 extern template void Calculate<Function, BasicBlock *>(
-    DominatorTreeBase<GraphTraits<BasicBlock *>::NodeType> &DT, Function &F);
+    DominatorTreeBaseByGraphTraits<GraphTraits<BasicBlock *>> &DT, Function &F);
 extern template void Calculate<Function, Inverse<BasicBlock *>>(
-    DominatorTreeBase<GraphTraits<Inverse<BasicBlock *>>::NodeType> &DT,
+    DominatorTreeBaseByGraphTraits<GraphTraits<Inverse<BasicBlock *>>> &DT,
     Function &F);
 
 typedef DomTreeNodeBase<BasicBlock> DomTreeNode;
diff --git a/include/llvm/Support/GenericDomTree.h b/include/llvm/Support/GenericDomTree.h
@@ -13,6 +13,12 @@
 /// dominance queries on the CFG, but is fully generic w.r.t. the underlying
 /// graph types.
 ///
+/// Unlike ADT/* graph algorithms, generic dominator tree has more reuiqrement
+/// on the graph's NodeRef. The NodeRef should be a pointer and, depending on
+/// the implementation, e.g. NodeRef->getParent() return the parent node.
+///
+/// FIXME: Maybe GenericDomTree needs a TreeTraits, instead of GraphTraits.
+///
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_SUPPORT_GENERICDOMTREE_H
@@ -30,6 +36,23 @@
 
 namespace llvm {
 
+template <class NodeT> class DominatorTreeBase;
+
+namespace detail {
+
+template <typename GT> struct DominatorTreeBaseTraits {
+  static_assert(std::is_pointer<typename GT::NodeRef>::value,
+                "Currently NodeRef must be a pointer type.");
+  using type = DominatorTreeBase<
+      typename std::remove_pointer<typename GT::NodeRef>::type>;
+};
+
+} // End namespace detail
+
+template <typename GT>
+using DominatorTreeBaseByGraphTraits =
+    typename detail::DominatorTreeBaseTraits<GT>::type;
+
 /// \brief Base class that other, more interesting dominator analyses
 /// inherit from.
 template <class NodeT> class DominatorBase {
@@ -62,7 +85,6 @@ template <class NodeT> class DominatorBase {
   bool isPostDominator() const { return IsPostDominators; }
 };
 
-template <class NodeT> class DominatorTreeBase;
 struct PostDominatorTree;
 
 /// \brief Base class for the actual dominator tree node.
@@ -177,8 +199,7 @@ void PrintDomTree(const DomTreeNodeBase<NodeT> *N, raw_ostream &o,
 
 // The calculate routine is provided in a separate header but referenced here.
 template <class FuncT, class N>
-void Calculate(DominatorTreeBase<typename GraphTraits<N>::NodeType> &DT,
-               FuncT &F);
+void Calculate(DominatorTreeBaseByGraphTraits<GraphTraits<N>> &DT, FuncT &F);
 
 /// \brief Core dominator tree base class.
 ///
@@ -251,14 +272,14 @@ template <class NodeT> class DominatorTreeBase : public DominatorBase<NodeT> {
   // NewBB is split and now it has one successor. Update dominator tree to
   // reflect this change.
   template <class N, class GraphT>
-  void Split(DominatorTreeBase<typename GraphT::NodeType> &DT,
-             typename GraphT::NodeType *NewBB) {
+  void Split(DominatorTreeBaseByGraphTraits<GraphT> &DT,
+             typename GraphT::NodeRef NewBB) {
     assert(std::distance(GraphT::child_begin(NewBB),
                          GraphT::child_end(NewBB)) == 1 &&
            "NewBB should have a single successor!");
-    typename GraphT::NodeType *NewBBSucc = *GraphT::child_begin(NewBB);
+    typename GraphT::NodeRef NewBBSucc = *GraphT::child_begin(NewBB);
 
-    std::vector<typename GraphT::NodeType *> PredBlocks;
+    std::vector<typename GraphT::NodeRef> PredBlocks;
     typedef GraphTraits<Inverse<N>> InvTraits;
     for (typename InvTraits::ChildIteratorType
              PI = InvTraits::child_begin(NewBB),
@@ -273,7 +294,7 @@ template <class NodeT> class DominatorTreeBase : public DominatorBase<NodeT> {
              PI = InvTraits::child_begin(NewBBSucc),
              E = InvTraits::child_end(NewBBSucc);
          PI != E; ++PI) {
-      typename InvTraits::NodeType *ND = *PI;
+      typename InvTraits::NodeRef ND = *PI;
       if (ND != NewBB && !DT.dominates(NewBBSucc, ND) &&
           DT.isReachableFromEntry(ND)) {
         NewBBDominatesNewBBSucc = false;
@@ -627,18 +648,17 @@ template <class NodeT> class DominatorTreeBase : public DominatorBase<NodeT> {
 
 protected:
   template <class GraphT>
-  friend typename GraphT::NodeType *
-  Eval(DominatorTreeBase<typename GraphT::NodeType> &DT,
-       typename GraphT::NodeType *V, unsigned LastLinked);
+  friend typename GraphT::NodeRef
+  Eval(DominatorTreeBaseByGraphTraits<GraphT> &DT, typename GraphT::NodeRef V,
+       unsigned LastLinked);
 
   template <class GraphT>
-  friend unsigned DFSPass(DominatorTreeBase<typename GraphT::NodeType> &DT,
-                          typename GraphT::NodeType *V, unsigned N);
+  friend unsigned DFSPass(DominatorTreeBaseByGraphTraits<GraphT> &DT,
+                          typename GraphT::NodeRef V, unsigned N);
 
   template <class FuncT, class N>
-  friend void
-  Calculate(DominatorTreeBase<typename GraphTraits<N>::NodeType> &DT, FuncT &F);
-
+  friend void Calculate(DominatorTreeBaseByGraphTraits<GraphTraits<N>> &DT,
+                        FuncT &F);
 
   DomTreeNodeBase<NodeT> *getNodeForBlock(NodeT *BB) {
     if (DomTreeNodeBase<NodeT> *Node = getNode(BB))
diff --git a/include/llvm/Support/GenericDomTreeConstruction.h b/include/llvm/Support/GenericDomTreeConstruction.h
@@ -29,9 +29,9 @@
 
 namespace llvm {
 
-template<class GraphT>
-unsigned DFSPass(DominatorTreeBase<typename GraphT::NodeType>& DT,
-                 typename GraphT::NodeType* V, unsigned N) {
+template <class GraphT>
+unsigned DFSPass(DominatorTreeBaseByGraphTraits<GraphT> &DT,
+                 typename GraphT::NodeRef V, unsigned N) {
   // This is more understandable as a recursive algorithm, but we can't use the
   // recursive algorithm due to stack depth issues.  Keep it here for
   // documentation purposes.
@@ -52,15 +52,16 @@ unsigned DFSPass(DominatorTreeBase<typename GraphT::NodeType>& DT,
 #else
   bool IsChildOfArtificialExit = (N != 0);
 
-  SmallVector<std::pair<typename GraphT::NodeType*,
-                        typename GraphT::ChildIteratorType>, 32> Worklist;
+  SmallVector<
+      std::pair<typename GraphT::NodeRef, typename GraphT::ChildIteratorType>,
+      32>
+      Worklist;
   Worklist.push_back(std::make_pair(V, GraphT::child_begin(V)));
   while (!Worklist.empty()) {
-    typename GraphT::NodeType* BB = Worklist.back().first;
+    typename GraphT::NodeRef BB = Worklist.back().first;
     typename GraphT::ChildIteratorType NextSucc = Worklist.back().second;
 
-    typename DominatorTreeBase<typename GraphT::NodeType>::InfoRec &BBInfo =
-                                                                    DT.Info[BB];
+    auto &BBInfo = DT.Info[BB];
 
     // First time we visited this BB?
     if (NextSucc == GraphT::child_begin(BB)) {
@@ -89,10 +90,9 @@ unsigned DFSPass(DominatorTreeBase<typename GraphT::NodeType>& DT,
     ++Worklist.back().second;
 
     // Visit the successor next, if it isn't already visited.
-    typename GraphT::NodeType* Succ = *NextSucc;
+    typename GraphT::NodeRef Succ = *NextSucc;
 
-    typename DominatorTreeBase<typename GraphT::NodeType>::InfoRec &SuccVInfo =
-                                                                  DT.Info[Succ];
+    auto &SuccVInfo = DT.Info[Succ];
     if (SuccVInfo.Semi == 0) {
       SuccVInfo.Parent = BBDFSNum;
       Worklist.push_back(std::make_pair(Succ, GraphT::child_begin(Succ)));
@@ -103,25 +103,23 @@ unsigned DFSPass(DominatorTreeBase<typename GraphT::NodeType>& DT,
 }
 
 template <class GraphT>
-typename GraphT::NodeType *
-Eval(DominatorTreeBase<typename GraphT::NodeType> &DT,
-     typename GraphT::NodeType *VIn, unsigned LastLinked) {
-  typename DominatorTreeBase<typename GraphT::NodeType>::InfoRec &VInInfo =
-                                                                  DT.Info[VIn];
+typename GraphT::NodeRef Eval(DominatorTreeBaseByGraphTraits<GraphT> &DT,
+                              typename GraphT::NodeRef VIn,
+                              unsigned LastLinked) {
+  auto &VInInfo = DT.Info[VIn];
   if (VInInfo.DFSNum < LastLinked)
     return VIn;
 
-  SmallVector<typename GraphT::NodeType*, 32> Work;
-  SmallPtrSet<typename GraphT::NodeType*, 32> Visited;
+  SmallVector<typename GraphT::NodeRef, 32> Work;
+  SmallPtrSet<typename GraphT::NodeRef, 32> Visited;
 
   if (VInInfo.Parent >= LastLinked)
     Work.push_back(VIn);
 
   while (!Work.empty()) {
-    typename GraphT::NodeType* V = Work.back();
-    typename DominatorTreeBase<typename GraphT::NodeType>::InfoRec &VInfo =
-                                                                     DT.Info[V];
-    typename GraphT::NodeType* VAncestor = DT.Vertex[VInfo.Parent];
+    typename GraphT::NodeRef V = Work.back();
+    auto &VInfo = DT.Info[V];
+    typename GraphT::NodeRef VAncestor = DT.Vertex[VInfo.Parent];
 
     // Process Ancestor first
     if (Visited.insert(VAncestor).second && VInfo.Parent >= LastLinked) {
@@ -134,10 +132,9 @@ Eval(DominatorTreeBase<typename GraphT::NodeType> &DT,
     if (VInfo.Parent < LastLinked)
       continue;
 
-    typename DominatorTreeBase<typename GraphT::NodeType>::InfoRec &VAInfo =
-                                                             DT.Info[VAncestor];
-    typename GraphT::NodeType* VAncestorLabel = VAInfo.Label;
-    typename GraphT::NodeType* VLabel = VInfo.Label;
+    auto &VAInfo = DT.Info[VAncestor];
+    typename GraphT::NodeRef VAncestorLabel = VAInfo.Label;
+    typename GraphT::NodeRef VLabel = VInfo.Label;
     if (DT.Info[VAncestorLabel].Semi < DT.Info[VLabel].Semi)
       VInfo.Label = VAncestorLabel;
     VInfo.Parent = VAInfo.Parent;
@@ -146,16 +143,18 @@ Eval(DominatorTreeBase<typename GraphT::NodeType> &DT,
   return VInInfo.Label;
 }
 
-template<class FuncT, class NodeT>
-void Calculate(DominatorTreeBase<typename GraphTraits<NodeT>::NodeType>& DT,
-               FuncT& F) {
+template <class FuncT, class NodeT>
+void Calculate(DominatorTreeBaseByGraphTraits<GraphTraits<NodeT>> &DT,
+               FuncT &F) {
   typedef GraphTraits<NodeT> GraphT;
+  static_assert(std::is_pointer<typename GraphT::NodeRef>::value,
+                "NodeRef should be pointer type");
+  typedef typename std::remove_pointer<typename GraphT::NodeRef>::type NodeType;
 
   unsigned N = 0;
   bool MultipleRoots = (DT.Roots.size() > 1);
   if (MultipleRoots) {
-    typename DominatorTreeBase<typename GraphT::NodeType>::InfoRec &BBInfo =
-        DT.Info[nullptr];
+    auto &BBInfo = DT.Info[nullptr];
     BBInfo.DFSNum = BBInfo.Semi = ++N;
     BBInfo.Label = nullptr;
 
@@ -188,14 +187,13 @@ void Calculate(DominatorTreeBase<typename GraphTraits<NodeT>::NodeType>& DT,
     Buckets[i] = i;
 
   for (unsigned i = N; i >= 2; --i) {
-    typename GraphT::NodeType* W = DT.Vertex[i];
-    typename DominatorTreeBase<typename GraphT::NodeType>::InfoRec &WInfo =
-                                                                     DT.Info[W];
+    typename GraphT::NodeRef W = DT.Vertex[i];
+    auto &WInfo = DT.Info[W];
 
     // Step #2: Implicitly define the immediate dominator of vertices
     for (unsigned j = i; Buckets[j] != i; j = Buckets[j]) {
-      typename GraphT::NodeType* V = DT.Vertex[Buckets[j]];
-      typename GraphT::NodeType* U = Eval<GraphT>(DT, V, i + 1);
+      typename GraphT::NodeRef V = DT.Vertex[Buckets[j]];
+      typename GraphT::NodeRef U = Eval<GraphT>(DT, V, i + 1);
       DT.IDoms[V] = DT.Info[U].Semi < i ? U : W;
     }
 
@@ -207,7 +205,7 @@ void Calculate(DominatorTreeBase<typename GraphTraits<NodeT>::NodeType>& DT,
     for (typename InvTraits::ChildIteratorType CI =
          InvTraits::child_begin(W),
          E = InvTraits::child_end(W); CI != E; ++CI) {
-      typename InvTraits::NodeType *N = *CI;
+      typename InvTraits::NodeRef N = *CI;
       if (DT.Info.count(N)) {  // Only if this predecessor is reachable!
         unsigned SemiU = DT.Info[Eval<GraphT>(DT, N, i + 1)].Semi;
         if (SemiU < WInfo.Semi)
@@ -227,17 +225,17 @@ void Calculate(DominatorTreeBase<typename GraphTraits<NodeT>::NodeType>& DT,
   }
 
   if (N >= 1) {
-    typename GraphT::NodeType* Root = DT.Vertex[1];
+    typename GraphT::NodeRef Root = DT.Vertex[1];
     for (unsigned j = 1; Buckets[j] != 1; j = Buckets[j]) {
-      typename GraphT::NodeType* V = DT.Vertex[Buckets[j]];
+      typename GraphT::NodeRef V = DT.Vertex[Buckets[j]];
       DT.IDoms[V] = Root;
     }
   }
 
   // Step #4: Explicitly define the immediate dominator of each vertex
   for (unsigned i = 2; i <= N; ++i) {
-    typename GraphT::NodeType* W = DT.Vertex[i];
-    typename GraphT::NodeType*& WIDom = DT.IDoms[W];
+    typename GraphT::NodeRef W = DT.Vertex[i];
+    typename GraphT::NodeRef &WIDom = DT.IDoms[W];
     if (WIDom != DT.Vertex[DT.Info[W].Semi])
       WIDom = DT.IDoms[WIDom];
   }
@@ -248,34 +246,32 @@ void Calculate(DominatorTreeBase<typename GraphTraits<NodeT>::NodeType>& DT,
   // 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.
-  typename GraphT::NodeType* Root = !MultipleRoots ? DT.Roots[0] : nullptr;
+  typename GraphT::NodeRef Root = !MultipleRoots ? DT.Roots[0] : nullptr;
 
   DT.RootNode =
       (DT.DomTreeNodes[Root] =
-           llvm::make_unique<DomTreeNodeBase<typename GraphT::NodeType>>(
-               Root, nullptr)).get();
+           llvm::make_unique<DomTreeNodeBase<NodeType>>(Root, nullptr))
+          .get();
 
   // Loop over all of the reachable blocks in the function...
   for (unsigned i = 2; i <= N; ++i) {
-    typename GraphT::NodeType* W = DT.Vertex[i];
+    typename GraphT::NodeRef W = DT.Vertex[i];
 
     // Don't replace this with 'count', the insertion side effect is important
     if (DT.DomTreeNodes[W])
       continue; // Haven't calculated this node yet?
 
-    typename GraphT::NodeType* ImmDom = DT.getIDom(W);
+    typename GraphT::NodeRef ImmDom = DT.getIDom(W);
 
     assert(ImmDom || DT.DomTreeNodes[nullptr]);
 
     // Get or calculate the node for the immediate dominator
-    DomTreeNodeBase<typename GraphT::NodeType> *IDomNode =
-                                                     DT.getNodeForBlock(ImmDom);
+    DomTreeNodeBase<NodeType> *IDomNode = DT.getNodeForBlock(ImmDom);
 
     // Add a new tree node for this BasicBlock, and link it as a child of
     // IDomNode
     DT.DomTreeNodes[W] = IDomNode->addChild(
-        llvm::make_unique<DomTreeNodeBase<typename GraphT::NodeType>>(
-            W, IDomNode));
+        llvm::make_unique<DomTreeNodeBase<NodeType>>(W, IDomNode));
   }
 
   // Free temporary memory used to construct idom's
diff --git a/lib/IR/Dominators.cpp b/lib/IR/Dominators.cpp
@@ -64,9 +64,13 @@ template class llvm::DomTreeNodeBase<BasicBlock>;
 template class llvm::DominatorTreeBase<BasicBlock>;
 
 template void llvm::Calculate<Function, BasicBlock *>(
-    DominatorTreeBase<GraphTraits<BasicBlock *>::NodeType> &DT, Function &F);
+    DominatorTreeBase<
+        typename std::remove_pointer<GraphTraits<BasicBlock *>::NodeRef>::type>
+        &DT,
+    Function &F);
 template void llvm::Calculate<Function, Inverse<BasicBlock *>>(
-    DominatorTreeBase<GraphTraits<Inverse<BasicBlock *>>::NodeType> &DT,
+    DominatorTreeBase<typename std::remove_pointer<
+        GraphTraits<Inverse<BasicBlock *>>::NodeRef>::type> &DT,
     Function &F);
 
 // dominates - Return true if Def dominates a use in User. This performs
