[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of explicitly emitting retainRV or claimRV calls in the IR

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

This patch causes opt to crash when the following IR is compiled:

$ cat test.ll

  @g0 = global i8 zeroinitializer, align 1
  
  define internal i8* @foo() {
    ret i8* @g0
  }
  
  define void @test() {
    %r = call i8* @foo() [ "clang.arc.rv"(i64 1) ]
    call void (...) @llvm.objc.clang.arc.noop.use(i8* %r)
    ret void
  }
  
  declare void @llvm.objc.clang.arc.noop.use(...)
  
  !llvm.module.flags = !{!0}
  
  !0 = !{i32 1, !"clang.arc.retainAutoreleasedReturnValueMarker", !"mov\09fp, fp\09\09// marker for objc_retainAutoreleaseReturnValue"}

$ opt -ipsccp -deadargelim -objc-arc -o - -S test.ll

What's happening is that ipsccp replaces argument `%r` passed to call `@llvm.objc.clang.arc.noop.use` with `@g0` and then deadargelim changes the return type of `@foo` to `void` since there are no explicit users of its return. ARC optimizer crashes because it expects a call with the operand bundle to have a return.

To fix the crash, we can modify `tryToReplaceWithConstant` to prevent ipsccp from replacing `@llvm.objc.clang.arc.noop.use`'s argument. Alternatively, we can restore the check in `DeadArgumentEliminationPass::SurveyFunction`, which was in earlier versions of the patch (see https://reviews.llvm.org/D92808?id=319082#inline-892965). I feel like the latter option is better since we wouldn't' have to prevent ipsccp from optimizing the IR, which is perfectly legal. If it's not desirable to add a check that is too specific to ObjC, we could add a new generic function to the core library, which indicates the function's return type shouldn't be changed, and use it to tell deadargelim not to change the return type. Or we could use a bundle that is more generic than `clang.arc.rv` (for example, `"implicitly.used.by"(@llvm.objc.retainAutoreleasedReturnValue)`, which indicates the result is used by an instruction that isn't explicitly emitted in the IR).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92808