[PATCH] D122375: [CoroSplit] Handle argument being the frame pointer

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added a reviewer: rjmccall.
ChuanqiXu added a subscriber: rjmccall.
ChuanqiXu added a comment.

In D122375#3404813 <https://reviews.llvm.org/D122375#3404813>, @nikic wrote:

> In D122375#3404809 <https://reviews.llvm.org/D122375#3404809>, @ChuanqiXu wrote:
>
>> I am not familiar with retcon coroutines and opaque pointers. But I am surprised for the sentence `If the frame pointer is an argument of the original pointer (which happens with opaque pointers)`. May I ask where we would make it possible to make the FramePtr to be the argument of  the original function with opaque pointers?
>
> Without opaque pointers, the frame pointer is a bitcast of the argument. With opaque pointers, the bitcast is not present, and it's directly the argument. So without opaque pointers, the old frame pointer first becomes `bitcast undef` and then gets replaced with the new frame pointer later. With opaque pointers it becomes undef, which cannot be correctly replaced. This patch tries to artificially add a dummy bitcast to make the replacement work correctly.

Oh I guess I understand the problem. The design of retcon ABI allows the user to specify the address of the coroutine frame. So it is possible that the frame is the argument of the original function indeed. Then I found true problem here is that the comment in https://github.com/llvm/llvm-project/blob/be5c3ca7fbaec90fff004af54d3cd5f6c30a9664/llvm/lib/Transforms/Coroutines/CoroSplit.cpp#L874-L876 is invalided. The arguments here shouldn't contain users.

So I believe the problem might be in `splitRetconCoroutine`. It replaces CoroBegin before cloning. (We treat the value returned by CoroBegin as the frame generally). But it replaces CoroBegin with the argument this time so that the argument still owns the user. So here is the problem. My solution is to replace CoroBegin later after the cloning. I sent https://reviews.llvm.org/D122383. And I add @rjmccall . He is the author of the retcon lowering. Maybe he could give further helps.


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

https://reviews.llvm.org/D122375