[llvm-dev] Fwd: [RFC] LLVM Coroutines

[Gor Nishanov via llvm-dev]

Hi Eli:

Thank you very much for your comments!

>> If you need some sort of unusual control flow construct, make it a
>> proper terminator instruction

I would love to. I was going by the advice in "docs/ExtendingLLVM.rst":

       "WARNING: Adding instructions changes the bitcode format, and it will
        take some effort to maintain compatibility with the previous version.
        Only add an instruction if it is absolutely necessary.

Having coro.fork and coro.suspend to be proper terminators (as they are) will
be much cleaner. Something like:

  corobegin to label %start
         suspend label %retblock

   corosuspend [final] [save %token]
           resume label %resume
            cleanup label %cleanup

I did consider "repurposing" 'invoke', but felt it is too much of an abuse.
If there is a support for making corobegin/corosuspend instructions, I would
love to do it. Also, if there is a support for invoke abuse :-), I will be
glad to try it out and come back with the report on how it went.

>> Maybe you could reduce the amount of magic involved by splitting the
>> initialization into a separate function?  So the initialization is always
>> just something like "f(int x) { frame = init(); f_inner(x, frame); return
>> frame; }", and you don't have to deal with the weirdness of fork().

I think outlining the "start" part will be sufficient (starting at the coro.fork
and cutting all of the branches ending in coro.end). In this case llvm won't see
coro.fork and coro.end as the clang will do the outlining work. Something like

 T f(int x, A y) {
   auto mem = coro.elide();
   if (!mem) mem = malloc(llvm.frame.size(&f_inner));
  auto hdl = coro.init(mem);
  T result = <whatever>;

  f_inner(hdl, x, &y);

  return result;
 }

f will be a normal function and f_inner will be magic function that will require
to be split at suspend points. It will be processed in the IPO order, so by the
time f_inner is considered for inlining into f, it will be a normal function.

I think this can work, but we are trading off one complexity for another.

As presented:

 coroutine is defined as a magic function (that has weird intrinsics) and
 requires splitting during IPO

With coro.fork / coro.end removed

 coroutine is defined as a combination of normal function + magic function that
 requires splitting during IPO. If there are other languages that acquire
 coroutines they would need to have the same outlining pass as clang.

I need to think about it more, but, I am currently leaning toward the first
option (with coro.fork and coro.end). The "f_inner" is still part of the f,
but nicely delineated with corobegin and coro.end.

  corobegin to label %coro.start
         suspend label %retblock

   corosuspend [final] [save %token] resume label %resume
            cleanup label %cleanup

    call void @llvm.coro.end();

Does it look better?

Gor

On Thu, Jun 9, 2016 at 1:33 AM, Eli Friedman <eli.friedman at gmail.com> wrote:
> On Wed, Jun 8, 2016 at 10:57 PM, Gor Nishanov via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>>
>> Hi all:
>>
>> Below is a proposal to add experimental coroutine support to LLVM. Though
>> this
>> proposal is motivated primarily by the desire to support C++ Coroutines
>> [1],
>> the llvm representation is language neutral and can be used to support
>> coroutines in other languages as well.
>
>
> I looked over the proposal... a few comments:
>
> 1. This adds a lot of intrinsics, and they have weird semantics.  This is
> bad in general; the more weirdness you add to the IR, the more you'll
> struggle to make the optimizer actually understand what you're doing.  And
> magic pattern matching *will* break.  You've already pointed out the issue
> with branches.  (It's actually worse than you think: there isn't any
> guarantee there will even be a branch when you've thrown the optimizer at
> it.)  The save intrinsic says "Its return value should be consumed by
> exactly one `coro.suspend` intrinsic.", but it might have zero uses by the
> time your lowering pass runs.  You could end up with code getting inserted
> before llvm.experimental.coro.fork.
> 2. If you need some sort of unusual control flow construct, make it a proper
> terminator instruction; don't try to glue your intrinsic to a normal branch
> instruction.  A new kind of terminator instruction might be appropriate.  Or
> maybe you can make "invoke" work for you.
> 3. Maybe you could reduce the amount of magic involved by splitting the
> initialization into a separate function?  So the initialization is always
> just something like "f(int x) { frame = init(); f_inner(x, frame); return
> frame; }", and you don't have to deal with the weirdness of fork().
>
> -Eli