[PATCH] D52915: [PM] keeping history when original SCC split and then merge into itself in the same round of SCC update .

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

Ok, also had some time to really think about the core problem and approach. Write-up below.

Sorry for the spam of different responses as I looked at this from different angles.



================
Comment at: test/Transforms/Inline/cgscc-cycle-2.ll:24-26
+; edge, and the two SCCs will be merged into the original one again. During
+; this cycle, the original SCC will be added into UR.CWorklist again and
+; this creates an infinite loop.
----------------
FWIW, this description and ascii art are *awesome*. Thanks for that.

The only high level question I have here is: *should* we have put the SCC onto the worklist?

One fix (which you have here) is to detect when we will revisit the SCC and retain information to break any cycles in inlining.

Another fix in theory is to avoid putting the SCC onto the queue.

What I haven't yet figured out (but you may have already thought about) is if the split+re-merge operations happen in a context where we might expect interesting SCC-structure changes to have occurred and thus it to be profitable to re-enqueue the SCC on the worklist. If that would normally happen, then ythe fix you have here makes perfect sense. But if this is (for example) just an artifact of the order in which we apply updates, and the SCC is never actually going to be different, maybe we should find a way to avoid enqueuing it? It might be too hard to do that though and we'd still end up where your current patch is...


Repository:
  rL LLVM

https://reviews.llvm.org/D52915