[PATCH] D24219: [LCG] Redesign the lazy post-order iteration mechanism for the LazyCallGraph to support repeated, stable iterations, even in the face of graph updates.

[Chandler Carruth via llvm-commits]

chandlerc marked an inline comment as done.

================
Comment at: lib/Analysis/LazyCallGraph.cpp:888
@@ -901,3 +887,3 @@
   int SCCIndex = 0;
-  for (RefSCC *C : reverse(Connected)) {
-    assert(C != this &&
+  for (RefSCC *RC : reverse(MergeRange)) {
+    assert(RC != this &&
----------------
eraman wrote:
> I don't think you should be reversing the MergeRange here. Consider this example where you have three RefSCCs R0, R1 and R2 with the numbers being index to the PostOrderRefSCCs. Assume R2 is this RefSCC and the edge being added is from R0 -> R2. Assume each Ri has one Si (call SCC). MergeRange is [R0, R1].  At the end of this loop, MergedSCCs will have [S1, S0] and outside the loop S2 gets added to MergedSCCs resulting in [S1, S0, S2]. I believe you want it to be [S0, S1, S2] to keep it in post order. 
> 
> The unit test below doesn't test that call SCC's are in post-order. Ensuring verify on the merged RefSCC doesn't assert helps in general (but still won't catch this)
> 
Arrg, still more things that need a more complex test case to trigger.

I've added a couple more tests that exercise this and some other aspects of the code. They cause the verify below to catch this bug, as well as independently verifying the postorder result.

Thanks again!


https://reviews.llvm.org/D24219