[PATCH] Minor improvements to scc_iterator

[Mehdi Amini]

  Update, thanks again Duncan for the careful review!

http://llvm-reviews.chandlerc.com/D2672

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D2672?vs=6843&id=6905#toc

Files:
  include/llvm/ADT/SCCIterator.h

Index: include/llvm/ADT/SCCIterator.h
===================================================================
--- include/llvm/ADT/SCCIterator.h
+++ include/llvm/ADT/SCCIterator.h
@@ -47,6 +47,22 @@
   typedef typename super::reference reference;
   typedef typename super::pointer pointer;
 
+  // Element of VisitStack during DFS
+  struct StackElement {
+    NodeType *Node;       ///< The current node pointer.
+    ChildItTy NextChild;  ///< The next child, modified inplace during DFS.
+    unsigned MinVisited;  ///< Minimum uplink value of all children of Node.
+
+    StackElement(NodeType *Node, const ChildItTy &Child, unsigned Min)
+      : Node(Node), NextChild(Child), MinVisited(Min) {}
+
+    bool operator==(const StackElement &Other) const {
+      return Node == Other.Node &&
+             NextChild == Other.NextChild &&
+             MinVisited == Other.MinVisited;
+    }
+  };
+
   // The visit counters used to detect when a complete SCC is on the stack.
   // visitNum is the global counter.
   // nodeVisitNumbers are per-node visit numbers, also used as DFS flags.
@@ -59,22 +75,17 @@
   // The current SCC, retrieved using operator*().
   SccTy CurrentSCC;
 
-  // Used to maintain the ordering.  The top is the current block, the first
-  // element is basic block pointer, second is the 'next child' to visit.
-  std::vector<std::pair<NodeType *, ChildItTy> > VisitStack;
 
-  // Stack holding the "min" values for each node in the DFS. This is used to
-  // track the minimum uplink values for all children of the corresponding node
-  // on the VisitStack.
-  std::vector<unsigned> MinVisitNumStack;
+  // DFS stack, Used to maintain the ordering.  The top contains the current
+  // node, the next child to visit, and the minimum uplink value of all child
+  std::vector<StackElement> VisitStack;
 
   // A single "visit" within the non-recursive DFS traversal.
   void DFSVisitOne(NodeType *N) {
     ++visitNum;
     nodeVisitNumbers[N] = visitNum;
     SCCNodeStack.push_back(N);
-    MinVisitNumStack.push_back(visitNum);
-    VisitStack.push_back(std::make_pair(N, GT::child_begin(N)));
+    VisitStack.push_back(StackElement(N, GT::child_begin(N), visitNum));
 #if 0 // Enable if needed when debugging.
     dbgs() << "TarjanSCC: Node " << N <<
           " : visitNum = " << visitNum << "\n";
@@ -84,35 +95,39 @@
   // The stack-based DFS traversal; defined below.
   void DFSVisitChildren() {
     assert(!VisitStack.empty());
-    while (VisitStack.back().second != GT::child_end(VisitStack.back().first)) {
+    while (VisitStack.back().NextChild !=
+           GT::child_end(VisitStack.back().Node)) {
       // TOS has at least one more child so continue DFS
-      NodeType *childN = *VisitStack.back().second++;
-      if (!nodeVisitNumbers.count(childN)) {
+      NodeType *childN = *VisitStack.back().NextChild++;
+      typename DenseMap<NodeType *, unsigned>::iterator Visited =
+        nodeVisitNumbers.find(childN);
+      if (Visited == nodeVisitNumbers.end()) {
         // this node has never been seen.
         DFSVisitOne(childN);
         continue;
       }
 
-      unsigned childNum = nodeVisitNumbers[childN];
-      if (MinVisitNumStack.back() > childNum)
-        MinVisitNumStack.back() = childNum;
+      unsigned childNum = Visited->second;
+      if (VisitStack.back().MinVisited > childNum)
+        VisitStack.back().MinVisited = childNum;
     }
   }
 
   // Compute the next SCC using the DFS traversal.
   void GetNextSCC() {
-    assert(VisitStack.size() == MinVisitNumStack.size());
     CurrentSCC.clear(); // Prepare to compute the next SCC
     while (!VisitStack.empty()) {
       DFSVisitChildren();
-      assert(VisitStack.back().second ==
-             GT::child_end(VisitStack.back().first));
-      NodeType *visitingN = VisitStack.back().first;
-      unsigned minVisitNum = MinVisitNumStack.back();
+      // Back from DFSVisitChildren with a leaf on top of the VisitStack, pop it
+      NodeType *visitingN = VisitStack.back().Node;
+      unsigned minVisitNum = VisitStack.back().MinVisited;
+      // DFS Recursion stop when all child are visited
+      assert(VisitStack.back().NextChild == GT::child_end(visitingN));
       VisitStack.pop_back();
-      MinVisitNumStack.pop_back();
-      if (!MinVisitNumStack.empty() && MinVisitNumStack.back() > minVisitNum)
-        MinVisitNumStack.back() = minVisitNum;
+
+      // Propagate MinVisitNum to parent so we can detect the SCC starting node.
+      if (!VisitStack.empty() && VisitStack.back().MinVisited > minVisitNum)
+        VisitStack.back().MinVisited = minVisitNum;
 
 #if 0 // Enable if needed when debugging.
       dbgs() << "TarjanSCC: Popped node " << visitingN <<
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2672.4.patch
Type: text/x-patch
Size: 4720 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140205/a2fae0ac/attachment.bin>