[PATCH] D96928: [LICM][Coroutine] Don't sink stores from loops with coro.suspend instructions

[JunMa via Phabricator via llvm-commits]

junparser added a comment.

In D96928#2576209 <https://reviews.llvm.org/D96928#2576209>, @lxfind wrote:

> In D96928#2576084 <https://reviews.llvm.org/D96928#2576084>, @junparser wrote:
>
>> In D96928#2571879 <https://reviews.llvm.org/D96928#2571879>, @lxfind wrote:
>>
>>> In D96928#2570981 <https://reviews.llvm.org/D96928#2570981>, @junparser wrote:
>>>
>>>> please see comments of D87817 <https://reviews.llvm.org/D87817>.
>>>
>>> I don't think we should go that route, because LICM will mostly hurt coroutine, as I explained in the summary.
>>> Hence we should simply disable LICM for coroutine, and I don't think this is a temporary change. What do you think?
>>
>> I do not think we should disable LICM for coroutine, also this is not semantic restriction of coroutine (GCC does not do this). It just caused by current pipeline of llvm coroutine as well as debug info issues.  I was thinking maybe we can invoke corosplit as early as possible (not considering performance). Anyway , we can discuss this in D95807 <https://reviews.llvm.org/D95807>.
>
> Could you explain why we want to enable LICM for coroutine (from performance perspective)?
> My theory is this:
>
> - majority of LICM should be memory operations. Constant function calls that are not inlined should be relatively rare compared to memory operations.
> - For memory operations, LICM does not reduce the number of memory operations in the loop in the case of coroutine; instead it adds one extra entry to the coroutine frame, increasing memory usage.

LICM move the memory operations out of the loop. It does reduce the number of memory operations. More importantly, I agree with @efriedma that either we have general solution or we describe these restrictions other than fix them anywhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96928