lxfind added inline comments.

================
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.
----------------
ychen wrote:
> 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?
Note that previously this is done only for Async, Retcon and RetconOnce ABIs, 
not for the Switch ABI.
I guess that's accurate for those ABIs? But for Switch ABI this is not true.
And before we were not adding back the split functions to the pipeline to be 
properly optimized. Now we are dong that. This should help improve the 
performance of the post-split functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95807

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to