[PATCH] D94834: [Coroutine] Do not CoroElide if there are musttail calls

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added a comment.

In D94834#2520137 <https://reviews.llvm.org/D94834#2520137>, @lxfind wrote:

> In D94834#2519348 <https://reviews.llvm.org/D94834#2519348>, @ChuanqiXu wrote:
>
>> In D94834#2504992 <https://reviews.llvm.org/D94834#2504992>, @lxfind wrote:
>>
>>> In D94834#2504128 <https://reviews.llvm.org/D94834#2504128>, @ChuanqiXu wrote:
>>>
>>>> I am a little unclear about this problem. From my point of view, it seems like that there is a Coroutine C elided in a normal function F. Then in the Coroutine Body of C, it would try to switch to itself by symmetric transfer. However, the Coroutine frame of C now is a stack variable. Then the tail call would pass the address of the stack variable whose lifetime has ended, so here is the corruption. Did I understand the situation?
>>>
>>> Yes I believe that's an accurate description
>>
>> I'm looking into this bug now. Now it is more confusing to me since it seems like that this process shouldn't happen.
>>
>> The IR generated for the attachment of bug48626 is like:
>>
>>   %19 = tail call noalias nonnull i8* @llvm.coro.begin(token %15, i8* %18), !noalias !1082 ; The handle we are going to elide
>>   // ... in another BB
>>   %35 = call i8* @llvm.coro.subfn.addr(i8* nonnull %19, i8 1) #28 ; destroy %19
>>   // ... in another BB
>>   musttail call fastcc void %51(i8* %retval.sroa.0.0.copyload.i.i.i120) ; dominated by `call i8* @llvm.coro.subfn.addr(i8* nonnull %19, i8 1)`
>>   ret void
>>
>> It seems like that the argument of the musttail call can't alias %19 in any sense since %19 already been destroyed before the musttail call. Then it makes sense to all situations I can image. Since we only elide the coro handles who wouldn't escape. And we did the escape analysis very conservatively (Not by AA analysis, but by a simple path based escape analysis). So I think it may be better to remove the check in the end of `ShouldElide` function. It seems like that the memory wouldn't crash due to the argument of the musttail call alias with the coroutine handle we elided. And if any corruption happens, I think is is a bug for the escape analysis.
>
> It won't crash in this particular case. But as discussed in the bug thread, in theory the symmetric transfer could return the same handle as passed in, and in that case you won't be able to do tail call, right?

Yes and my thoughts about this case is that CoroElide pass won't elide the frame. As a general example:

  define void @xxx.resume(%FrameTy* %FramePtr) {
  entry:
       ; ...
  BB:
       %handle = tail call noalias nonnull i8* @llvm.coro.begin(...)
       ; ...
  BB1:
       call i8* @llvm.subfn.addr(%handle, 1)
       ; ...
  BB.final:
        musttail call fastcc void @resume.call(i8* %val) 
        ret void
  }

And I think there are two cases about this example:
(1) The musttail call is dominated by  `@llvm.subfn.addr(%handle, 1)` calls. Then `%handle` is destroyed at the musttail call, so it makes no sense that the musttail call returns `%handle`.
(2) The musttail call isn't dominated by  `@llvm.subfn.addr(%handle, 1)` calls. Then the musttail call may returns `%handle`. And it doesn't matter since `%handle` won't be elided in this case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94834