Cleanup some GraphTraits iteration code

Use children<> and nodes<> in appropriate places to cleanup the code.

Also, as part of the cleanup,
change the signature of DominatorTreeBase's Split.
It is a protected non-virtual member function called only twice,
both from within the class,
and the removed passed argument in both cases is '*this'.
The reason for the existence of that argument seems to be that
back before r43115 Split was a free function,
so an argument to get '*this' was needed - but now that is no longer the
case.

Patch by Yoav Ben-Shalom!

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

llvm-svn: 300656

llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h

@@ -1164,9 +1164,8 @@ template <class BT> struct BlockEdgesAdder {
   void operator()(IrreducibleGraph &G, IrreducibleGraph::IrrNode &Irr,
                   const LoopData *OuterLoop) {
     const BlockT *BB = BFI.RPOT[Irr.Node.Index];
-    for (auto I = Successor::child_begin(BB), E = Successor::child_end(BB);
-         I != E; ++I)
-      G.addEdge(Irr, BFI.getNode(*I), OuterLoop);
+    for (const auto Succ : children<const BlockT *>(BB))
+      G.addEdge(Irr, BFI.getNode(Succ), OuterLoop);
   }
 };
 }
@@ -1210,10 +1209,9 @@ BlockFrequencyInfoImpl<BT>::propagateMassToSuccessors(LoopData *OuterLoop,
       return false;
   } else {
     const BlockT *BB = getBlock(Node);
-    for (auto SI = Successor::child_begin(BB), SE = Successor::child_end(BB);
-         SI != SE; ++SI)
-      if (!addToDist(Dist, OuterLoop, Node, getNode(*SI),
-                     getWeightFromBranchProb(BPI->getEdgeProbability(BB, SI))))
+    for (const auto Succ : children<const BlockT *>(BB))
+      if (!addToDist(Dist, OuterLoop, Node, getNode(Succ),
+                     getWeightFromBranchProb(BPI->getEdgeProbability(BB, Succ))))
         // Irreducible backedge.
         return false;
   }

llvm/include/llvm/Analysis/DominanceFrontierImpl.h

