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

152

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

1420

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

[davidxl]

include/llvm/Analysis/InlineCost.h

152

make it a static member function of InlineCostAnalysis?

155

Same here.

lib/Analysis/InlineCost.cpp

1384

Change it to DefaultInlineThreshold?

1413

Should it be a static member function of InlineCostAnalysis class?

1413

Missing documentation of the method.

1413

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

-> DefaultThreshold

58

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

1413

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

[eraman]

include/llvm/Analysis/InlineCost.h

152

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

1384

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

1413

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.

1413

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

1413

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.

1420

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

58

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

152

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

58

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

[davidxl]

lib/Analysis/InlineCost.cpp

1413

sounds reasonable.

1413

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

152

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

1443

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

1443

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

40

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

165

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

165

Ok, will address these in a separate patch.

include/llvm/Analysis/InlineCost.h

public:
 
   // Pass interface implementation.
   void getAnalysisUsage(AnalysisUsage &AU) const override;
   bool runOnSCC(CallGraphSCC &SCC) override;
 
   /// \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);
 
   /// \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.
   //
   //  Note: This is used by out-of-tree passes, please do not remove without
   //  adding a replacement API.
-  InlineCost getInlineCost(CallSite CS, Function *Callee, int Threshold);
+  InlineCost getInlineCost(CallSite CS, Function *Callee, int DefaultThreshold);
 
   /// \brief Minimal filter to detect invalid constructs for inlining.
   bool isInlineViable(Function &Callee);
+
+  /// \brief Compute threshold for a given optimization level.
+  /// SizeOptLevel of 1 corresponds to -Os and 2 corresponds to -Oz. OptLevel N
+  /// corresponds to -ON.
+  static int computeThresholdFromOptLevels(unsigned OptLevel,
+                                           unsigned SizeOptLevel);
+
+  /// \brief Return the default value of -inline-threshold.
+  static int getDefaultInlineThreshold();
 };
 
 }
 
 #endif

include/llvm/Transforms/IPO/InlinerPass.h

namespace llvm {
   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);
 };
 
 } // End llvm namespace
 
 #endif

lib/Analysis/InlineCost.cpp

class CallAnalyzer : public InstVisitor<CallAnalyzer, bool> {
   bool visitIndirectBrInst(IndirectBrInst &IBI);
   bool visitResumeInst(ResumeInst &RI);
   bool visitCleanupReturnInst(CleanupReturnInst &RI);
   bool visitCatchReturnInst(CatchReturnInst &RI);
   bool visitUnreachableInst(UnreachableInst &I);
 
 public:
   CallAnalyzer(const TargetTransformInfo &TTI, AssumptionCacheTracker *ACT,
                Function &Callee, int Threshold, CallSite CSArg)
     : TTI(TTI), ACT(ACT), F(Callee), CandidateCS(CSArg), Threshold(Threshold),
         Cost(0), IsCallerRecursive(false), IsRecursiveCall(false),
         ExposesReturnsTwice(false), HasDynamicAlloca(false),
         ContainsNoDuplicateCall(false), HasReturn(false), HasIndirectBr(false),
         HasFrameEscape(false), AllocatedSize(0), NumInstructions(0),
         NumVectorInstructions(0), FiftyPercentVectorBonus(0),
         TenPercentVectorBonus(0), VectorBonus(0), NumConstantArgs(0),
         NumConstantOffsetPtrArgs(0), NumAllocaArgs(0), NumConstantPtrCmps(0),

 }
 
 bool InlineCostAnalysis::runOnSCC(CallGraphSCC &SCC) {
   TTIWP = &getAnalysis<TargetTransformInfoWrapperPass>();
   ACT = &getAnalysis<AssumptionCacheTracker>();
   return false;
 }
 
