[PATCH] D29381: [PM/LCG] Remove the lazy RefSCC formation from the LazyCallGraph during iteration.

Davide Italiano via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Feb 5 11:29:47 PST 2017

davide added a comment.

I personally like this solution as it strips away a lot of complexity. Some comments while I do some testing on this patch.

Comment at: include/llvm/Analysis/LazyCallGraph.h:1086-1090
+  template <typename RootsT, typename GetBeginT, typename GetEndT,
+            typename GetNodeT, typename FormSCCCallbackT>
+  static void buildGenericSCCs(RootsT &&Roots, GetBeginT &&GetBegin,
+                               GetEndT &&GetEnd, GetNodeT &&GetNode,
+                               FormSCCCallbackT &&FormSCC);
I think this API deserves a comment, also for consistency with everything else in the file.

Comment at: lib/Analysis/LazyCallGraph.cpp:1692-1693
-          assert(!lookupSCC(ChildN) &&
-                 "Found a node with 0 DFS number but already in an SCC!");
+          //assert(!lookupSCC(ChildN) &&
+          //       "Found a node with 0 DFS number but already in an SCC!");
           ChildN.DFSNumber = ChildN.LowLink = NextDFSNumber++;
Commented code?

Comment at: lib/Analysis/LazyCallGraph.cpp:1788
+  if (EntryEdges.empty() || !PostOrderRefSCCs.empty())
+    // RefSCCs are either non-existent or already bulit!
+    return;

Comment at: lib/Analysis/LazyCallGraph.cpp:1797-1798
+  std::reverse(Roots.begin(), Roots.end());
+  buildGenericSCCs(
Comment on why you need this reverse, maybe?

Comment at: lib/Analysis/LazyCallGraph.cpp:1810-1811
+        // Push the new node into the postorder list.
+        bool Inserted =
+            RefSCCIndices.insert({NewRC, PostOrderRefSCCs.size()}).second;
"Push the new node into the postorder list " ... 
I would also add the fact you're inserting in the indices map (or move this comment down, up to you).


More information about the llvm-commits mailing list