[Coroutines] Support `@llvm.coro.await_suspend` actually

[ChuanqiXu9]

After c467245, we will add a noinline attribute to the await_suspend member function of an awaiter if the awaiter has any non static member functions.

Obviously, this decision will bring some performance regressions. e.g., #64933

The suggested long term solution is to introduce the new intrinisic:

call @llvm.coro.await_suspend(ptr %awaiter, ptr %handle,
                                                      ptr @awaitSuspendFn)

Then in the middle end, we can perform analysis the awaitSuspendFn to see if the coroutine handle escapes or not. If it won't escape, we can replace above intrinsic to the direct call. Otherwise, we will only convert them in the CoroSplit phase.

The key point of the solution is that it is much easier and better to do analysis in the middle end instead of the frontend.

[llvmbot]

@llvm/issue-subscribers-coroutines

[kelbon]

Sorry, maybe i dont fully understand problem described in #56301 , but it seems common problem is that all inlined functions locals for some reason becames part of coroutine frame, while they are not.

If it is problem may we somehow mark coroutine-used locals/args/unnamed temp values(including awaiters) at frontend and then on middle end they will use coroutine frame as storage and others just use stack?

[ChuanqiXu9]

Sorry, maybe i dont fully understand problem described in #56301 , but it seems common problem is that all inlined functions locals for some reason becames part of coroutine frame, while they are not.

If it is problem may we somehow mark coroutine-used locals/args/unnamed temp values(including awaiters) at frontend and then on middle end they will use coroutine frame as storage and others just use stack?

It may not be easy to explain in several words. But generally it is bad/annoying/inefficient to do analysis jobs in the frontend.

And I know (and all the reviewers and commentors know) the method will bring some bad codes. But the correctness is better than the performance. BTW, although we had a long term solution plan to fix the issue, I would expect that there are still some performance regressions if your await_suspend is super complex.

[ChuanqiXu9]

Oh, wait. Maybe it is a idea to not do anything if the await_suspend function is marked always_suspend by the user explicitly. I'll do this.

[kelbon]

Oh, wait. Maybe it is a idea to not do anything if the await_suspend function is marked always_suspend by the user explicitly. I'll do this.

I was talking about this:
#56301 (comment)

For example we have code:

my_coroutine c() {
  foo(...);
  co_await <expr>;
  bar(args...);
}

Here compiler must be sure, that no matter what 'foo' and 'bar' do and return, coroutine frame must never store anything which is not used in <expr>

But as i understand after inlining 'foo' and 'bar' clang do not sees difference between local coroutine variable and something created as local in inlined function. I saw this on clang 14/15 when repeating function call was increasing coroutine frame without real reasion

co_await <expr> turns to

• create awaiter (stored in frame)
• just call functions (await_ready, await_suspend, await_resume ...), all what they will produce must not be stored in coroutine frame

And problem with #56301 coro handle returned from 'Register' and stored as local variable in 'final_suspend' stored on coroutine frame.

If clang 'splits' coroutine only on clang-IR stage, may be introduce tags like 'call in the coroutine' which after inline will be like:

inlined_call_start:
/* something */
inlined_call_end:

[ChuanqiXu9]

@kelbon BTW, I don't know why I can't receive the mail from your comment. And your comment about inlining tags is relatively implemented as lifetime intrinsics in LLVM IR... But things in the middle end transformation are pretty complex to handle and explain... the variables in the frontend may get moved and merged or optimized away. And the many optimization are not teached about the idea of coroutines well. Then this is the root problem and we can't a lot things for that at once.