[PATCH] Minor improvements to scc_iterator
Mehdi Amini
mehdi.amini at silkan.com
Sat Feb 1 15:56:10 PST 2014
We previously kept two stacks during DFS, with an invariant that s1.size() == s2.size(). Indeed what is really needed is to keep tuples of three elements on a stack, so it is!
Since the guidelines for C++11 are not yet published I assumed that std::tuple is OK. If it is not it can be replaced by a struct{} with operator == defined.
Note also that I replaced a sequence:
if(hash.count(key)) hash[key]=val;
with:
iterator = hash.find(key)
if(iterator != hash.end()) hash[key] = val;
It should save a hash lookup.
make check is ok on my laptop, with and without assertion enabled.
Expected Passes : 9617
Expected Failures : 57
Unsupported Tests : 34
(that's my first patch submission to LLVM)
http://llvm-reviews.chandlerc.com/D2672
Files:
include/llvm/ADT/SCCIterator.h
Index: include/llvm/ADT/SCCIterator.h
===================================================================
--- include/llvm/ADT/SCCIterator.h
+++ include/llvm/ADT/SCCIterator.h
@@ -25,6 +25,7 @@
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/GraphTraits.h"
+#include <tuple>
#include <vector>
namespace llvm {
@@ -46,6 +47,19 @@
std::vector<typename GT::NodeType>, ptrdiff_t> super;
typedef typename super::reference reference;
typedef typename super::pointer pointer;
+ // Type for elements kept on VisitStack during DFS
+ // The first element is the current node pointer
+ // The second is the 'next child' to visit, it is modified inplace during DFS
+ // The last element is the "minValue", i.e. it identify the node below on the
+ // stack that delimit the current SCC.
+ typedef std::tuple<NodeType *, ChildItTy, unsigned> StackElement;
+
+ // Indexing in the StackElement tuple
+ enum {
+ NODE = 0,
+ CURRENT_CHILD = 1,
+ MINVAL = 2
+ };
// The visit counters used to detect when a complete SCC is on the stack.
// visitNum is the global counter.
@@ -59,22 +73,15 @@
// 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;
+ // Used to maintain the ordering. The top is the current node
+ 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(std::make_tuple(N, GT::child_begin(N), visitNum));
#if 0 // Enable if needed when debugging.
dbgs() << "TarjanSCC: Node " << N <<
" : visitNum = " << visitNum << "\n";
@@ -84,35 +91,43 @@
// The stack-based DFS traversal; defined below.
void DFSVisitChildren() {
assert(!VisitStack.empty());
- while (VisitStack.back().second != GT::child_end(VisitStack.back().first)) {
+ while (std::get<CURRENT_CHILD>(VisitStack.back())
+ != GT::child_end(std::get<NODE>(VisitStack.back()))) {
// TOS has at least one more child so continue DFS
- NodeType *childN = *VisitStack.back().second++;
- if (!nodeVisitNumbers.count(childN)) {
+ NodeType *childN = *std::get<CURRENT_CHILD>(VisitStack.back())++;
+ auto 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 (std::get<MINVAL>(VisitStack.back()) > childNum)
+ std::get<MINVAL>(VisitStack.back()) = 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 = std::get<NODE>(VisitStack.back());
+ unsigned minVisitNum = std::get<MINVAL>(VisitStack.back());
+ // DFS Recursion stop when all child are visited
+ assert(std::get<CURRENT_CHILD>(VisitStack.back()) ==
+ GT::child_end(visitingN));
VisitStack.pop_back();
- MinVisitNumStack.pop_back();
- if (!MinVisitNumStack.empty() && MinVisitNumStack.back() > minVisitNum)
- MinVisitNumStack.back() = minVisitNum;
+
+ // If we found a SCC (loop in the graph), then minVisitNum for the leaf
+ // may be lower than its predecessor in the DFS path, propagate it back
+ // this is required because we detect the starting node of the SCC when
+ // minVisitNum is unchanged.
+ if (!VisitStack.empty() &&
+ std::get<MINVAL>(VisitStack.back()) > minVisitNum)
+ std::get<MINVAL>(VisitStack.back()) = minVisitNum;
#if 0 // Enable if needed when debugging.
dbgs() << "TarjanSCC: Popped node " << visitingN <<
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2672.1.patch
Type: text/x-patch
Size: 4926 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140201/d6d55e3c/attachment.bin>
More information about the llvm-commits
mailing list