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

[Xun Li via Phabricator via llvm-commits]

lxfind added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96928