[PATCH] D72226: Short-circuit SCC update for self-referential edge

[Brian Gesiak via Phabricator via llvm-commits]

modocache marked an inline comment as done.
modocache added inline comments.


================
Comment at: llvm/unittests/Analysis/CGSCCPassManagerTest.cpp:1593
+          // a node for 'g' into the call graph. This also connects creates a
+          // ref edge 'f -> g'.
+          auto *G =
----------------
I think I see the issue. 

First of all, this comment is incorrect: `CallGraphUpdater::registerOutlinedFunction` doesn't automatically add any edges between `f` and `g`, that's up to the user to do themself -- it doesn't create a reference edge, a call edge, it doesn't create anything.

Here I'm inserting `g` into the same SCC as `f`, but for that to be a valid SCC to insert into, `f` and `g` need to be part of the same strongly-connected component, meaning a call edge path exists connecting `f *-> g` and `g *-> f`. (For typical "outlining" of a part of the body of `f` into some outlined function, a call edge path from `f *-> g` would almost certainly exist.)

However, in the case of coroutine funclets, there is only a ref edge `f -> g`. And in this example, there's not even a ref edge `f` -> g`.

So this is user error: I shouldn't be inserting `f` into `g`, I ought to instead create an SCC especially for `g`. If I do so, then later when the call graph is updated to handle the demotion of `f -> f` from a call edge to a ref edge, the DFS walk won't find a new SCC `(g)`, and `incorporateNewSCCRange` won't be called with ranges `[(g), (f)]` for `f`.

That's my theory, anyway. If I end up being right, I'll probably end up abandoning this change and changing D71899 slightly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72226/new/

https://reviews.llvm.org/D72226