[PATCH] D76248: Fix a bug in the inliner that causes subsequent double inlining

[Hongtao Yu via Phabricator via llvm-commits]

hoyFB added a comment.

In D76248#1934941 <https://reviews.llvm.org/D76248#1934941>, @davidxl wrote:

> ok -- I misunderstood (it to be a runtime problem) -- it is actually a stale reference related issue. In the failing case,  is the inlining order the following?
>
> top->second 
>  top->first (original site)
>  top->first (from the clone of 'second')?
>
> Even without ICE, this is still wrong unless first has no side effect.


Sorry, the function names in my test case are confusing. I should have named them more clearly. The actual inline order, in short, is
top->second
top->third
top->run (from //third//)
top->gen (from //third//)
top->gen (exactly the same callsite above, from the simplification of //wrapper// that comes with //run//. This is double inlining! The fix is to keep the //wrapper// call instead of simplifying to //gen//).

The order seems fine to me, except that //wrapper// is not inlined into //run// before //run// is cloned into //top//. This causes the callsite of //wrapper// in //top// to be considered as a further inline candidate. Normally a bottom-up inlining order ensures //run// is fully processed before its caller processed. However, with indirect calls the call graph is not precisely built, thus the bottom-up order is not fully fulfilled. We carefully elaborated the test case to expose the issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76248