[PATCH] D104007: [BasicAA] Properly mark that coroutine suspension may modify parameters

[Xun Li via Phabricator via llvm-commits]

lxfind added a comment.

In D104007#2812401 <https://reviews.llvm.org/D104007#2812401>, @efriedma wrote:

> In D104007#2812395 <https://reviews.llvm.org/D104007#2812395>, @lxfind wrote:
>
>> In D104007#2812383 <https://reviews.llvm.org/D104007#2812383>, @efriedma wrote:
>>
>>> This doesn't seem right.  A byval argument is essentially a local variable.  We should treat it the same way as we would an alloca.  If it's live across the first suspend point, it should become part of the coroutine frame.
>>
>> It will become part of the coroutine frame, by copying it through memcpy. That's actually what's the first memcpy for in the test case.
>
> I don't understand.  As far as I can tell, the destination of both memcpys in the testcase are allocas.

OK, so the way coroutine codegen works is to first copy all parameters to local alloca, and then if any of those allocas need to live through coroutine suspension, the alloca will be put on the coroutine frame during the CoroSplit pass.

>> But the parameter pointer itself will still die during a coroutine suspension. Because of that I would say the parameter pointer is different from an alloca pointer, because during a coroutine suspension, allocas won't change.
>
> I think we're talking past each other.
>
> During coroutine suspension, allocas won't change because corosplit rewrites them.  I'm saying that the same rewrite should apply to byval arguments.  A byval argument is basically the same thing as an alloca: it's memory owned by the callee that's deallocated when the function returns.

Just to be clear, MemCpyOptPass happens before CoroSplit pass. So at the point of MemCpyOptPass, all it sees are two memcpys, one that copies from a byval readonly parameter to a local alloca, and another one from the local alloca to another. The only way to prevent these two memcpys to merge is to tell AA that coro_suspend intrinsics call could potentially modify the parameter pointer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104007