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

[Xun Li via Phabricator via llvm-commits]

lxfind added a comment.

> Here what I want to say is we **shouldn't**  handle all the symmetric transfer from the above analysis. And we shouldn't change the ASTNodes and Sema part. We need to solve about the above pattern. It is not easy to give a solution since user could implement symmetric transfer in final awaiter without destroying the handle, which is more common.

Just to clarify, in case there are any confusions around this. This patch would work no matter whether the coroutine frame is destroyed or not during await_suspend(). It simply makes sure that the temporary handle returned by await_suspend will be put in the stack instead of heap, and it will always be safe to do so, no matter what happens.
Whether or not the current coroutine frame would be destroyed completely depend on the implementation of await_suspend. So we cannot predict or know in advance. Therefore, the temporary handle returned by await_suspend must be put on the stack. I don't really see any other solutions other than this.

> It seems to be a workaround to use @llvm.coro.forcestack(%result_of_final_await_suspend) . Since I wondering if there are other corner cases as the %gro. My opinion about '@llvm.coro.forcestack' is that we could use it as a patch if we find any holes that is hard to handle immediately. But we also need to find a solution to solve problems more fundamentally.

Yes as I mentioned in the description, there are really only two cases, one is after await_suspend call, and one is gro. gro is easy to handle and I will likely send a separate patch latter. But this problem with await_suspend is particularly challenging to solve.

What do you think is the fundamental problem, though?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98638