-InlineCost InlineCostAnalysis::getInlineCost(CallSite CS, int Threshold) {
-  return getInlineCost(CS, CS.getCalledFunction(), Threshold);
+InlineCost InlineCostAnalysis::getInlineCost(CallSite CS,
+                                             int DefaultThreshold) {
+  return getInlineCost(CS, CS.getCalledFunction(), DefaultThreshold);
 }
 
 /// \brief Test that two functions either have or have not the given attribute
 ///        at the same time.
 template<typename AttrKind>
 static bool attributeMatches(Function *F1, Function *F2, AttrKind Attr) {
   return F1->getFnAttribute(Attr) == F2->getFnAttribute(Attr);
 }
 
 /// \brief Test that there are no attribute conflicts between Caller and Callee
 ///        that prevent inlining.
 static bool functionsHaveCompatibleAttributes(Function *Caller,
                                               Function *Callee,
                                               TargetTransformInfo &TTI) {
   return TTI.areInlineCompatible(Caller, Callee) &&
          attributeMatches(Caller, Callee, Attribute::SanitizeAddress) &&
          attributeMatches(Caller, Callee, Attribute::SanitizeMemory) &&
          attributeMatches(Caller, Callee, Attribute::SanitizeThread);
 }
 
+// Inline threshold computation
+
+// 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"));
+
+int InlineCostAnalysis::computeThresholdFromOptLevels(unsigned OptLevel,
+                                                      unsigned SizeOptLevel) {
+  if (OptLevel > 2)
+    return OptAggressiveThreshold;
+  if (SizeOptLevel == 1) // -Os
+    return OptSizeThreshold;
+  if (SizeOptLevel == 2) // -Oz
+    return OptMinSizeThreshold;
+  return DefaultInlineThreshold;
+}
+
+int InlineCostAnalysis::getDefaultInlineThreshold() {
+  return DefaultInlineThreshold;
+}
+
+// Returns the inline threshold for a given callsite whose callee is explicitly
+// specified. The caller of this method passes a default threshold value that
+// is either obtained from optimization level and options or explicitly
+// specified. This is modified based on properties of the callsite and returned.
+
+static int getInlineThreshold(CallSite CS, int DefaultThreshold,
+                              Function *Callee) {
+
+  int Threshold = DefaultThreshold;
+  // 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 (!(DefaultInlineThreshold.getNumOccurrences() > 0) && OptSize &&
+      OptSizeThreshold < Threshold)
+    Threshold = OptSizeThreshold;
+
+  if (!Callee || Callee->isDeclaration())
+    return Threshold;
+
+  // Listen to the inlinehint attribute when it would increase the threshold
+  // and the caller does not need to minimize its size.
+  bool InlineHint = Callee->hasFnAttribute(Attribute::InlineHint);
+  if (InlineHint && HintThreshold > Threshold && !Caller->optForMinSize())
+    Threshold = HintThreshold;
+
+  // Listen to the cold attribute when it would decrease the threshold.
+  bool ColdCallee = Callee->hasFnAttribute(Attribute::Cold);
+  // 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;
+
+  return Threshold;
+}
+
 InlineCost InlineCostAnalysis::getInlineCost(CallSite CS, Function *Callee,
-                                             int Threshold) {
+                                             int DefaultThreshold) {
+  int Threshold =
+      getInlineThreshold(CS, DefaultThreshold, CS.getCalledFunction());
+
   // 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))

lib/Transforms/IPO/InlineAlways.cpp

 
 namespace {
 
 /// \brief Inliner pass which only handles "always inline" functions.
 class AlwaysInliner : public Inliner {
   InlineCostAnalysis *ICA;
 
 public:
-  // Use extremely low threshold.
-  AlwaysInliner() : Inliner(ID, -2000000000, /*InsertLifetime*/ true),
-                    ICA(nullptr) {
+  AlwaysInliner() : Inliner(ID, /*InsertLifetime*/ true), ICA(nullptr) {
     initializeAlwaysInlinerPass(*PassRegistry::getPassRegistry());
   }
 
   AlwaysInliner(bool InsertLifetime)
-      : Inliner(ID, -2000000000, InsertLifetime), ICA(nullptr) {
+      : Inliner(ID, InsertLifetime), ICA(nullptr) {
     initializeAlwaysInlinerPass(*PassRegistry::getPassRegistry());
   }
 
   static char ID; // Pass identification, replacement for typeid
 
   InlineCost getInlineCost(CallSite CS) override;
 
   void getAnalysisUsage(AnalysisUsage &AU) const override;

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 {
   InlineCostAnalysis *ICA;
+  int DefaultThreshold;
 
 public:
-  SimpleInliner() : Inliner(ID), ICA(nullptr) {
+  SimpleInliner()
+      : Inliner(ID), ICA(nullptr),
+        DefaultThreshold(InlineCostAnalysis::getDefaultInlineThreshold()) {
     initializeSimpleInlinerPass(*PassRegistry::getPassRegistry());
   }
 
   SimpleInliner(int Threshold)
-      : Inliner(ID, Threshold, /*InsertLifetime*/ true), ICA(nullptr) {
+      : Inliner(ID), ICA(nullptr), DefaultThreshold(Threshold) {
     initializeSimpleInlinerPass(*PassRegistry::getPassRegistry());
   }
 
   static char ID; // Pass identification, replacement for typeid
 
   InlineCost getInlineCost(CallSite CS) override {
-    return ICA->getInlineCost(CS, getInlineThreshold(CS));
+    return ICA->getInlineCost(CS, DefaultThreshold);
   }
 
   bool runOnSCC(CallGraphSCC &SCC) override;
   void getAnalysisUsage(AnalysisUsage &AU) const override;
 };
 
-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(InlineCostAnalysis)
 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));
+  return new SimpleInliner(InlineCostAnalysis::computeThresholdFromOptLevels(
+      OptLevel, SizeOptLevel));
 }
 
 bool SimpleInliner::runOnSCC(CallGraphSCC &SCC) {
   ICA = &getAnalysis<InlineCostAnalysis>();
   return Inliner::runOnSCC(SCC);
 }
 
 void SimpleInliner::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.addRequired<InlineCostAnalysis>();
   Inliner::getAnalysisUsage(AU);
 }

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;
-
-  // Listen to the inlinehint attribute when it would increase the threshold
-  // and the caller does not need to minimize its size.
-  Function *Callee = CS.getCalledFunction();
-  bool InlineHint = Callee && !Callee->isDeclaration() &&
-                    Callee->hasFnAttribute(Attribute::InlineHint);
-  if (InlineHint && HintThreshold > Threshold &&
-      !Caller->hasFnAttribute(Attribute::MinSize))
-    Threshold = HintThreshold;
-
-  // Listen to the cold attribute when it would decrease the threshold.
-  bool ColdCallee = Callee && !Callee->isDeclaration() &&
-                    Callee->hasFnAttribute(Attribute::Cold);
-  // 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.