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

[Yuanfang Chen via Phabricator via llvm-commits]

ychen added a comment.

> 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.

Agreed.

>> 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.



> it looks naturally to merge 2 `@llvm.coro.save(ptr null)`

Hmmm, I don't quite understand this. Do you please give an example?

One more complication is that for

  define void @f(i32 %x) {
  entry:
    %cmp = icmp slt i32 %x, 0
    br i1 %cmp, label %await.suspend, label %final.suspend
  
  await.suspend:
    %0 = call token @llvm.coro.save(ptr null)
    %1 = call i8 @llvm.coro.suspend(token %0, i1 false)
    br label %coro.ret
  
  final.suspend:
    %2 = call token @llvm.coro.save(ptr null)
    %3 = call i8 @llvm.coro.suspend(token %2, i1 true)
    br label %coro.ret
  
  coro.ret:
    ret void
  }

this is not correct (SimplifyCFG only hoists `llvm.coro.save` too):

  define void @f(i32 %x) {
  entry:
    %cmp = icmp slt i32 %x, 0
    %0 = call token @llvm.coro.save(ptr null)
    br i1 %cmp, label %await.suspend, label %final.suspend
  
  await.suspend:
    %1 = call i8 @llvm.coro.suspend(token %0, i1 false)
    br label %coro.ret
  
  final.suspend:
    %3 = call i8 @llvm.coro.suspend(token %0, i1 false)
    br label %coro.ret
  
  coro.ret:
    ret void
  }

but this is correct (SimplifyCFG hoists `llvm.coro.suspend` too):

  define void @f(i32 %x) {
  entry:
    %cmp = icmp slt i32 %x, 0
    %0 = call token @llvm.coro.save(ptr null)
    %1 = call i8 @llvm.coro.suspend(token %0, i1 false)
    ret void
  }

I think what we need is that: only allow merging `llvm.coro.save` when its user `llvm.coro.suspend` is also merged. But I don't know a good way to express that with IR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129230