[PATCH] D129230: [IR][coroutine] make final and non-final llvm.coro.saves unique

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added a comment.

> D129025 <https://reviews.llvm.org/D129025> also excludes some valid

transformation: for this test case, if both branches of the if statement
contains non-final suspend, it is ok to hoisting llvm.coro.save.

It looks not valid to me to hoist non-final suspend llvm.coro.save. From https://github.com/llvm/llvm-project/blob/011d2bf86487520c3515f16e0b1d32994bf2b48f/llvm/lib/Transforms/Coroutines/CoroSplit.cpp#L388-L405 we could know a `llvm.coro.save` would be lowered into:

  %index.addr = getelementptr %f.frame... (index field number)
  store i32 %IndexVal, i32* %index.addr1

So the following code:

  if.then:
    %0 = call token @llvm.coro.save(ptr null)
    %1 = call i8 @llvm.coro.suspend(token %0, i1 false)
    br label %merge
  
  if.else:
    %2 = call token @llvm.coro.save(ptr null)
    %3 = call i8 @llvm.coro.suspend(token %2, i1 false)
    br label %merge

would be lowered into:

  if.then:
    %0 = getelementptr %f.frame... (index field number)
    store i32 0, i32* %0 ; assume this is the first suspend points
    %1 = call i8 @llvm.coro.suspend(token %0, i1 false)
    br label %merge
  
  if.else:
    %2 = getelementptr %f.frame... (index field number)
    store i32 1, i32* %2
    %3 = call i8 @llvm.coro.suspend(token %2, i1 false)
    br label %merge

so it clearly makes no sense to merge the non-final coro saves. D129025 <https://reviews.llvm.org/D129025> looks good enough to me temporarily.

> however, the only other choice would be to change llvm.coro.save in some way like:
>
> 1.add new flag parameter to llvm.coro.save
> 2.add llvm.coro.save.final
>
> I'm not a fan of both.

So option2 is not valid. But I am a fan of option 1. Since it looks naturally to merge 2 `@llvm.coro.save(ptr null)` but it wouldn't be naturally to merge `@llvm.coro.save(ptr null, i32 0)` and `@llvm.coro.save(ptr null, i32 1)`. So we solve the problem in the IR level.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129230