================
@@ -401,36 +432,85 @@ static ReadySuspendResumeResult buildCoawaitCalls(Sema
&S, VarDecl *CoroPromise,
return Calls;
}
Expr *CoroHandle = CoroHandleRes.get();
+ Calls.UseAwaitSuspendDestroy = false;
CallExpr *AwaitSuspend = cast_or_null<CallExpr>(
BuildSubExpr(ACT::ACT_Suspend, "await_suspend", CoroHandle));
if (!AwaitSuspend)
return Calls;
+
+ // When this `await_suspend()` overload is annotated with
+ // `[[clang::coro_await_suspend_destroy]]`, do NOT call `await_suspend()` --
+ // instead call `await_suspend_destroy(Promise&)`. This assumes that the
+ // `await_suspend()` is just a compatibility stub consisting of:
+ // await_suspend_destroy(handle.promise());
+ // handle.destroy();
+ // Users of the attribute must follow this contract. Then, diagnostics from
+ // both `await_suspend` and `await_suspend_destroy` will get exposed.
+ CallExpr *PlainAwaitSuspend = nullptr;
+ if (FunctionDecl *AwaitSuspendCallee = AwaitSuspend->getDirectCallee()) {
+ if (AwaitSuspendCallee->hasAttr<CoroAwaitSuspendDestroyAttr>()) {
+ Calls.UseAwaitSuspendDestroy = true;
+ ExprResult PromiseRefRes =
+ buildPromiseRef(S, CoroPromise->getType(), Loc);
+ if (PromiseRefRes.isInvalid()) {
+ Calls.IsInvalid = true;
+ return Calls;
+ }
+ Expr *PromiseRef = PromiseRefRes.get();
+ PlainAwaitSuspend = AwaitSuspend;
+ AwaitSuspend = cast_or_null<CallExpr>(
+ BuildSubExpr(ACT::ACT_Suspend, "await_suspend_destroy", PromiseRef));
+ if (!AwaitSuspend)
+ return Calls;
+ }
+ }
----------------
snarkmaster wrote:
> emit await_suspend directly, instead of via `coro.await_suspend` intrinsics
I agree it's a nice, clean idea. This is sort of what I tried in the very first
version of this (https://github.com/llvm/llvm-project/pull/152029), which I
quickly put into "draft" mode because I realized my implementation was wrong.
Here's my old thinking, which led to wrong / broken code:
- `await_suspend()` must contain an `h.destroy()` call since we want
portability to compilers without the attribute.
- I wanted to make the `await_suspend` wrapper omit `coro.suspend` -- my goal
here is to make it look like a non-suspending coroutine so it optimizes like a
plain function.
- The `h.destroy()` ultimately invokes a `coro.destroy` intrinsic. The
implementation of that intrinsic does an indirect call through a `destroy`
pointer that is written to the coro frame via one of the intrinsics that I
elided. That frame setup would also write `resume` pointer which is required by
`h.done()`.
- So, now the `h.destroy()` call will fail with a null pointer dereference.
I tried a few permutations, but at the time I didn't understand the code well
enough to try the right one, perhaps.
Are you saying that specifically **the following** version will both maintain
the internal invariants, and optimize well?
- Keep `coro.save` and `coro.suspend`
- Replace `coro.awaits.suspend.XXX` by a direct `await_suspend`-wrapper call
I am happy to try this out, though I'll be pleasantly surprised all the
indirection optimizes away and it emits good code.
@ChuanqiXu9, can you confirm I have the correct idea now? Are there any gotchas?
https://github.com/llvm/llvm-project/pull/152623
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
