[PATCH] D24338: [InlineCost] Remove CallPenalty and change MinSizeThreshold to 5

[Mehdi Amini via llvm-commits]

> On Nov 1, 2016, at 10:01 PM, Xinliang David Li <davidxl at google.com> wrote:
> 
> 
> 
> On Tue, Nov 1, 2016 at 8:06 PM, Mehdi AMINI <mehdi.amini at apple.com <mailto:mehdi.amini at apple.com>> wrote:
> mehdi_amini added a comment.
> 
> (Add inline the answer to David, so that there is the relevant context)
> 
> 
> 
> ================
> Comment at: lib/Analysis/InlineCost.cpp:938
> -      if (!isa<InlineAsm>(CS.getCalledValue()))
> -        Cost += InlineConstants::CallPenalty;
>      }
> ----------------
> >>! In D24338, @davidxl wrote:
> > Do you mean parameter passing? That should be counted independently  by the instruction visitor.
> 
> No, I don’t mean the parameters, the visitor account for one instruction per parameter just before this indeed.
> However a call has other extra-cost on top of a regular instruction, even without any argument: it splits live-ranges and may cause spills around it to preserve registers that aren't callee-saved.
> 
> In this sense, it make sense that a call can "cost" more than a normal instruction.
> 
> yes, inlining can lead to increased register pressure leading to use of caller save registers (and spills around new callsites). However I think this should not be blindly applied -- better tied with register pressure analysis.

I’m not sure we’re talking on the same level here: I don’t claim the the register pressure will increase in the caller after inlining here, or at least not because of call instructions in the callee.
My assumption (possibly incorrect), is that the current inliner cost model trades off the cost of the call site with the increased code size resulting from the inlining.

When we consider the overall “cost” of a callee: we a cost per-instruction in the visitor. The question is what is this cost representing? From a code size point of view, it make sense to add a penalty for call instructions because they have potentially more impacts (spills is an example that is specific to calls).

I don’t believe the patch as-is demonstrate that removing this penalty is the right thing to do: I’d like to see first how it compares to:
1) simply starting by deducing the cost of one call (the call-site considered for inlining) ; it is included in this patch, but should be split and committed first: that’s mostly a “bug fix”.
2) simply increasing the threshold.

Without these additional baseline, it is not clear to me that the measurement is sound: we can very well obtain better result because we will now inline a function that has multiple calls, while there can exist similar function without many calls but a bit more arithmetic that would also be profitable to inline.

— 
Mehdi

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161101/07c7d5fc/attachment.html>