[PATCH] D98638: [RFC][Coroutine] Force stack allocation after await_suspend() call

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added a comment.

In D98638#2630864 <https://reviews.llvm.org/D98638#2630864>, @lxfind wrote:

> That's a fair point. I agree that we have no guarantee these are the only two cases.
> It does seem to me that coroutine implementation somewhat relies on proper lifetime markers so that data are being put correctly, which may be the fundamental problem we are trying to solve.

It is hard to prove it. This topic need more discuss and more folks get involved. But it is really valuable. I can't remember how many patches we had to judge whether values should be put on the coroutine frame. I am OK to emit lifetime markers even at O0. But I think you need to ask for other's opinion.

In D98638#2630864 <https://reviews.llvm.org/D98638#2630864>, @lxfind wrote:

> We probably could, but it would be very very tedious. 
> During CodeGen, we only have the AST that's calling __builtin_coro_resume, which we will call Emit as a whole.
> So we need to manually match the AST 2 levels down to find the await_suspend call, get its name, and then walk through the emitted IR to find a call with the same name, and then find the tmp that's used to store the return value of the call, and then emit llvm.coro.forcestack.

Can't we did as inline comments?



================
Comment at: clang/lib/CodeGen/CGCoroutine.cpp:221
     CGF.EmitBlock(RealSuspendBlock);
+  } else if (ForcestackStart) {
+    Builder.CreateCall(
----------------
can we rewrite it into:
```
else if (SuspendRet != nullptr && SuspendRet->getType()->isClassType()) {
     // generate:
     // llvm.coro.forcestack(SuspendRet)
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98638