[llvm] [Coroutines][LazyCallGraph] resumes are not really SCC (PR #116285)

[Arthur Eubanks via llvm-commits]

================
@@ -1082,11 +1082,14 @@ class LazyCallGraph {
   /// Add new ref-recursive functions split/outlined from an existing function.
   ///
   /// The new functions may only reference other functions that the original
-  /// function did. The new functions may reference (not call) the original
-  /// function.
+  /// function did. The new functions may reference the original function. New
+  /// functions must not call other new functions or the original function.
   ///
-  /// The original function must reference (not call) all new functions.
-  /// All new functions must reference (not call) each other.
+  /// Marks the original function as referencing all new functions.
+  ///
+  /// The CG must be updated following the use of this helper, for example with
+  /// updateCGAndAnalysisManagerForCGSCCPass(), to ensure the RefSCCs and SCCs
+  /// are correctly identified.
----------------
aeubanks wrote:

> In our case the original and new functions are requeued by addSplitRefRecursiveFunctions, so no need to call updateCGAndAnalysisManagerForCGSCCPass().

`addSplitRefRecursiveFunctions()` doesn't requeue SCCs to be revisited (since it's just a LazyCallGraph method that has no idea of SCC pass manager scheduling), either `updateCGAndAnalysisManagerFor*Pass()` does [here](llvm/lib/Analysis/CGSCCPassManager.cpp), or you can do it manually like CoroSplit does [here](https://github.com/llvm/llvm-project/blob/c9ba2b08d57fb682d4fe9ab2dd6f14d464b081df/llvm/lib/Transforms/Coroutines/CoroSplit.cpp#L2277).

> Just to clarify: after we call addSplitRefRecursiveFunctions we immediately call updateCGAndAnalysisManagerForCGSCCPass, followed by simplifying the original function, then a call to updateCGAndAnalysisManagerForFunctionPass. You can see this in CoroSplit.cpp::updateCallGraphAfterCoroutineSplit(...). When we simplify the original function some of the references to the new functions may be removed. I am starting to suspect the call to updateCGAndAnalysisManagerForCGSCCPass is not required, does that match your interpretation as well?

yes, the call to `updateCGAndAnalysisManagerForCGSCCPass()` shouldn't be necessary since you've already updated LazyCallGraph, but you would still need to manually requeue the new SCCs to be visited, which CoroSplit already does as mentioned above.

https://github.com/llvm/llvm-project/pull/116285