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

[David Li via llvm-commits]

davidxl added a comment.

You


================
Comment at: include/llvm/Analysis/InlineCost.h:114
@@ +113,3 @@
+/// options.
+struct InlineKnobs {
+  int HintThreshold;
----------------
eraman wrote:
> davidxl wrote:
> > I don't quite like the name 'Knobs'. Perhaps more plain "Parameters"?
> Ok, will rename this.
The renaming to Parameters has not being done.

================
Comment at: include/llvm/Analysis/InlineCost.h:116
@@ +115,3 @@
+  int HintThreshold;
+  Optional<int> ColdThreshold;
+  Optional<int> OptSizeThreshold;
----------------
Add brief documentation on each params

================
Comment at: include/llvm/Analysis/InlineCost.h:132
@@ -112,3 +131,3 @@
 /// inlining the callsite. It is an expensive, heavyweight call.
 InlineCost getInlineCost(CallSite CS, int DefaultThreshold,
                          TargetTransformInfo &CalleeTTI,
----------------
Should defaultThreshold be wrapped in inline-params too? 

Edit: probably not. The defaultThreshold here means many different things (opt level specific default when default-threshold option is not specified, threshold passed in API, or indirect call threshold etc.) -- I hope this can be cleaned up at some point:  cost analysis should just look at the InlineParams to make decisions.


https://reviews.llvm.org/D22120