[PATCH] D66230: [coroutine] Fixes "cannot move instruction since its users are not dominated by CoroBegin" problem.

[John McCall via Phabricator via llvm-commits]

rjmccall added a comment.

In D66230#1632621 <https://reviews.llvm.org/D66230#1632621>, @GorNishanov wrote:

> In D66230#1632510 <https://reviews.llvm.org/D66230#1632510>, @rjmccall wrote:
>
> > Is there a case you're imagining where the adjustment would have side-effects?  Because I can see a reason to have an intrinsic that returns a frame pointer, but I don't see why that intrinsic would have any of the restrictions of `@llvm.coro.begin`.
>
>
> Which restrictions are you thinking about?


"A frontend should emit exactly one coro.begin intrinsic per coroutine."

> Marking it as NoDuplicate in CoroEarly helps simplify the CoroSplit logic.

Does it?  Why?  There already has to be a single `coro.id` in the coroutine, and that's the intrinsic that takes useful information.  `coro.begin` just has a bunch of requirements and no clear purpose except to return the frame, which could just as well be done with a duplicable intrinsic.  Frame allocation has to happen in the ramp prologue anyway, and the underlying observation of this particular patch is that trying to move `coro.begin` to reflect its logical order in the function is just a lot of complication for no clear purpose.  And honestly the frame pointer is not usually a useful thing to expose in a coroutine representation; aspects of frame layout being exposed in the ABI is a property of the switch lowering, not coroutines in general.

John.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66230