[PATCH] D22120: Move inline threshold related flags to InlineSimple.cpp

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 8 17:56:11 PDT 2016


chandlerc added inline comments.

================
Comment at: lib/Transforms/IPO/InlineSimple.cpp:79-81
@@ +78,5 @@
+
+  // Generate the parameters to tune the inline cost analysis based on
+  // commandline options.
+  InlineParams getInlineParams(int Threshold) {
+    InlineParams Params;
----------------
eraman wrote:
> chandlerc wrote:
> > Sorry, I guess I wasn't clear before...
> > 
> > I'm specifically suggesting keeping all of this logic inside of InlineCost.cpp as how the *default* parameters get set.
> > 
> > If we need the ability to get custom parameters which still reflect the command line flags, there should be some API in InlineCost.h that the inliner queries to get the necessary params.
> > 
> > 
> > My goal here stems from two related principles:
> > - The tunable flags and the constants that interact are are fundamentally in the same unit domain should live in a single place (InlineCost.{h,cpp}).
> > - The inliner *passes* should have no real logic to compute thresholds, they should call well documented APIs provided by the cost layer.
> > 
> > A minor secondary goal -- the inliner shouldn't have to do extra work to get the defaults. Those should just be used. It is when an inliner pass wants to deviate from the defaults that it should take explicit steps and call explicit flags.
> Thanks for the clarification. I have moved getInlineParams and other associated logic that computes the threshold from the flags to InlineCost.cpp. SimpleInliner now calls one of these methods to get an InlineParams object which gets passed to getInlineCost. 
Why can't getInlineCost do this by-default?


https://reviews.llvm.org/D22120





More information about the llvm-commits mailing list