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

[Gor Nishanov via Phabricator via llvm-commits]

GorNishanov added a comment.

In D66230#1632625 <https://reviews.llvm.org/D66230#1632625>, @rjmccall wrote:

> aspects of frame layout being exposed in the ABI is a property of the switch lowering, not coroutines in general.


You may be right. Given that now we have two frontends targeting LLVM Coroutines, some refactoring may be in order. I need to study more the Swift approach before I can form an opinion.

>> 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.

Here is how it looks to me: C++ frontends emits the following structure (simplified):

  %id = coro.id(stuff)
  %mem = SomeAllocCodeCouldBeAnything(stuff)
  %frame = coro.begin(%mem)

coro.begin "blesses" the memory as the one to be used in the coroutine and therefore should dominate any possible uses of data that go into the coroutine frame and gives a convenient place to dump spills, copies, etc.

If we did not allow arbitrary allocation logic in c++, there would be no need for coro.begin at all.

Gor


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66230