ychen 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.
----------------
lxfind wrote:
> 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.
I missed the ABI condition. Thanks for the explanation.
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
