[PATCH] D87817: [LICM][Coroutine] Don't sink stores to coroutine suspend point.

Xun Li via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 24 22:02:35 PDT 2020


lxfind added a comment.

In D87817#2294122 <https://reviews.llvm.org/D87817#2294122>, @junparser wrote:

> In D87817#2291684 <https://reviews.llvm.org/D87817#2291684>, @lxfind wrote:
>
>> In D87817#2289365 <https://reviews.llvm.org/D87817#2289365>, @junparser wrote:
>>
>>> In D87817#2278816 <https://reviews.llvm.org/D87817#2278816>, @junparser wrote:
>>>
>>>> In D87817#2278717 <https://reviews.llvm.org/D87817#2278717>, @junparser wrote:
>>>>
>>>>> We try to find an general solution to handle some conservative  & mismatch analysis in transformations caused by the change of cfg with coro.suspend intrinsic.  However afaik,  it seems quiet difficult to deal this without change framework fo coroutine. 
>>>>> This patch just fixes the known issue, it may happens in other passes, FYI.
>>>>
>>>> one of the idea is that we do not generate default case for switch instruction of coro.suspend intrinsic. Instead, we find fall through coro.end for coro.suspend directly in corosplit pass. The only uncertain thing is whether  fall through coro.end is moved by passes which may change the semantic.
>>>
>>> @lxfind, what do you think about this idea?
>>
>> Are you suggesting to not have an edge from the coro.suspend switch to the coro.end block? Would that make the coro.end BB a dead BB and could potentially get removed?
>
> No, There is always path from entry to coro.end BB.
>
>> This is likely the right solution, but we do need to make it more general. In order for this to always work, I wonder if the proper place to insert this transformation is in LoopSimplify, which is a pass always executed before any Loop pass. That might guarantee that any loop pass will be able to handle this properly. I am not too familiar with loop passes, but maybe someone else could comment on that.
>
> Loop exit-block insertion in LoopSimplify pass guarantees that all exit blocks from the loop only have predecessors from inside of the loop. It will break the dedicated form of exit blocks when we consider ignore coro.suspend path. Maybe there is other idea i haven't thought about.

I wasn't referring to ignoring the coro.suspend path, but to move this code in this patch into the LoopSimplify pass, so that all loop passes can benefit, not sure if that makes sense. Anyway, I don't have a better idea.. So don't let that block you here.


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

https://reviews.llvm.org/D87817



More information about the llvm-commits mailing list