[PATCH] D22261: [InlineCost] Set minsize inline threshold to 0

[Chandler Carruth via llvm-commits]

chandlerc added a comment.

In https://reviews.llvm.org/D22261#499087, @jmolloy wrote:

> Ideally, we'd teach the inliner to much more accurately determine the overall expansion of a tree of thunks and tiny template expansions. However simply giving a bonus bump to linkonce_odr functions in the same way as internal functions appears to do the trick quite well.


I agree with Mehdi that this seems arbitrary, but I'll go farther -- I really think that focusing on linkonce_odr functions from the cost analysis side is the wrong approach.

I think instead you'll need to look at the nature of the (admitedly linkonce_odr) functions which get inlined at a threshold of 25 but not at a threshold of 0, and try to understand what pattern we're missing that causes us to misestimate the size. There is probably a reasonably small number of patterns that are missing at the low end of the threshold (because we've not stared at it as much) that will be generally beneficial to recognize and accurately model.

But what is more concerning is that the "bonus" you're using is the "call once" bonus. That isn't really a bonus. What it does is it pretty much completely removes the inlining threshold. As a consequence, with this change, you can pretty cause *massive* code size explosions by just a huge linkonce_odr function that is called once in every translation unit, but by a different function in every translation unit. You'll get O(number of translation units) copies of that function. =[

I really think we need to understand why you see this swing in size between two very close thresholds of 0 and 25. My suspicion is that we have some flaw in how we calculate the cost of inlining a function which just calls another function with (perhaps a permutation of) the arguments it is called with. Fixing that cost computation so that it (correctly) is modeled as 0 cost seems really valuable *outside* of -Oz as well.

-Chandler


https://reviews.llvm.org/D22261