Refactor threshold computation for inline cost analysis

[eraman]

Summary

This refactoring

• Moves the threshold computation methods to InlineCost.cpp.
• Removes state related to Threshold from the base Inliner class and moves it to SimpleInliner since threshold is only relevant for ICA which is used by SimpleInliner.
• Cleans up getInlineThreshold a little bit.

NFC intended. The motivation for this refactor is to allow getInlineThreshold to be called from inside CallAnalyzer when peering through indirect calls instead of using a constant value.

[hfinkel]

include/llvm/Analysis/InlineCost.h

145 ↗

(On Diff #42359)

Given that we don't really have a SizeOptLevel in LLVM, but -Os, -Oz settings, we should document here what SizeOptLevel means.

lib/Analysis/InlineCost.cpp

1421 ↗

(On Diff #42359)

Seems like we could fix this FIXME while we're here.

[davidxl]

include/llvm/Analysis/InlineCost.h

145 ↗

(On Diff #42359)

make it a static member function of InlineCostAnalysis?

148 ↗

(On Diff #42359)

Same here.

lib/Analysis/InlineCost.cpp

1385 ↗

(On Diff #42359)

Change it to DefaultInlineThreshold?

1414 ↗

(On Diff #42359)

Should it be a static member function of InlineCostAnalysis class?

1414 ↗

(On Diff #42359)

Missing documentation of the method.

1414 ↗

(On Diff #42359)

The interface of this method also needs to be changed to take an additional callee function as the argument so that indirect callsite's threshold can be computed.

The existing method can be a wrapper to this one.

lib/Transforms/IPO/InlineSimple.cpp

41 ↗

(On Diff #42359)

-> DefaultThreshold

57 ↗

(On Diff #42359)

This does not look right -- DefaultThreshold is used here which should not be.

[davidxl]

I also don't see the need to pass CallsiteIndependent Threshold all the way down from SimpleInliner::getInlineCost all the way down to CallAnalyzer (without being used anywhere else) -- the CallAnalyzer can simply compute the CallSiteIndependentThreshold itself. This will greatly simplify the interfaces.

lib/Analysis/InlineCost.cpp

1414 ↗

(On Diff #42359)

Why do we need a threshold parameter here? The base threshold should be the default threshold.

[eraman]

include/llvm/Analysis/InlineCost.h

145 ↗

(On Diff #42359)

I initially had it as a static member function, but the thought of calling this inside CallAnalyzer (not in this patch, but when replacing the constant threshold for indirect call bonus) didn't feel right to me (CallAnalyzer being 'aware' of InlineCostAnalysis). Functionally there is no difference and if you strongly prefer I'll make it a static member function.

lib/Analysis/InlineCost.cpp

1385 ↗

(On Diff #42359)

I don't really like the name, but went ahead with it. Of course we don't want to change the option itself.

1414 ↗

(On Diff #42359)

The idea of calling a method of InlineCostAnalysis from this inside CallAnalyzer (not in this patch, but when replacing the constant threshold for indirect call bonus) doesn't feel right to me. Functionally there is no difference and if you strongly prefer I'll make it a static member function.

1414 ↗

(On Diff #42359)

Since there is only one caller, I haven't added the wrapper.

1414 ↗

(On Diff #42359)

I am not sure what you mean by the base threshold. You pass a threshold that is not dependent on the callsite. This may get changed based on callsite properties and returned.

1421 ↗

(On Diff #42359)

If the function has a MinSize attribute, this OptSize bool will be false, but I think that is not intentional. If I want a NFC patch, I should replace Caller->hasFnAttribute(Attribute::OptimizeForSize) with (Caller->optForSize() && ! Caller->optForMinSize()) to replicate the exact behavior. Instead I thought I'll just replace it with Caller->optForSize() as a separate patch (or perhaps I am obsessing a lot over keeping this NFC since this not using a smaller threshold for MinSize attribute seems unintentional)

lib/Transforms/IPO/InlineSimple.cpp

57 ↗

(On Diff #42359)

Why? the Threshold could be either the default one or based on opt levels. This is the callsite independent threshold which is modified by getInlineCost

[eraman]

Addressed reviewer comments (mainly variable renaming, documentation and making some functions static member functions)

[davidxl]

Threshold is used in inline cost analysis to bail out early for compile time reasons, so it is an implementation detail that should not be exposed at interface level. Currently it is needed to be passed because the DefaultThreshold (aka callSiteIndependent Threshold) depends on opt level, sizeOptLevel which ICA does not have access to.

The solution is pretty simple: just pass the optlevel to ICA when ICA is created. If Threshold does need to be passed for some reason (which I doubt) please rename such parameters (including the member field in SimpleInliner) to be DefaultThreshold (with comment it depends on opt level) to avoid confusion. CallsiteIndependentThreshold is too long a name to use.

include/llvm/Analysis/InlineCost.h

145 ↗

(On Diff #42359)

SizeOptLevel == 1 maps to -Os, while SizeOptLevel == 2 maps to -Oz. In fact, FE also sets OptimizeForSize and MinSize function attribute when the command line option is on:

if (!HasOptnone) {

  if (CodeGenOpts.OptimizeSize)
    FuncAttrs.addAttribute(llvm::Attribute::OptimizeForSize);
  if (CodeGenOpts.OptimizeSize == 2)
    FuncAttrs.addAttribute(llvm::Attribute::MinSize);
}

This means inliner SizeOptLevel is not needed here.

lib/Transforms/IPO/InlineSimple.cpp

57 ↗

(On Diff #42359)

Ok -- I see that the getInlineThreshold call is pushed into getInlineCost method so this is correct.

[davidxl]

lib/Analysis/InlineCost.cpp

1414 ↗

(On Diff #42464)

sounds reasonable.

1414 ↗

(On Diff #42464)

I thought the threshold was the opt level independent one -- but looks like not. See my previous comments -- we should need to pass any threshold.

[eraman]

In D15401#309175, @davidxl wrote:

Threshold is used in inline cost analysis to bail out early for compile time reasons, so it is an implementation detail that should not be exposed at interface level. Currently it is needed to be passed because the DefaultThreshold (aka callSiteIndependent Threshold) depends on opt level, sizeOptLevel which ICA does not have access to.

The solution is pretty simple: just pass the optlevel to ICA when ICA is created. If Threshold does need to be passed for some reason (which I doubt) please rename such parameters (including the member field in SimpleInliner) to be DefaultThreshold (with comment it depends on opt level) to avoid confusion. CallsiteIndependentThreshold is too long a name to use.

SimpleInliner instances are created through createFunctionInliningPass. There are three overloads of this method. One passes the size and opt level, but the other two either explicitly passes the threshold (this is used in bugpoint) or uses a default threshold. So there is no way to avoid passing the threshold. I will do the renaming.

include/llvm/Analysis/InlineCost.h

153 ↗

(On Diff #42464)

Unfortunately, there is subtle difference in how the threshold is computed for -Os and OptimizeForSize attribute. If -Os is specified, the default threshold is lowered to 75. This is independent of whether we pass -inline-threshold or not. However, if -Os is not specified and the caller function has OptimizeForSize attribute, the threshold is lowered only if -inline-threshold is not specified.

Additionally if -Oz is specified, the threshold is lowered to 25, but the getInlineThreshold does not check for MinSize attribute at all.

Clearly, this needs to be fixed to give a consistent behavior but I think should be done separately.

[junbuml]

lib/Analysis/InlineCost.cpp

1444 ↗

(On Diff #42464)

It appears that here we check only fn attributes for Callee, but I think it's also possible for a CS can have its own attributes. Shouldn't we check CS.hasFnAttr(Attribute::Cold) or CS.hasFnAttr(Attribute::InlineHint) as well? E.g., a Callee itself is not Cold, but a CS to the Callee is Cold. If yes, may be a separate patch.

[davidxl]

lib/Analysis/InlineCost.cpp

1444 ↗

(On Diff #42464)

This is a refactoring patch, so what you suggested can be done in a separate patch.

[eraman]

Renamed CallSiteIndependentThreshold to DefaultThreshold and fixed some comments.

[chandlerc]

I generally like the direction here, but I think you should actually go further than this.

I think you should change the input to the cost analysis from a numeric threshold to a symbolic enum that selects between the high-level "kinds" of thresholds to use. Then you can internalize all of the logic inside of the cost analysis.

Further, you should feel relatively free to change the behavior of the '-inline-threshold' flag. That flag is primarily a debugging aid. We don't want to flagrantly change its behavior (as that would just be annoying), we don't have any firm or hard contract around exactly how that flag is interpreted. Does that make sense?

[eraman]

In D15401#311342, @chandlerc wrote:

I generally like the direction here, but I think you should actually go further than this.

I think you should change the input to the cost analysis from a numeric threshold to a symbolic enum that selects between the high-level "kinds" of thresholds to use. Then you can internalize all of the logic inside of the cost analysis.

Further, you should feel relatively free to change the behavior of the '-inline-threshold' flag. That flag is primarily a debugging aid. We don't want to flagrantly change its behavior (as that would just be annoying), we don't have any firm or hard contract around exactly how that flag is interpreted. Does that make sense?

Passing an absolute number in inline-threshold is useful in writing tests. How about passing a symbolic enum to the getInlineCost, modify it based on properties of callsite but finally override it with -inline-threshold (or its variants) if it is explicitly passed. Does this sound reasonable?

[chandlerc]

In D15401#311397, @eraman wrote:

In D15401#311342, @chandlerc wrote:

I generally like the direction here, but I think you should actually go further than this.

I think you should change the input to the cost analysis from a numeric threshold to a symbolic enum that selects between the high-level "kinds" of thresholds to use. Then you can internalize all of the logic inside of the cost analysis.

Further, you should feel relatively free to change the behavior of the '-inline-threshold' flag. That flag is primarily a debugging aid. We don't want to flagrantly change its behavior (as that would just be annoying), we don't have any firm or hard contract around exactly how that flag is interpreted. Does that make sense?

Passing an absolute number in inline-threshold is useful in writing tests. How about passing a symbolic enum to the getInlineCost, modify it based on properties of callsite but finally override it with -inline-threshold (or its variants) if it is explicitly passed. Does this sound reasonable?

I'm just suggesting changing the *APIs* to deal in symbolic settings. Clearly the flag will be numeric. I'm suggested sinking the inline threshold flag into the inline cost analysis completely.

[davidxl]

I think we should do what Chandler suggested as a follow up. This patch IMO is good enough as an incremental step. We should move a little faster as there are also other cleanups mentioned in the review (Os command line vs attribute, Cold Function attribute and callsite attribute) needed to to be done.

[eraman]

I'm just suggesting changing the *APIs* to deal in symbolic settings. Clearly the flag will be numeric. I'm suggested sinking the inline threshold flag into the inline cost analysis completely.

I attempted to do that and hit an issue. InlineSimple.cpp provides a createFunctionInliningPass((int Threshold) API. To sink thresholds to InlineCost, this needs to be removed, but this is called by LLVMPassManagerBuilderUseInlinerWithThreshold which is exposed by the llvm-c API

[chandlerc]

In D15401#312525, @eraman wrote:

I'm just suggesting changing the *APIs* to deal in symbolic settings. Clearly the flag will be numeric. I'm suggested sinking the inline threshold flag into the inline cost analysis completely.

I attempted to do that and hit an issue. InlineSimple.cpp provides a createFunctionInliningPass((int Threshold) API. To sink thresholds to InlineCost, this needs to be removed, but this is called by LLVMPassManagerBuilderUseInlinerWithThreshold which is exposed by the llvm-c API

I had to spend a bunch of time thinking about this.

On one hand, I think exposing this kind of configurability is really frustrating from an API-design perspective. But I think I can imagine users of LLVM (particularly library users) wanting to have pretty wildly different inlining tolerances. However, this raises an important question of how that should be propagated to when the cost analysis has to recurse across yet another function call.

I think we need to move *completely* away from having different *initial* thresholds for things like inline-hint and opt-size or min-size. We have numerous adjustments to the threshold based on different analyses properties. I think inline-hint and size based stuff should work the same way. This will let you just sink the capping and ballooning of the threshold into analyzeCall where we also compute all the bonuses. Does that make sense? It should also avoid the need to separately call 'getInlineThreshold' -- you'll just store and pass along the initial threshold.

[chandlerc]

In D15401#314253, @chandlerc wrote:

In D15401#312525, @eraman wrote:

I'm just suggesting changing the *APIs* to deal in symbolic settings. Clearly the flag will be numeric. I'm suggested sinking the inline threshold flag into the inline cost analysis completely.

I attempted to do that and hit an issue. InlineSimple.cpp provides a createFunctionInliningPass((int Threshold) API. To sink thresholds to InlineCost, this needs to be removed, but this is called by LLVMPassManagerBuilderUseInlinerWithThreshold which is exposed by the llvm-c API

I had to spend a bunch of time thinking about this.

On one hand, I think exposing this kind of configurability is really frustrating from an API-design perspective. But I think I can imagine users of LLVM (particularly library users) wanting to have pretty wildly different inlining tolerances. However, this raises an important question of how that should be propagated to when the cost analysis has to recurse across yet another function call.

I think we need to move *completely* away from having different *initial* thresholds for things like inline-hint and opt-size or min-size. We have numerous adjustments to the threshold based on different analyses properties. I think inline-hint and size based stuff should work the same way. This will let you just sink the capping and ballooning of the threshold into analyzeCall where we also compute all the bonuses. Does that make sense? It should also avoid the need to separately call 'getInlineThreshold' -- you'll just store and pass along the initial threshold.

One thing that may make it much easier to innact this refactoring in a simple way would be to first do another much over-due refactoring to make InlineCostAnalysis not *actually* be an analysis *pass*. The only "analysis" done is to capture two pointers. Instead, this should just be a utility class and method that the inliner pass constructs (passing in the relevant bits) and uses.

If it would be helpful, I can send you a patch that does that, but I don't want to make merging and such more complicated if it makes more sense for you to do this on your end.

[eraman]

Moved the code that updates threshold based on callsite properties to CallAnalyzer. This also merges in the changes made in http://reviews.llvm.org/D15245

[eraman]

Ping.

[davidxl]

This version looks very good -- the only thing I dislike is that we still need to pass default threshold around via getInlineCost interface -- I think we can deal with that in the future.

[eraman]

Ping

[davidxl]

Sorry about the delay!

This patch is NFC cleanup with general direction agreed upon. The version LGTM (with the documentation nit). Please follow up post-commit if there are more issues to be handled.

lib/Transforms/IPO/InlineSimple.cpp

41 ↗

(On Diff #43710)

Document this field. The DefaultThreshold can either be the default threshold specified by -inline-threshold, or the opt level dependent default threshold or a user specified value.

[davidxl]

lib/Analysis/InlineCost.cpp

197 ↗

(On Diff #43710)

A related follow up that can be done:

1. change the parameter name to DefaultThreshold
2. Save the default threshold value in a different member so that it is not overriden by updatethreshold
3. Use the default threshold when recurisively analyizing the indirect call (instead of a constant).

(Not required for this patch).

[eraman]

Chandler, if you don't have any other comments, I'll check this in tomorrow.

lib/Analysis/InlineCost.cpp

197 ↗

(On Diff #43710)

Ok, will address these in a separate patch.

llvm/trunk/include/llvm/Analysis/InlineCost.h

public:
   /// Only valid if the cost is of the variable kind. Returns a negative
   /// value if the cost is too high to inline.
   int getCostDelta() const { return Threshold - getCost(); }
 };
 
 /// \brief Get an InlineCost object representing the cost of inlining this
 /// callsite.
 ///
-/// Note that threshold is passed into this function. Only costs below the
-/// threshold are computed with any accuracy. The threshold can be used to
-/// bound the computation necessary to determine whether the cost is
+/// Note that a default threshold is passed into this function. This threshold
+/// could be modified based on callsite's properties and only costs below this
+/// new threshold are computed with any accuracy. The new threshold can be
+/// used to bound the computation necessary to determine whether the cost is
 /// sufficiently low to warrant inlining.
 ///
 /// Also note that calling this function *dynamically* computes the cost of
 /// inlining the callsite. It is an expensive, heavyweight call.
-InlineCost getInlineCost(CallSite CS, int Threshold,
+InlineCost getInlineCost(CallSite CS, int DefaultThreshold,
                          TargetTransformInfo &CalleeTTI,
                          AssumptionCacheTracker *ACT);
 
 /// \brief Get an InlineCost with the callee explicitly specified.
 /// This allows you to calculate the cost of inlining a function via a
 /// pointer. This behaves exactly as the version with no explicit callee
 /// parameter in all other respects.
 //
-InlineCost getInlineCost(CallSite CS, Function *Callee, int Threshold,
+InlineCost getInlineCost(CallSite CS, Function *Callee, int DefaultThreshold,
                          TargetTransformInfo &CalleeTTI,
                          AssumptionCacheTracker *ACT);
 
+int computeThresholdFromOptLevels(unsigned OptLevel, unsigned SizeOptLevel);
+
+/// \brief Return the default value of -inline-threshold.
+int getDefaultInlineThreshold();
+
 /// \brief Minimal filter to detect invalid constructs for inlining.
 bool isInlineViable(Function &Callee);
 }
 
 #endif

llvm/trunk/include/llvm/Transforms/IPO/InlinerPass.h

 class InlineCost;
 template <class PtrType, unsigned SmallSize> class SmallPtrSet;
 
 /// Inliner - This class contains all of the helper code which is used to
 /// perform the inlining operations that do not depend on the policy.
 ///
 struct Inliner : public CallGraphSCCPass {
   explicit Inliner(char &ID);
-  explicit Inliner(char &ID, int Threshold, bool InsertLifetime);
+  explicit Inliner(char &ID, bool InsertLifetime);
 
   /// getAnalysisUsage - For this class, we declare that we require and preserve
   /// the call graph.  If the derived class implements this method, it should
   /// always explicitly call the implementation here.
   void getAnalysisUsage(AnalysisUsage &Info) const override;
 
   // Main run interface method, this implements the interface required by the
   // Pass class.
   bool runOnSCC(CallGraphSCC &SCC) override;
 
   using llvm::Pass::doFinalization;
   // doFinalization - Remove now-dead linkonce functions at the end of
   // processing to avoid breaking the SCC traversal.
   bool doFinalization(CallGraph &CG) override;
 
-  /// This method returns the value specified by the -inline-threshold value,
-  /// specified on the command line.  This is typically not directly needed.
-  ///
-  unsigned getInlineThreshold() const { return InlineThreshold; }
-
-  /// Calculate the inline threshold for given Caller. This threshold is lower
-  /// if the caller is marked with OptimizeForSize and -inline-threshold is not
-  /// given on the comand line. It is higher if the callee is marked with the
-  /// inlinehint attribute.
-  ///
-  unsigned getInlineThreshold(CallSite CS) const;
-
   /// getInlineCost - This method must be implemented by the subclass to
   /// determine the cost of inlining the specified call site.  If the cost
   /// returned is greater than the current inline threshold, the call site is
   /// not inlined.
   ///
   virtual InlineCost getInlineCost(CallSite CS) = 0;
 
   /// removeDeadFunctions - Remove dead functions.
   ///
   /// This also includes a hack in the form of the 'AlwaysInlineOnly' flag
   /// which restricts it to deleting functions with an 'AlwaysInline'
   /// attribute. This is useful for the InlineAlways pass that only wants to
   /// deal with that subset of the functions.
   bool removeDeadFunctions(CallGraph &CG, bool AlwaysInlineOnly = false);
 
 private:
-  // InlineThreshold - Cache the value here for easy access.
-  unsigned InlineThreshold;
-
   // InsertLifetime - Insert @llvm.lifetime intrinsics.
   bool InsertLifetime;
 
   /// shouldInline - Return true if the inliner should attempt to
   /// inline at the given CallSite.
   bool shouldInline(CallSite CS);
 
 protected:
   AssumptionCacheTracker *ACT;
 };
 
 } // End llvm namespace
 
 #endif

llvm/trunk/lib/Analysis/InlineCost.cpp

 #include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 
 #define DEBUG_TYPE "inline-cost"
 
 STATISTIC(NumCallsAnalyzed, "Number of call sites analyzed");
 
+// Threshold to use when optsize is specified (and there is no
+// -inline-threshold).
+const int OptSizeThreshold = 75;
+
+// Threshold to use when -Oz is specified (and there is no -inline-threshold).
+const int OptMinSizeThreshold = 25;
+
+// Threshold to use when -O[34] is specified (and there is no
+// -inline-threshold).
+const int OptAggressiveThreshold = 275;
+
+static cl::opt<int> DefaultInlineThreshold(
+    "inline-threshold", cl::Hidden, cl::init(225), cl::ZeroOrMore,
+    cl::desc("Control the amount of inlining to perform (default = 225)"));
+
+static cl::opt<int> HintThreshold(
+    "inlinehint-threshold", cl::Hidden, cl::init(325),
+    cl::desc("Threshold for inlining functions with inline hint"));
+
+// We introduce this threshold to help performance of instrumentation based
+// PGO before we actually hook up inliner with analysis passes such as BPI and
+// BFI.
+static cl::opt<int> ColdThreshold(
+    "inlinecold-threshold", cl::Hidden, cl::init(225),
+    cl::desc("Threshold for inlining functions with cold attribute"));
+
 namespace {
 
 class CallAnalyzer : public InstVisitor<CallAnalyzer, bool> {
   typedef InstVisitor<CallAnalyzer, bool> Base;
   friend class InstVisitor<CallAnalyzer, bool>;
 
   /// The TargetTransformInfo available for this compilation.
   const TargetTransformInfo &TTI;
class CallAnalyzer : public InstVisitor<CallAnalyzer, bool> {
   /// attributes since these can be more precise than the ones on the callee
   /// itself.
   bool paramHasAttr(Argument *A, Attribute::AttrKind Attr);
   
   /// Return true if the given value is known non null within the callee if
   /// inlined through this particular callsite.
   bool isKnownNonNullInCallee(Value *V);
 
+  /// Update Threshold based on callsite properties such as callee
+  /// attributes and callee hotness for PGO builds. The Callee is explicitly
+  /// passed to support analyzing indirect calls whose target is inferred by
+  /// analysis.
+  void updateThreshold(CallSite CS, Function &Callee);
+
   // Custom analysis routines.
   bool analyzeBlock(BasicBlock *BB, SmallPtrSetImpl<const Value *> &EphValues);
 
   // Disable several entry points to the visitor so we don't accidentally use
   // them by declaring but not defining them here.
   void visit(Module *);     void visit(Module &);
   void visit(Function *);   void visit(Function &);
   void visit(BasicBlock *); void visit(BasicBlock &);
if (isAllocaDerivedArg(V))
     // We can actually predict the result of comparisons between an
     // alloca-derived value and null. Note that this fires regardless of
     // SROA firing.
     return true;
   
   return false;
 }
 
+void CallAnalyzer::updateThreshold(CallSite CS, Function &Callee) {
+  // If -inline-threshold is not given, listen to the optsize attribute when it
+  // would decrease the threshold.
+  Function *Caller = CS.getCaller();
+
+  // FIXME: Use Function::optForSize()
+  bool OptSize = Caller->hasFnAttribute(Attribute::OptimizeForSize);
+
+  if (!(DefaultInlineThreshold.getNumOccurrences() > 0) && OptSize &&
+      OptSizeThreshold < Threshold)
+    Threshold = OptSizeThreshold;
+
+  // If profile information is available, use that to adjust threshold of hot
+  // and cold functions.
+  // FIXME: The heuristic used below for determining hotness and coldness are
+  // based on preliminary SPEC tuning and may not be optimal. Replace this with
+  // a well-tuned heuristic based on *callsite* hotness and not callee hotness.
+  uint64_t FunctionCount = 0, MaxFunctionCount = 0;
+  bool HasPGOCounts = false;
+  if (Callee.getEntryCount() && Callee.getParent()->getMaximumFunctionCount()) {
+    HasPGOCounts = true;
+    FunctionCount = Callee.getEntryCount().getValue();
+    MaxFunctionCount = Callee.getParent()->getMaximumFunctionCount().getValue();
+  }
+
+  // Listen to the inlinehint attribute or profile based hotness information
+  // when it would increase the threshold and the caller does not need to
+  // minimize its size.
+  bool InlineHint =
+      Callee.hasFnAttribute(Attribute::InlineHint) ||
+      (HasPGOCounts &&
+       FunctionCount >= (uint64_t)(0.3 * (double)MaxFunctionCount));
+  if (InlineHint && HintThreshold > Threshold && !Caller->optForMinSize())
+    Threshold = HintThreshold;
+
+  // Listen to the cold attribute or profile based coldness information
+  // when it would decrease the threshold.
+  bool ColdCallee =
+      Callee.hasFnAttribute(Attribute::Cold) ||
+      (HasPGOCounts &&
+       FunctionCount <= (uint64_t)(0.01 * (double)MaxFunctionCount));
+  // Command line argument for DefaultInlineThreshold will override the default
+  // ColdThreshold. If we have -inline-threshold but no -inlinecold-threshold,
+  // do not use the default cold threshold even if it is smaller.
+  if ((DefaultInlineThreshold.getNumOccurrences() == 0 ||
+       ColdThreshold.getNumOccurrences() > 0) &&
+      ColdCallee && ColdThreshold < Threshold)
+    Threshold = ColdThreshold;
+}
+
 bool CallAnalyzer::visitCmpInst(CmpInst &I) {
   Value *LHS = I.getOperand(0), *RHS = I.getOperand(1);
   // First try to handle simplified comparisons.
   if (!isa<Constant>(LHS))
     if (Constant *SimpleLHS = SimplifiedValues.lookup(LHS))
       LHS = SimpleLHS;
   if (!isa<Constant>(RHS))
     if (Constant *SimpleRHS = SimplifiedValues.lookup(RHS))
bool CallAnalyzer::analyzeCall(CallSite CS) {
   // threshold, and we'll lower it if the % of vector instructions gets too
   // low. Note that these bonuses are some what arbitrary and evolved over time
   // by accident as much as because they are principled bonuses.
   //
   // FIXME: It would be nice to remove all such bonuses. At least it would be
   // nice to base the bonus values on something more scientific.
   assert(NumInstructions == 0);
   assert(NumVectorInstructions == 0);
+
+  // Update the threshold based on callsite properties
+  updateThreshold(CS, F);
+
   FiftyPercentVectorBonus = 3 * Threshold / 2;
   TenPercentVectorBonus = 3 * Threshold / 4;
   const DataLayout &DL = F.getParent()->getDataLayout();
 
   // Track whether the post-inlining function would have more than one basic
   // block. A single basic block is often intended for inlining. Balloon the
   // threshold by 50% until we pass the single-BB phase.
   bool SingleBB = true;

 ///        that prevent inlining.
 static bool functionsHaveCompatibleAttributes(Function *Caller,
                                               Function *Callee,
                                               TargetTransformInfo &TTI) {
   return TTI.areInlineCompatible(Caller, Callee) &&
          AttributeFuncs::areInlineCompatible(*Caller, *Callee);
 }
 
-InlineCost llvm::getInlineCost(CallSite CS, int Threshold,
+InlineCost llvm::getInlineCost(CallSite CS, int DefaultThreshold,
                                TargetTransformInfo &CalleeTTI,
                                AssumptionCacheTracker *ACT) {
-  return getInlineCost(CS, CS.getCalledFunction(), Threshold, CalleeTTI, ACT);
+  return getInlineCost(CS, CS.getCalledFunction(), DefaultThreshold, CalleeTTI,
+                       ACT);
+}
+
+int llvm::computeThresholdFromOptLevels(unsigned OptLevel,
+                                        unsigned SizeOptLevel) {
+  if (OptLevel > 2)
+    return OptAggressiveThreshold;
+  if (SizeOptLevel == 1) // -Os
+    return OptSizeThreshold;
+  if (SizeOptLevel == 2) // -Oz
+    return OptMinSizeThreshold;
+  return DefaultInlineThreshold;
 }
 
-InlineCost llvm::getInlineCost(CallSite CS, Function *Callee, int Threshold,
+int llvm::getDefaultInlineThreshold() { return DefaultInlineThreshold; }
+
+InlineCost llvm::getInlineCost(CallSite CS, Function *Callee,
+                               int DefaultThreshold,
                                TargetTransformInfo &CalleeTTI,
                                AssumptionCacheTracker *ACT) {
+
   // Cannot inline indirect calls.
   if (!Callee)
     return llvm::InlineCost::getNever();
 
   // Calls to functions with always-inline attributes should be inlined
   // whenever possible.
   if (CS.hasFnAttr(Attribute::AlwaysInline)) {
     if (isInlineViable(*Callee))
InlineCost llvm::getInlineCost(CallSite CS, Function *Callee,
   // marked noinline.
   if (Callee->mayBeOverridden() ||
       Callee->hasFnAttribute(Attribute::NoInline) || CS.isNoInline())
     return llvm::InlineCost::getNever();
 
   DEBUG(llvm::dbgs() << "      Analyzing call of " << Callee->getName()
         << "...\n");
 
-  CallAnalyzer CA(CalleeTTI, ACT, *Callee, Threshold, CS);
+  CallAnalyzer CA(CalleeTTI, ACT, *Callee, DefaultThreshold, CS);
   bool ShouldInline = CA.analyzeCall(CS);
 
   DEBUG(CA.dump());
 
   // Check if there was a reason to force inlining or no inlining.
   if (!ShouldInline && CA.getCost() < CA.getThreshold())
     return InlineCost::getNever();
   if (ShouldInline && CA.getCost() >= CA.getThreshold())

llvm/trunk/lib/Transforms/IPO/InlineAlways.cpp

 #define DEBUG_TYPE "inline"
 
 namespace {
 
 /// \brief Inliner pass which only handles "always inline" functions.
 class AlwaysInliner : public Inliner {
 
 public:
-  // Use extremely low threshold.
-  AlwaysInliner() : Inliner(ID, -2000000000, /*InsertLifetime*/ true) {
+  AlwaysInliner() : Inliner(ID, /*InsertLifetime*/ true) {
     initializeAlwaysInlinerPass(*PassRegistry::getPassRegistry());
   }
 
-  AlwaysInliner(bool InsertLifetime)
-      : Inliner(ID, -2000000000, InsertLifetime) {
+  AlwaysInliner(bool InsertLifetime) : Inliner(ID, InsertLifetime) {
     initializeAlwaysInlinerPass(*PassRegistry::getPassRegistry());
   }
 
   static char ID; // Pass identification, replacement for typeid
 
   InlineCost getInlineCost(CallSite CS) override;
 
   using llvm::Pass::doFinalization;

llvm/trunk/lib/Transforms/IPO/InlineSimple.cpp

 namespace {
 
 /// \brief Actual inliner pass implementation.
 ///
 /// The common implementation of the inlining logic is shared between this
 /// inliner pass and the always inliner pass. The two passes use different cost
 /// analyses to determine when to inline.
 class SimpleInliner : public Inliner {
+  // This field is populated based on one of the following:
+  //  optimization or size optimization levels,
+  //  --inline-threshold flag,
+  //  user specified value.
+  int DefaultThreshold;
 
 public:
-  SimpleInliner() : Inliner(ID) {
+  SimpleInliner()
+      : Inliner(ID), DefaultThreshold(llvm::getDefaultInlineThreshold()) {
     initializeSimpleInlinerPass(*PassRegistry::getPassRegistry());
   }
 
-  SimpleInliner(int Threshold)
-      : Inliner(ID, Threshold, /*InsertLifetime*/ true) {
+  SimpleInliner(int Threshold) : Inliner(ID), DefaultThreshold(Threshold) {
     initializeSimpleInlinerPass(*PassRegistry::getPassRegistry());
   }
 
   static char ID; // Pass identification, replacement for typeid
 
   InlineCost getInlineCost(CallSite CS) override {
     Function *Callee = CS.getCalledFunction();
     TargetTransformInfo &TTI = TTIWP->getTTI(*Callee);
-    return llvm::getInlineCost(CS, getInlineThreshold(CS), TTI, ACT);
+    return llvm::getInlineCost(CS, DefaultThreshold, TTI, ACT);
   }
 
   bool runOnSCC(CallGraphSCC &SCC) override;
   void getAnalysisUsage(AnalysisUsage &AU) const override;
 
 private:
   TargetTransformInfoWrapperPass *TTIWP;
 };
 
-static int computeThresholdFromOptLevels(unsigned OptLevel,
-                                         unsigned SizeOptLevel) {
-  if (OptLevel > 2)
-    return 275;
-  if (SizeOptLevel == 1) // -Os
-    return 75;
-  if (SizeOptLevel == 2) // -Oz
-    return 25;
-  return 225;
-}
-
 } // end anonymous namespace
 
 char SimpleInliner::ID = 0;
 INITIALIZE_PASS_BEGIN(SimpleInliner, "inline",
                 "Function Integration/Inlining", false, false)
 INITIALIZE_PASS_DEPENDENCY(AssumptionCacheTracker)
 INITIALIZE_PASS_DEPENDENCY(CallGraphWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(TargetTransformInfoWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(TargetLibraryInfoWrapperPass)
 INITIALIZE_PASS_END(SimpleInliner, "inline",
                 "Function Integration/Inlining", false, false)
 
 Pass *llvm::createFunctionInliningPass() { return new SimpleInliner(); }
 
 Pass *llvm::createFunctionInliningPass(int Threshold) {
   return new SimpleInliner(Threshold);
 }
 
 Pass *llvm::createFunctionInliningPass(unsigned OptLevel,
                                        unsigned SizeOptLevel) {
   return new SimpleInliner(
-      computeThresholdFromOptLevels(OptLevel, SizeOptLevel));
+      llvm::computeThresholdFromOptLevels(OptLevel, SizeOptLevel));
 }
 
 bool SimpleInliner::runOnSCC(CallGraphSCC &SCC) {
   TTIWP = &getAnalysis<TargetTransformInfoWrapperPass>();
   return Inliner::runOnSCC(SCC);
 }
 
 void SimpleInliner::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.addRequired<TargetTransformInfoWrapperPass>();
   Inliner::getAnalysisUsage(AU);
 }

llvm/trunk/lib/Transforms/IPO/Inliner.cpp

 STATISTIC(NumDeleted, "Number of functions deleted because all callers found");
 STATISTIC(NumMergedAllocas, "Number of allocas merged together");
 
 // This weirdly named statistic tracks the number of times that, when attempting
 // to inline a function A into B, we analyze the callers of B in order to see
 // if those would be more profitable and blocked inline steps.
 STATISTIC(NumCallerCallersAnalyzed, "Number of caller-callers analyzed");
 
-static cl::opt<int>
-InlineLimit("inline-threshold", cl::Hidden, cl::init(225), cl::ZeroOrMore,
-        cl::desc("Control the amount of inlining to perform (default = 225)"));
-
-static cl::opt<int>
-HintThreshold("inlinehint-threshold", cl::Hidden, cl::init(325),
-              cl::desc("Threshold for inlining functions with inline hint"));
-
-// We instroduce this threshold to help performance of instrumentation based
-// PGO before we actually hook up inliner with analysis passes such as BPI and
-// BFI.
-static cl::opt<int>
-ColdThreshold("inlinecold-threshold", cl::Hidden, cl::init(225),
-              cl::desc("Threshold for inlining functions with cold attribute"));
-
-// Threshold to use when optsize is specified (and there is no -inline-limit).
-const int OptSizeThreshold = 75;
-
-Inliner::Inliner(char &ID)
-    : CallGraphSCCPass(ID), InlineThreshold(InlineLimit), InsertLifetime(true) {
-}
-
-Inliner::Inliner(char &ID, int Threshold, bool InsertLifetime)
-    : CallGraphSCCPass(ID),
-      InlineThreshold(InlineLimit.getNumOccurrences() > 0 ? InlineLimit
-                                                          : Threshold),
-      InsertLifetime(InsertLifetime) {}
+Inliner::Inliner(char &ID) : CallGraphSCCPass(ID), InsertLifetime(true) {}
+
+Inliner::Inliner(char &ID, bool InsertLifetime)
+    : CallGraphSCCPass(ID), InsertLifetime(InsertLifetime) {}
 
 /// For this class, we declare that we require and preserve the call graph.
 /// If the derived class implements this method, it should
 /// always explicitly call the implementation here.
 void Inliner::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.addRequired<AssumptionCacheTracker>();
   AU.addRequired<TargetLibraryInfoWrapperPass>();
   CallGraphSCCPass::getAnalysisUsage(AU);
for (unsigned AllocaNo = 0, e = IFI.StaticAllocas.size();
     // operation.
     AllocasForType.push_back(AI);
     UsedAllocas.insert(AI);
   }
   
   return true;
 }
 
-unsigned Inliner::getInlineThreshold(CallSite CS) const {
-  int Threshold = InlineThreshold; // -inline-threshold or else selected by
-                                   // overall opt level
-
-  // If -inline-threshold is not given, listen to the optsize attribute when it
-  // would decrease the threshold.
-  Function *Caller = CS.getCaller();
-  bool OptSize = Caller && !Caller->isDeclaration() &&
-                 // FIXME: Use Function::optForSize().
-                 Caller->hasFnAttribute(Attribute::OptimizeForSize);
-  if (!(InlineLimit.getNumOccurrences() > 0) && OptSize &&
-      OptSizeThreshold < Threshold)
-    Threshold = OptSizeThreshold;
-
-  Function *Callee = CS.getCalledFunction();
-  if (!Callee || Callee->isDeclaration())
-    return Threshold;
-
-  // If profile information is available, use that to adjust threshold of hot
-  // and cold functions.
-  // FIXME: The heuristic used below for determining hotness and coldness are
-  // based on preliminary SPEC tuning and may not be optimal. Replace this with
-  // a well-tuned heuristic based on *callsite* hotness and not callee hotness.
-  uint64_t FunctionCount = 0, MaxFunctionCount = 0;
-  bool HasPGOCounts = false;
-  if (Callee->getEntryCount() &&
-      Callee->getParent()->getMaximumFunctionCount()) {
-    HasPGOCounts = true;
-    FunctionCount = Callee->getEntryCount().getValue();
-    MaxFunctionCount =
-        Callee->getParent()->getMaximumFunctionCount().getValue();
-  }
-
-  // Listen to the inlinehint attribute or profile based hotness information
-  // when it would increase the threshold and the caller does not need to
-  // minimize its size.
-  bool InlineHint =
-      Callee->hasFnAttribute(Attribute::InlineHint) ||
-      (HasPGOCounts &&
-       FunctionCount >= (uint64_t)(0.3 * (double)MaxFunctionCount));
-  if (InlineHint && HintThreshold > Threshold &&
-      !Caller->hasFnAttribute(Attribute::MinSize))
-    Threshold = HintThreshold;
-
-  // Listen to the cold attribute or profile based coldness information
-  // when it would decrease the threshold.
-  bool ColdCallee =
-      Callee->hasFnAttribute(Attribute::Cold) ||
-      (HasPGOCounts &&
-       FunctionCount <= (uint64_t)(0.01 * (double)MaxFunctionCount));
-  // Command line argument for InlineLimit will override the default
-  // ColdThreshold. If we have -inline-threshold but no -inlinecold-threshold,
-  // do not use the default cold threshold even if it is smaller.
-  if ((InlineLimit.getNumOccurrences() == 0 ||
-       ColdThreshold.getNumOccurrences() > 0) && ColdCallee &&
-      ColdThreshold < Threshold)
-    Threshold = ColdThreshold;
-
-  return Threshold;
-}
-
 static void emitAnalysis(CallSite CS, const Twine &Msg) {
   Function *Caller = CS.getCaller();
   LLVMContext &Ctx = Caller->getContext();
   DebugLoc DLoc = CS.getInstruction()->getDebugLoc();
   emitOptimizationRemarkAnalysis(Ctx, DEBUG_TYPE, *Caller, DLoc, Msg);
 }
 
 /// Return true if the inliner should attempt to inline at the given CallSite.