[PATCH] D50704: [Inline-cost] Teach cost function to account for accumulative code-size growth

[Easwaran Raman via Phabricator via llvm-commits]

eraman added inline comments.


================
Comment at: lib/Analysis/InlineCost.cpp:1913
+
+  int Growth = (NumInstructions - NumInstructionsSimplified) * F.getNumUses();
+
----------------
dnsampaio wrote:
> greened wrote:
> > This assumes all uses are inlined.  That seems overly pessimistic.  I'm not sure it's wise to predict what the inliner will do in the cost model.  It seems like an incremental cost would be better.  The inliner would then do the cost analysis for each inlining opportunity and decide on a case-by-case basis whether to inline or not.  As written, this seems to have the cost model driving inline decisions (inline everything or nothing) rather than the inliner using cost information to make decisions.
> The cost-function analysis using -Oz are not that complex. It only considers each of the instructions (if they can be simplified if inlined), arguments passed and a constant cost for the call site. And see that it is incremental already. The additional cost decreases each time the function is inlined, that is, N reduces. The first one has the higher penalty, the last ones the higher benefit. Not counting the last one that gains huge bonus already.
> 
The cost of a callee depends on the callsite so multiplying the cost for one callsite by number of uses is not very meaningful. 


================
Comment at: lib/Analysis/InlineCost.cpp:286
+                                       Params.ComputeFullInlineCost || ORE ||
+                                       F.optForMinSize()),
         IsCallerRecursive(false), IsRecursiveCall(false),
----------------
Turning on ComputeFullInlineCost can lead to excessive compile time for pathological cases.  It changes the cost to analyze a function F from O(1) [the constant here depends on inline threshold] to O(size of F) and will particularly be bad for Oz because the threshold is low to start with.


https://reviews.llvm.org/D50704