[PATCH] D75815: [InstSimplify] Simplify calls with "returned" attribute

[Nikita Popov via Phabricator via llvm-commits]

nikic reopened this revision.
nikic added a comment.
This revision is now accepted and ready to land.

Sorry for the delay here, and thanks for taking care of the revert and providing a test case!

Here's an IR test case under `opt -inline -instcombine`:

  declare dso_local i64 @g()
  
  ; Function Attrs: norecurse nounwind readnone
  define dso_local i64 @i(i64 returned %h) #0 {
  entry:
    ret i64 %h
  }
  
  define dso_local i32 @j(i32 ()* nocapture %h) {
  entry:
    %call = call i32 %h()
    ret i32 undef
  }
  
  define dso_local i32 @l() {
  entry:
    %call = call i32 @j(i32 ()* @k)
    ret i32 undef
  }
  
  define internal i32 @k() {
  entry:
    %call = call i64 @g()
    %call1 = call i64 @i(i64 returned %call)
    ret i32 undef
  }
  
  attributes #0 = { norecurse nounwind readnone }

I believe the problematic code is in `UpdateCallGraphAfterInlining` <https://github.com/llvm/llvm-project/blob/acaf1442226ac1871a6df35fea032a2aef32567a/llvm/lib/Transforms/Utils/InlineFunction.cpp#L1179>, which assumes that the instruction an inlined call maps to still refers to the same callee. After this change, this is no longer the case.

I don't see a great way to fix the call graph updating code. I think it might make more sense to move this simplification from InstSimplify into InstCombine to avoid the problem. This would also reduce the compile-time impact <http://llvm-compile-time-tracker.com/compare.php?from=623cb95eb337618406f1095b9c67a4aae8aae81c&to=0928368f623a0f885894f9c3ef1b740b060c0d9c&stat=instructions> of this change, which I didn't really expect.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75815