[PATCH] D95807: [Coroutines] Add the newly generated SCCs back to the CGSCC work queue after CoroSplit actually happened

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added inline comments.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroSplit.cpp:2115
+                      << "\n");
     F.removeFnAttr(CORO_PRESPLIT_ATTR);
 
----------------
drive-by nit: would it be better to move it up near `Coroutines.push_back(&N);`?


================
Comment at: llvm/lib/Transforms/Coroutines/CoroSplit.cpp:2001
 
-    if ((Shape.ABI == coro::ABI::Async || Shape.ABI == coro::ABI::Retcon ||
-         Shape.ABI == coro::ABI::RetconOnce) &&
-        !Shape.CoroSuspends.empty()) {
-      // Run the CGSCC pipeline on the newly split functions.
-      // All clones will be in the same RefSCC, so choose a random clone.
-      UR.RCWorklist.insert(CG.lookupRefSCC(CG.get(*Clones[0])));
+    if (!Shape.CoroSuspends.empty()) {
+      // Run the CGSCC pipeline on the original and newly split functions.
----------------
aeubanks wrote:
> ChuanqiXu wrote:
> > I am not familiar with the Shape.ABI other than coro::ABI:switch. But the diff line seems strange, it looks like that condition gets weaker.
> I believe that's intentional, and a big part of this patch. We want to re-add the current SCC (and the split SCCs) any time we split an SCC. Before we weren't properly doing that.
I got your point. So "// All clones will be in the same RefSCC ...." : this is not accurate I think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95807