[PATCH] D30899: [InlineCost] CallPenalty knob to provide custom values

[Evgeny Astigeevich via Phabricator via llvm-commits]

eastig added inline comments.


================
Comment at: include/llvm/Analysis/InlineCost.h:80
+  InlineCost(int Cost, int Threshold, int CallPenalty)
+      : Cost(Cost), Threshold(Threshold), CallPenalty(CallPenalty) {}
 
----------------
efriedma wrote:
> It seems kind of weird to return the CallPenalty here... but I guess you need it in lib/Transforms/IPO/Inliner.cpp?
Yes, this is because of Inliner.cpp.
Should CallPenalty be a part of InlineParams?
# If no, then we know its value either default or cmd-option provided. In such case we can have a function accessing to CallPenalty defined in InlineCost.cpp.
# If yes, its value can be whatever a caller of llvm::getInlineCost has been provided. In such case we need to save the value somewhere. I chose this option. As InlineCost object is only way of communication I stored used CallPenalty in it as it's done for Threshold.

What about having it in TTI?



================
Comment at: include/llvm/Analysis/InlineCost.h:92
   static InlineCost getNever() {
-    return InlineCost(NeverInlineCost, 0);
+    return InlineCost(NeverInlineCost, 0, INT_MIN);
   }
----------------
efriedma wrote:
> Using INT_MIN and INT_MAX seems like it's just asking for trouble with overflow... but I guess we don't actually use it in that case?  Seems better to just use 0.
I used zeros in the initial version. Then I thought if I could use some values which could protect from incorrect uses. Like -infinity penalty means no penalty and no inline. +infinity means a lot of penalty and always inline.
I have nothing against using zeros.


================
Comment at: lib/Analysis/InlineCost.cpp:1235
 
+  updateCallPenaltyValue();
+
----------------
efriedma wrote:
> The call penalty should be the same for every callsite; we shouldn't need to update it here... unless you're planning to change that?
As I wrote above if the penalty is a part of InlineParams it can be different per a call. So we need to update the current value based on the value passed through InlineParams.


https://reviews.llvm.org/D30899