@@ -174,12 +174,10 @@ ForwardDominanceFrontierBase<BlockT>::calculate(const DomTreeT &DT,
     // Visit each block only once.
     if (visited.insert(currentBB).second) {
       // Loop over CFG successors to calculate DFlocal[currentNode]
-      for (auto SI = BlockTraits::child_begin(currentBB),
-                SE = BlockTraits::child_end(currentBB);
-           SI != SE; ++SI) {
+      for (const auto Succ : children<BlockT *>(currentBB)) {
         // Does Node immediately dominate this successor?
-        if (DT[*SI]->getIDom() != currentNode)
-          S.insert(*SI);
+        if (DT[Succ]->getIDom() != currentNode)
+          S.insert(Succ);
       }
     }
 

llvm/include/llvm/Analysis/LoopInfo.h

@@ -158,11 +158,8 @@ class LoopBase {
   /// True if terminator in the block can branch to another block that is
   /// outside of the current loop.
   bool isLoopExiting(const BlockT *BB) const {
-    typedef GraphTraits<const BlockT*> BlockTraits;
-    for (typename BlockTraits::ChildIteratorType SI =
-         BlockTraits::child_begin(BB),
-         SE = BlockTraits::child_end(BB); SI != SE; ++SI) {
-      if (!contains(*SI))
+    for (const auto Succ : children<const BlockT*>(BB)) {
+      if (!contains(Succ))
         return true;
     }
     return false;
@@ -186,11 +183,8 @@ class LoopBase {
     unsigned NumBackEdges = 0;
     BlockT *H = getHeader();
 
-    typedef GraphTraits<Inverse<BlockT*> > InvBlockTraits;
-    for (typename InvBlockTraits::ChildIteratorType I =
-         InvBlockTraits::child_begin(H),
-         E = InvBlockTraits::child_end(H); I != E; ++I)
-      if (contains(*I))
+    for (const auto Pred : children<Inverse<BlockT*> >(H))
+      if (contains(Pred))
         ++NumBackEdges;
 
     return NumBackEdges;
@@ -249,12 +243,9 @@ class LoopBase {
   /// contains a branch back to the header.
   void getLoopLatches(SmallVectorImpl<BlockT *> &LoopLatches) const {
     BlockT *H = getHeader();
-    typedef GraphTraits<Inverse<BlockT*> > InvBlockTraits;
-    for (typename InvBlockTraits::ChildIteratorType I =
-         InvBlockTraits::child_begin(H),
-         E = InvBlockTraits::child_end(H); I != E; ++I)
-      if (contains(*I))
-        LoopLatches.push_back(*I);
+    for (const auto Pred : children<Inverse<BlockT*>>(H))
+      if (contains(Pred))
+        LoopLatches.push_back(Pred);
   }
 
   //===--------------------------------------------------------------------===//

llvm/include/llvm/Analysis/LoopInfoImpl.h

@@ -34,14 +34,11 @@ namespace llvm {
 template<class BlockT, class LoopT>
 void LoopBase<BlockT, LoopT>::
 getExitingBlocks(SmallVectorImpl<BlockT *> &ExitingBlocks) const {
-  typedef GraphTraits<BlockT*> BlockTraits;
-  for (block_iterator BI = block_begin(), BE = block_end(); BI != BE; ++BI)
-    for (typename BlockTraits::ChildIteratorType I =
-           BlockTraits::child_begin(*BI), E = BlockTraits::child_end(*BI);
-         I != E; ++I)
-      if (!contains(*I)) {
+  for (const auto BB : blocks())
+    for (const auto Succ : children<BlockT*>(BB))
+      if (!contains(Succ)) {
         // Not in current loop? It must be an exit block.
-        ExitingBlocks.push_back(*BI);
+        ExitingBlocks.push_back(BB);
         break;
       }
 }
@@ -63,14 +60,11 @@ BlockT *LoopBase<BlockT, LoopT>::getExitingBlock() const {
 template<class BlockT, class LoopT>
 void LoopBase<BlockT, LoopT>::
 getExitBlocks(SmallVectorImpl<BlockT*> &ExitBlocks) const {
-  typedef GraphTraits<BlockT*> BlockTraits;
-  for (block_iterator BI = block_begin(), BE = block_end(); BI != BE; ++BI)
-    for (typename BlockTraits::ChildIteratorType I =
-           BlockTraits::child_begin(*BI), E = BlockTraits::child_end(*BI);
-         I != E; ++I)
-      if (!contains(*I))
+  for (const auto BB : blocks())
+    for (const auto Succ : children<BlockT*>(BB))
+      if (!contains(Succ))
         // Not in current loop? It must be an exit block.
-        ExitBlocks.push_back(*I);
+        ExitBlocks.push_back(Succ);
 }
 
 /// getExitBlock - If getExitBlocks would return exactly one block,
@@ -88,14 +82,11 @@ BlockT *LoopBase<BlockT, LoopT>::getExitBlock() const {
 template<class BlockT, class LoopT>
 void LoopBase<BlockT, LoopT>::
 getExitEdges(SmallVectorImpl<Edge> &ExitEdges) const {
-  typedef GraphTraits<BlockT*> BlockTraits;
-  for (block_iterator BI = block_begin(), BE = block_end(); BI != BE; ++BI)
-    for (typename BlockTraits::ChildIteratorType I =
-           BlockTraits::child_begin(*BI), E = BlockTraits::child_end(*BI);
-         I != E; ++I)
-      if (!contains(*I))
+  for (const auto BB : blocks())
+    for (const auto Succ : children<BlockT*>(BB))
+      if (!contains(Succ))
         // Not in current loop? It must be an exit block.
-        ExitEdges.push_back(Edge(*BI, *I));
+        ExitEdges.emplace_back(BB, Succ);
 }
 
 /// getLoopPreheader - If there is a preheader for this loop, return it.  A
@@ -134,15 +125,11 @@ BlockT *LoopBase<BlockT, LoopT>::getLoopPredecessor() const {
 
   // Loop over the predecessors of the header node...
   BlockT *Header = getHeader();
-  typedef GraphTraits<Inverse<BlockT*> > InvBlockTraits;
-  for (typename InvBlockTraits::ChildIteratorType PI =
-         InvBlockTraits::child_begin(Header),
-         PE = InvBlockTraits::child_end(Header); PI != PE; ++PI) {
-    typename InvBlockTraits::NodeRef N = *PI;
-    if (!contains(N)) {     // If the block is not in the loop...
-      if (Out && Out != N)
+  for (const auto Pred : children<Inverse<BlockT*>>(Header)) {
+    if (!contains(Pred)) {     // If the block is not in the loop...
+      if (Out && Out != Pred)
         return nullptr;     // Multiple predecessors outside the loop
-      Out = N;
+      Out = Pred;
     }
   }
 
@@ -156,17 +143,11 @@ BlockT *LoopBase<BlockT, LoopT>::getLoopPredecessor() const {
 template<class BlockT, class LoopT>
 BlockT *LoopBase<BlockT, LoopT>::getLoopLatch() const {
   BlockT *Header = getHeader();
-  typedef GraphTraits<Inverse<BlockT*> > InvBlockTraits;
-  typename InvBlockTraits::ChildIteratorType PI =
-    InvBlockTraits::child_begin(Header);
-  typename InvBlockTraits::ChildIteratorType PE =
-    InvBlockTraits::child_end(Header);
   BlockT *Latch = nullptr;
-  for (; PI != PE; ++PI) {
-    typename InvBlockTraits::NodeRef N = *PI;
-    if (contains(N)) {
+  for (const auto Pred : children<Inverse<BlockT*>>(Header)) {
+    if (contains(Pred)) {
       if (Latch) return nullptr;
-      Latch = N;
+      Latch = Pred;
     }
   }
 
@@ -394,11 +375,9 @@ static void discoverAndMapSubloop(LoopT *L, ArrayRef<BlockT*> Backedges,
       // within this subloop tree itself. Note that a predecessor may directly
       // reach another subloop that is not yet discovered to be a subloop of
       // this loop, which we must traverse.
-      for (typename InvBlockTraits::ChildIteratorType PI =
-             InvBlockTraits::child_begin(PredBB),
-             PE = InvBlockTraits::child_end(PredBB); PI != PE; ++PI) {
-        if (LI->getLoopFor(*PI) != Subloop)
-          ReverseCFGWorklist.push_back(*PI);
+      for (const auto Pred : children<Inverse<BlockT*>>(PredBB)) {
+        if (LI->getLoopFor(Pred) != Subloop)
+          ReverseCFGWorklist.push_back(Pred);
       }
     }
   }
@@ -482,13 +461,7 @@ analyze(const DominatorTreeBase<BlockT> &DomTree) {
     SmallVector<BlockT *, 4> Backedges;
 
     // Check each predecessor of the potential loop header.
-    typedef GraphTraits<Inverse<BlockT*> > InvBlockTraits;
-    for (typename InvBlockTraits::ChildIteratorType PI =
-           InvBlockTraits::child_begin(Header),
-           PE = InvBlockTraits::child_end(Header); PI != PE; ++PI) {
-
-      BlockT *Backedge = *PI;
-
+    for (const auto Backedge : children<Inverse<BlockT*>>(Header)) {
       // If Header dominates predBB, this is a new loop. Collect the backedges.
       if (DomTree.dominates(Header, Backedge)
           && DomTree.isReachableFromEntry(Backedge)) {

llvm/include/llvm/Support/GenericDomTree.h

@@ -276,32 +276,25 @@ 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(DominatorTreeBaseByGraphTraits<GraphT> &DT,
-             typename GraphT::NodeRef NewBB) {
+  template <class N>
+  void Split(typename GraphTraits<N>::NodeRef NewBB) {
+    using GraphT = GraphTraits<N>;
+    using NodeRef = typename GraphT::NodeRef;
     assert(std::distance(GraphT::child_begin(NewBB),
                          GraphT::child_end(NewBB)) == 1 &&
            "NewBB should have a single successor!");
-    typename GraphT::NodeRef NewBBSucc = *GraphT::child_begin(NewBB);
+    NodeRef NewBBSucc = *GraphT::child_begin(NewBB);
 
-    std::vector<typename GraphT::NodeRef> PredBlocks;
-    typedef GraphTraits<Inverse<N>> InvTraits;
-    for (typename InvTraits::ChildIteratorType
-             PI = InvTraits::child_begin(NewBB),
-             PE = InvTraits::child_end(NewBB);
-         PI != PE; ++PI)
-      PredBlocks.push_back(*PI);
+    std::vector<NodeRef> PredBlocks;
+    for (const auto Pred : children<Inverse<N>>(NewBB))
+      PredBlocks.push_back(Pred);
 
     assert(!PredBlocks.empty() && "No predblocks?");
 
     bool NewBBDominatesNewBBSucc = true;
-    for (typename InvTraits::ChildIteratorType
-             PI = InvTraits::child_begin(NewBBSucc),
-             E = InvTraits::child_end(NewBBSucc);
-         PI != E; ++PI) {
-      typename InvTraits::NodeRef ND = *PI;
-      if (ND != NewBB && !DT.dominates(NewBBSucc, ND) &&
-          DT.isReachableFromEntry(ND)) {
+    for (const auto Pred : children<Inverse<N>>(NewBBSucc)) {
+      if (Pred != NewBB && !dominates(NewBBSucc, Pred) &&
+          isReachableFromEntry(Pred)) {
         NewBBDominatesNewBBSucc = false;
         break;
       }
@@ -312,7 +305,7 @@ template <class NodeT> class DominatorTreeBase : public DominatorBase<NodeT> {
     NodeT *NewBBIDom = nullptr;
     unsigned i = 0;
     for (i = 0; i < PredBlocks.size(); ++i)
-      if (DT.isReachableFromEntry(PredBlocks[i])) {
+      if (isReachableFromEntry(PredBlocks[i])) {
         NewBBIDom = PredBlocks[i];
         break;
       }
@@ -324,18 +317,18 @@ template <class NodeT> class DominatorTreeBase : public DominatorBase<NodeT> {
       return;
 
     for (i = i + 1; i < PredBlocks.size(); ++i) {
-      if (DT.isReachableFromEntry(PredBlocks[i]))
-        NewBBIDom = DT.findNearestCommonDominator(NewBBIDom, PredBlocks[i]);
+      if (isReachableFromEntry(PredBlocks[i]))
+        NewBBIDom = findNearestCommonDominator(NewBBIDom, PredBlocks[i]);
     }
 
     // Create the new dominator tree node... and set the idom of NewBB.
-    DomTreeNodeBase<NodeT> *NewBBNode = DT.addNewBlock(NewBB, NewBBIDom);
+    DomTreeNodeBase<NodeT> *NewBBNode = addNewBlock(NewBB, NewBBIDom);
 
     // If NewBB strictly dominates other blocks, then it is now the immediate
     // dominator of NewBBSucc.  Update the dominator tree as appropriate.
     if (NewBBDominatesNewBBSucc) {
-      DomTreeNodeBase<NodeT> *NewBBSuccNode = DT.getNode(NewBBSucc);
-      DT.changeImmediateDominator(NewBBSuccNode, NewBBNode);
+      DomTreeNodeBase<NodeT> *NewBBSuccNode = getNode(NewBBSucc);
+      changeImmediateDominator(NewBBSuccNode, NewBBNode);
     }
   }
 
@@ -379,7 +372,7 @@ template <class NodeT> class DominatorTreeBase : public DominatorBase<NodeT> {
     if (DomTreeNodes.size() != OtherDomTreeNodes.size())
       return true;
 
-    for (const auto &DomTreeNode : this->DomTreeNodes) {
+    for (const auto &DomTreeNode : DomTreeNodes) {
       NodeT *BB = DomTreeNode.first;
       typename DomTreeNodeMapType::const_iterator OI =
           OtherDomTreeNodes.find(BB);
@@ -663,10 +656,9 @@ template <class NodeT> class DominatorTreeBase : public DominatorBase<NodeT> {
   /// tree to reflect this change.
   void splitBlock(NodeT *NewBB) {
     if (this->IsPostDominators)
-      this->Split<Inverse<NodeT *>, GraphTraits<Inverse<NodeT *>>>(*this,
-                                                                   NewBB);
+      Split<Inverse<NodeT *>>(NewBB);
     else
-      this->Split<NodeT *, GraphTraits<NodeT *>>(*this, NewBB);
+      Split<NodeT *>(NewBB);
   }
 
   /// print - Convert to human readable form
@@ -677,7 +669,7 @@ template <class NodeT> class DominatorTreeBase : public DominatorBase<NodeT> {
       o << "Inorder PostDominator Tree: ";
     else
       o << "Inorder Dominator Tree: ";
-    if (!this->DFSInfoValid)
+    if (!DFSInfoValid)
       o << "DFSNumbers invalid: " << SlowQueries << " slow queries.";
     o << "\n";
 
@@ -712,12 +704,12 @@ template <class NodeT> class DominatorTreeBase : public DominatorBase<NodeT> {
     // immediate dominator.
     NodeT *IDom = getIDom(BB);
 
-    assert(IDom || this->DomTreeNodes[nullptr]);
+    assert(IDom || DomTreeNodes[nullptr]);
     DomTreeNodeBase<NodeT> *IDomNode = getNodeForBlock(IDom);
 
     // Add a new tree node for this NodeT, and link it as a child of
     // IDomNode
-    return (this->DomTreeNodes[BB] = IDomNode->addChild(
+    return (DomTreeNodes[BB] = IDomNode->addChild(
                 llvm::make_unique<DomTreeNodeBase<NodeT>>(BB, IDomNode))).get();
   }
 
@@ -780,7 +772,7 @@ template <class NodeT> class DominatorTreeBase : public DominatorBase<NodeT> {
   template <class FT> void recalculate(FT &F) {
     typedef GraphTraits<FT *> TraitsTy;
     reset();
-    this->Vertex.push_back(nullptr);
+    Vertex.push_back(nullptr);
 
     if (!this->IsPostDominators) {
       // Initialize root

llvm/include/llvm/Support/GraphWriter.h

@@ -143,10 +143,9 @@ class GraphWriter {
 
   void writeNodes() {
     // Loop over the graph, printing it out...
-    for (node_iterator I = GTraits::nodes_begin(G), E = GTraits::nodes_end(G);
-         I != E; ++I)
-      if (!isNodeHidden(*I))
-        writeNode(*I);
+    for (const auto Node : nodes<GraphType>(G))
+      if (!isNodeHidden(Node))
+        writeNode(Node);
   }
 
   bool isNodeHidden(NodeRef Node) {