[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
cfe-commits at lists.llvm.org
Wed Jun 30 10:10:43 PDT 2021
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
More information about the cfe-commits
mailing list