[PATCH] D122383: [Coroutines] [Retcon] Replace CoroBegin with FramePtr after

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added a comment.

In D122383#3413345 <https://reviews.llvm.org/D122383#3413345>, @rjmccall wrote:

> Hmm.  Doing this after doing the clones means you're only actually doing the replacement in the ramp function, which I think means it'll explode in the clones if there are uses remaining.  On some level, that might be okay, because I don't think the Swift frontend ever actually uses the result of coro.begin in retcon functions except to pass it to coro.end, and retcon lowering of coro.end just ignores the argument.  On the other hand, we shouldn't leave something around that'll blow up if the code pattern ever changes.
>
> I think you can just recognize this case (when Shape.FramePtr is an argument) and remove the mapping for that argument from VMap before cloning.

I found another idea. Here I found the second argument of coro.begin is meant to point to the coroutine frame. (https://llvm.org/docs/Coroutines.html#llvm-coro-begin-intrinsic) I found it says that it is ignored by `returned-continuation coroutines`. But I think it is for the frontend users. We don't invalidate it if we set it in the middle end. And if we could do it, then the codes in CoroCleanup would replace coro.begin with the second argument. Do you think this is available? If yes, I think it might be better than handle the argument case specially.



================
Comment at: llvm/lib/Transforms/Coroutines/CoroSplit.cpp:1777
   }
 
   // Create a unique return block.
----------------
How about setting the memory argument for CoroBegin here:
```
Shape.CoroBegin->setOperand(1, RawFramePtr);
```

Then the CoroBegin would be replaced to `RawFramePtr` later in https://github.com/llvm/llvm-project/blob/8568d681ae14fb8c7769c80e0196a16934647413/llvm/lib/Transforms/Coroutines/CoroCleanup.cpp#L67-L69


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

https://reviews.llvm.org/D122383