[PATCH] D145516: [Inliner] Avoid excessive inlining through devirtualised calls

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a comment.

Splitting out the inline comments,

> but negative costs are very common for inlining because it's cheaper to not setup a call, so I don't think this change makes sense

Hrrmmm -- removing the floor on `Cost` causes the too-much-inlining behaviour in the added test. It looks like, at the point where the function-inline-cost-multiplier is added,

- With the floor added, `Cost` is 5, and is eventually multiplied to be larger than the threshold,
- Without the floor `Cost` is -50, the multiplier can't stop it being negative, so the inlining always succeeds.

This strays even further outside of my comfort zone, but it seems that:

- If the cost is permitted to be negative at any point then there'll inevitably be an input where the cost-multiplier isn't able to suppress inlining,
- The cost needs to be transiently negative sometimes to correctly sum the costs and benefits of different parts of a function.

Perhaps the solution is a fail-safe upper bound on the function-inline-cost-multiplier past which inlining is disallowed, i.e. "we've inlined through this SCC ten times now in this context, quit inlining it"?


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

https://reviews.llvm.org/D145516