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

[David Li via llvm-commits]

davidxl added a comment.

The change makes sense overall.     Easwaran mentioned this exact issue a short while ago.



================
Comment at: include/llvm/Analysis/InlineCost.h:44
 const int IndirectCallThreshold = 100;
-const int CallPenalty = 25;
 const int LastCallToStaticBonus = 15000;
----------------
We should not remove this parameter. Instead it should be used in the opposite way.   Currently the CallAnalyzer::analyzeCall only considers cost of parameter passing (subtracted from the overall cost), it in fact should subtract CallPenalty : Cost -= CallPenalty; to model cost of function call (call/ret, pro/epi logue).  Note that the cost here means runtime cost. The current inline cost is highly tuned toward size cost.


================
Comment at: lib/Analysis/InlineCost.cpp:1254
+  // The call instruction itself will also disappear after inlining.
+  Cost -= InlineConstants::InstrCost;
+  
----------------
>From the point of view of modelling runtime cost, this should really be Cost -= CallPenalty. The penalty should actually be defined by target.


Repository:
  rL LLVM

https://reviews.llvm.org/D24338