[PATCH] D100282: [Coroutines] Move CoroEarly pass to before AlwaysInliner

2021-04-12 Thread Xun Li via Phabricator via cfe-commits
lxfind updated this revision to Diff 336850. lxfind added a comment. Set the attributes in Clang instead of CoroEarly Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100282/new/ https://reviews.llvm.org/D100282 Files: clang/lib/CodeGen/CGCoroutine

[PATCH] D100282: [Coroutines] Move CoroEarly pass to before AlwaysInliner

2021-04-11 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. We don't have so many coroutine-emitting frontends that it's unreasonable to modify them all. Swift can make this change; it's not on you to do it. (I also work on the Swift frontend, so don't worry, I'm signing my own teammates up for work here.) Repository: rG

[PATCH] D100282: [Coroutines] Move CoroEarly pass to before AlwaysInliner

2021-04-11 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D100282#2682297 , @lxfind wrote: >> Ah, if the pass does more than just setting the attribute, then sure, it >> makes sense to keep it. But I do think we should be requiring the attribute >> to be added by frontends, since

[PATCH] D100282: [Coroutines] Move CoroEarly pass to before AlwaysInliner

2021-04-11 Thread Xun Li via Phabricator via cfe-commits
lxfind added a comment. > Ah, if the pass does more than just setting the attribute, then sure, it > makes sense to keep it. But I do think we should be requiring the attribute > to be added by frontends, since it's really an IR invariant that it's present > on all unlowered coroutines. By th

[PATCH] D100282: [Coroutines] Move CoroEarly pass to before AlwaysInliner

2021-04-11 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. This patch looks OK to me. > The best we can do is lower the coroutine and then inline the ramp function, > which is not really the same thing. So warning about marking a coroutine > always_inline does make some sense. It makes sense to emit a warning instead of an e

[PATCH] D100282: [Coroutines] Move CoroEarly pass to before AlwaysInliner

2021-04-11 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In D100282#2682269 , @lxfind wrote: > In D100282#2682251 , @rjmccall > wrote: > >> Why does this pass even exist? We should just expect the frontend to set >> the attribute. It's not

[PATCH] D100282: [Coroutines] Move CoroEarly pass to before AlwaysInliner

2021-04-11 Thread Xun Li via Phabricator via cfe-commits
lxfind added a comment. In D100282#2682251 , @rjmccall wrote: > Why does this pass even exist? We should just expect the frontend to set the > attribute. It's not like frontends don't have to otherwise know that they're > emitting a coroutine; a ton o

[PATCH] D100282: [Coroutines] Move CoroEarly pass to before AlwaysInliner

2021-04-11 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Why does this pass even exist? We should just expect the frontend to set the attribute. It's not like frontends don't have to otherwise know that they're emitting a coroutine; a ton of things about the expected entire IR pattern are different. Repository: rG LLVM

[PATCH] D100282: [Coroutines] Move CoroEarly pass to before AlwaysInliner

2021-04-11 Thread Xun Li via Phabricator via cfe-commits
lxfind created this revision. lxfind added reviewers: junparser, dongAxis1944, rjmccall, ChuanqiXu. Herald added subscribers: hoy, modimo, wenlei, hiraditya. lxfind requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. Presplit