ChuanqiXu added a comment. It looks like there are two things this patch wants to do:
1. Don't put the temporary generated by symmetric-transfer on the coroutine frame. 2. Offer a mechanism to force some values (it is easy to extend Alloca to Value) to put in the stack instead of the coroutine frame. I am a little confused about the first problem. Would it cause the program to crash? (e.g., we access the fields of coroutine frame after the frame gets destroyed). Or it just wastes some storage? And I want to ask about the change of the AST nodes and SemaCoroutine. Can we know if a CoroutineSuspendExpr stands for a symmetric-transfer? If yes, it seems we can only do changes in CodeGen part. Then I agree to introduce new intrinsic to hint the middle end to put some values on the stack. And the design of `@llvm.coro.forcestack.begin()` and `@llvm.coro.forcestack.end()` is a little strange to me. It says they mark a region where only data from the local stack can be accessed. But it looks error-prone since it is hard for the front-end to decide whether all the access of the region should be put on the stack. I think we could introduce only one intrinisic `@llvm.coro.forcestack(Value* v)`, we can use the argument to mark the value need to be put on the stack. And about the problem you mentioned in D96922 <https://reviews.llvm.org/D96922>: "The lifetime of %coro.gro" starts early and %coro.gro" would be used after `coro.end` (Possibly the destructor?) which would cause the program to access destroyed coroutine frame". It looks like the mechanism could solve this problem by a call to `@llvm.coro.forcestack(%coro.gro)`. ================ Comment at: clang/include/clang/AST/ExprCXX.h:4695 +/// afterwards on the stack. class CoroutineSuspendExpr : public Expr { friend class ASTStmtReader; ---------------- It looks strange for the change of `CoroutineSuspendExpr` at the first glance. It is easy to understand the coroutine suspend expression is consists of three parts: Ready, Suspend and resume. It is written in the language documentation. And the new added AwaitSuspendCall is confusing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98638/new/ https://reviews.llvm.org/D98638 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
