Initial commit for the rewrite of the inline cost analysis to operate

on a per-callsite walk of the called function's instructions, in
breadth-first order over the potentially reachable set of basic blocks.

This is a major shift in how inline cost analysis works to improve the
accuracy and rationality of inlining decisions. A brief outline of the
algorithm this moves to:

- Build a simplification mapping based on the callsite arguments to the
  function arguments.
- Push the entry block onto a worklist of potentially-live basic blocks.
- Pop the first block off of the *front* of the worklist (for
  breadth-first ordering) and walk its instructions using a custom
  InstVisitor.
- For each instruction's operands, re-map them based on the
  simplification mappings available for the given callsite.
- Compute any simplification possible of the instruction after
  re-mapping, and store that back int othe simplification mapping.
- Compute any bonuses, costs, or other impacts of the instruction on the
  cost metric.
- When the terminator is reached, replace any conditional value in the
  terminator with any simplifications from the mapping we have, and add
  any successors which are not proven to be dead from these
  simplifications to the worklist.
- Pop the next block off of the front of the worklist, and repeat.
- As soon as the cost of inlining exceeds the threshold for the
  callsite, stop analyzing the function in order to bound cost.

The primary goal of this algorithm is to perfectly handle dead code
paths. We do not want any code in trivially dead code paths to impact
inlining decisions. The previous metric was *extremely* flawed here, and
would always subtract the average cost of two successors of
a conditional branch when it was proven to become an unconditional
branch at the callsite. There was no handling of wildly different costs
between the two successors, which would cause inlining when the path
actually taken was too large, and no inlining when the path actually
taken was trivially simple. There was also no handling of the code
*path*, only the immediate successors. These problems vanish completely
now. See the added regression tests for the shiny new features -- we
skip recursive function calls, SROA-killing instructions, and high cost
complex CFG structures when dead at the callsite being analyzed.

Switching to this algorithm required refactoring the inline cost
interface to accept the actual threshold rather than simply returning
a single cost. The resulting interface is pretty bad, and I'm planning
to do lots of interface cleanup after this patch.

Several other refactorings fell out of this, but I've tried to minimize
them for this patch. =/ There is still more cleanup that can be done
here. Please point out anything that you see in review.

I've worked really hard to try to mirror at least the spirit of all of
the previous heuristics in the new model. It's not clear that they are
all correct any more, but I wanted to minimize the change in this single
patch, it's already a bit ridiculous. One heuristic that is *not* yet
mirrored is to allow inlining of functions with a dynamic alloca *if*
the caller has a dynamic alloca. I will add this back, but I think the
most reasonable way requires changes to the inliner itself rather than
just the cost metric, and so I've deferred this for a subsequent patch.
The test case is XFAIL-ed until then.

As mentioned in the review mail, this seems to make Clang run about 1%
to 2% faster in -O0, but makes its binary size grow by just under 4%.
I've looked into the 4% growth, and it can be fixed, but requires
changes to other parts of the inliner.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@153812 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/Analysis/CodeMetrics.h

@@ -20,9 +20,13 @@
 namespace llvm {
   class BasicBlock;
   class Function;
+  class Instruction;
   class TargetData;
   class Value;
 
+  /// \brief Check whether an instruction is likely to be "free" when lowered.
+  bool isInstructionFree(const Instruction *I, const TargetData *TD = 0);
+
   /// \brief Check whether a call will lower to something small.
   ///
   /// This tests checks whether calls to this function will lower to something

include/llvm/Analysis/InlineCost.h

@@ -16,6 +16,7 @@
 
 #include "llvm/Function.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/ValueMap.h"
 #include "llvm/Analysis/CodeMetrics.h"
 #include <cassert>
@@ -25,162 +26,105 @@
 namespace llvm {
 
   class CallSite;
-  template<class PtrType, unsigned SmallSize>
-  class SmallPtrSet;
   class TargetData;
 
   namespace InlineConstants {
     // Various magic constants used to adjust heuristics.
     const int InstrCost = 5;
-    const int IndirectCallBonus = -100;
+    const int IndirectCallThreshold = 100;
     const int CallPenalty = 25;
     const int LastCallToStaticBonus = -15000;
     const int ColdccPenalty = 2000;
     const int NoreturnPenalty = 10000;
   }
 
-  /// InlineCost - Represent the cost of inlining a function. This
-  /// supports special values for functions which should "always" or
-  /// "never" be inlined. Otherwise, the cost represents a unitless
-  /// amount; smaller values increase the likelihood of the function
-  /// being inlined.
+  /// \brief Represents the cost of inlining a function.
+  ///
+  /// This supports special values for functions which should "always" or
+  /// "never" be inlined. Otherwise, the cost represents a unitless amount;
+  /// smaller values increase the likelihood of the function being inlined.
+  ///
+  /// Objects of this type also provide the adjusted threshold for inlining
+  /// based on the information available for a particular callsite. They can be
+  /// directly tested to determine if inlining should occur given the cost and
+  /// threshold for this cost metric.
   class InlineCost {
-    enum Kind {
-      Value,
-      Always,
-      Never
+    enum CostKind {
+      CK_Variable,
+      CK_Always,
+      CK_Never
     };
 
-    // This is a do-it-yourself implementation of
-    //   int Cost : 30;
-    //   unsigned Type : 2;
-    // We used to use bitfields, but they were sometimes miscompiled (PR3822).
-    enum { TYPE_BITS = 2 };
-    enum { COST_BITS = unsigned(sizeof(unsigned)) * CHAR_BIT - TYPE_BITS };
-    unsigned TypedCost; // int Cost : COST_BITS; unsigned Type : TYPE_BITS;
+    const int      Cost : 30; // The inlining cost if neither always nor never.
+    const unsigned Kind : 2;  // The type of cost, one of CostKind above.
 
-    Kind getType() const {
-      return Kind(TypedCost >> COST_BITS);
-    }
+    /// \brief The adjusted threshold against which this cost should be tested.
+    const int Threshold;
 
-    int getCost() const {
-      // Sign-extend the bottom COST_BITS bits.
-      return (int(TypedCost << TYPE_BITS)) >> TYPE_BITS;
-    }
+    // Trivial constructor, interesting logic in the factory functions below.
+    InlineCost(int Cost, CostKind Kind, int Threshold)
+      : Cost(Cost), Kind(Kind), Threshold(Threshold) {}
 
-    InlineCost(int C, int T) {
-      TypedCost = (unsigned(C << TYPE_BITS) >> TYPE_BITS) | (T << COST_BITS);
-      assert(getCost() == C && "Cost exceeds InlineCost precision");
-    }
   public:
-    static InlineCost get(int Cost) { return InlineCost(Cost, Value); }
-    static InlineCost getAlways() { return InlineCost(0, Always); }
-    static InlineCost getNever() { return InlineCost(0, Never); }
+    static InlineCost get(int Cost, int Threshold) {
+      InlineCost Result(Cost, CK_Variable, Threshold);
+      assert(Result.Cost == Cost && "Cost exceeds InlineCost precision");
+      return Result;
+    }
+    static InlineCost getAlways() {
+      return InlineCost(0, CK_Always, 0);
+    }
+    static InlineCost getNever() {
+      return InlineCost(0, CK_Never, 0);
+    }
 
-    bool isVariable() const { return getType() == Value; }
-    bool isAlways() const { return getType() == Always; }
-    bool isNever() const { return getType() == Never; }
+    /// \brief Test whether the inline cost is low enough for inlining.
+    operator bool() const {
+      if (isAlways()) return true;
+      if (isNever()) return false;
+      return Cost < Threshold;
+    }
 
-    /// getValue() - Return a "variable" inline cost's amount. It is
+    bool isVariable() const { return Kind == CK_Variable; }
+    bool isAlways() const   { return Kind == CK_Always; }
+    bool isNever() const    { return Kind == CK_Never; }
+
+    /// getCost() - Return a "variable" inline cost's amount. It is
     /// an error to call this on an "always" or "never" InlineCost.
-    int getValue() const {
-      assert(getType() == Value && "Invalid access of InlineCost");
-      return getCost();
+    int getCost() const {
+      assert(Kind == CK_Variable && "Invalid access of InlineCost");
+      return Cost;
+    }
+
+    /// \brief Get the cost delta from the threshold for inlining.
+    /// 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();
     }
   };
 
   /// InlineCostAnalyzer - Cost analyzer used by inliner.
   class InlineCostAnalyzer {
-    struct ArgInfo {
-    public:
-      unsigned ConstantWeight;
-      unsigned AllocaWeight;
-
-      ArgInfo(unsigned CWeight, unsigned AWeight)
-        : ConstantWeight(CWeight), AllocaWeight(AWeight)
-          {}
-    };
-
-    struct FunctionInfo {
-      CodeMetrics Metrics;
-
-      /// ArgumentWeights - Each formal argument of the function is inspected to
-      /// see if it is used in any contexts where making it a constant or alloca
-      /// would reduce the code size.  If so, we add some value to the argument
-      /// entry here.
-      std::vector<ArgInfo> ArgumentWeights;
-
-      /// PointerArgPairWeights - Weights to use when giving an inline bonus to
-      /// a call site due to correlated pairs of pointers.
-      DenseMap<std::pair<unsigned, unsigned>, unsigned> PointerArgPairWeights;
-
-      /// countCodeReductionForConstant - Figure out an approximation for how
-      /// many instructions will be constant folded if the specified value is
-      /// constant.
-      unsigned countCodeReductionForConstant(const CodeMetrics &Metrics,
-                                             Value *V);
-
-      /// countCodeReductionForAlloca - Figure out an approximation of how much
-      /// smaller the function will be if it is inlined into a context where an
-      /// argument becomes an alloca.
-      unsigned countCodeReductionForAlloca(const CodeMetrics &Metrics,
-                                           Value *V);
-
-      /// countCodeReductionForPointerPair - Count the bonus to apply to an
-      /// inline call site where a pair of arguments are pointers and one
-      /// argument is a constant offset from the other. The idea is to
-      /// recognize a common C++ idiom where a begin and end iterator are
-      /// actually pointers, and many operations on the pair of them will be
-      /// constants if the function is called with arguments that have
-      /// a constant offset.
-      void countCodeReductionForPointerPair(
-          const CodeMetrics &Metrics,
-          DenseMap<Value *, unsigned> &PointerArgs,
-          Value *V, unsigned ArgIdx);
-
-      /// analyzeFunction - Add information about the specified function
-      /// to the current structure.
-      void analyzeFunction(Function *F, const TargetData *TD);
-
-      /// NeverInline - Returns true if the function should never be
-      /// inlined into any caller.
-      bool NeverInline();
-    };
-
-    // The Function* for a function can be changed (by ArgumentPromotion);
-    // the ValueMap will update itself when this happens.
-    ValueMap<const Function *, FunctionInfo> CachedFunctionInfo;
-
     // TargetData if available, or null.
     const TargetData *TD;
 
-    int CountBonusForConstant(Value *V, Constant *C = NULL);
-    int ConstantFunctionBonus(CallSite CS, Constant *C);
-    int getInlineSize(CallSite CS, Function *Callee);
-    int getInlineBonuses(CallSite CS, Function *Callee);
   public:
     InlineCostAnalyzer(): TD(0) {}
 
     void setTargetData(const TargetData *TData) { TD = TData; }
 
-    /// getInlineCost - The heuristic used to determine if we should inline the
-    /// function call or not.
+    /// \brief Get an InlineCost object representing the cost of inlining this
+    /// callsite.
     ///
-    InlineCost getInlineCost(CallSite CS);
-    /// getCalledFunction - The heuristic used to determine if we should inline
-    /// the function call or not.  The callee is explicitly specified, to allow
-    /// you to calculate the cost of inlining a function via a pointer.  The
-    /// result assumes that the inlined version will always be used.  You should
-    /// weight it yourself in cases where this callee will not always be called.
-    InlineCost getInlineCost(CallSite CS, Function *Callee);
-
-    /// getInlineFudgeFactor - Return a > 1.0 factor if the inliner should use a
-    /// higher threshold to determine if the function call should be inlined.
-    float getInlineFudgeFactor(CallSite CS);
+    /// 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
+    /// sufficiently low to warrant inlining.
+    InlineCost getInlineCost(CallSite CS, int Threshold);
 
     /// resetCachedFunctionInfo - erase any cached cost info for this function.
     void resetCachedCostInfo(Function* Caller) {
-      CachedFunctionInfo[Caller] = FunctionInfo();
     }
 
     /// growCachedCostInfo - update the cached cost info for Caller after Callee

include/llvm/Transforms/IPO/InlinerPass.h

@@ -65,11 +65,6 @@ struct Inliner : public CallGraphSCCPass {
   ///
   virtual InlineCost getInlineCost(CallSite CS) = 0;
 
-  // getInlineFudgeFactor - Return a > 1.0 factor if the inliner should use a
-  // higher threshold to determine if the function call should be inlined.
-  ///
-  virtual float getInlineFudgeFactor(CallSite CS) = 0;
-
   /// resetCachedCostInfo - erase any cached cost data from the derived class.
   /// If the derived class has no such data this can be empty.
   /// 

lib/Analysis/CodeMetrics.cpp

@@ -50,6 +50,52 @@ bool llvm::callIsSmall(const Function *F) {
   return false;
 }
 
+bool llvm::isInstructionFree(const Instruction *I, const TargetData *TD) {
+  if (isa<PHINode>(I))
+    return true;
+
+  // If a GEP has all constant indices, it will probably be folded with
+  // a load/store.
+  if (const GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(I))
+    return GEP->hasAllConstantIndices();
+
+  if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(I)) {
+    switch (II->getIntrinsicID()) {
+    default:
+      return false;
+    case Intrinsic::dbg_declare:
+    case Intrinsic::dbg_value:
+    case Intrinsic::invariant_start:
+    case Intrinsic::invariant_end:
+    case Intrinsic::lifetime_start:
+    case Intrinsic::lifetime_end:
+    case Intrinsic::objectsize:
+    case Intrinsic::ptr_annotation:
+    case Intrinsic::var_annotation:
+      // These intrinsics don't count as size.
+      return true;
+    }
+  }
+
+  if (const CastInst *CI = dyn_cast<CastInst>(I)) {
+    // Noop casts, including ptr <-> int,  don't count.
+    if (CI->isLosslessCast() || isa<IntToPtrInst>(CI) || isa<PtrToIntInst>(CI))
+      return true;
+    // trunc to a native type is free (assuming the target has compare and
+    // shift-right of the same width).
+    if (TD && isa<TruncInst>(CI) &&
+        TD->isLegalInteger(TD->getTypeSizeInBits(CI->getType())))
+      return true;
+    // Result of a cmp instruction is often extended (to be used by other
+    // cmp instructions, logical or return instructions). These are usually
+    // nop on most sane targets.
+    if (isa<CmpInst>(CI->getOperand(0)))
+      return true;
+  }
+
+  return false;
+}
+
 /// analyzeBasicBlock - Fill in the current structure with information gleaned
 /// from the specified block.
 void CodeMetrics::analyzeBasicBlock(const BasicBlock *BB,
@@ -58,27 +104,11 @@ void CodeMetrics::analyzeBasicBlock(const BasicBlock *BB,
   unsigned NumInstsBeforeThisBB = NumInsts;
   for (BasicBlock::const_iterator II = BB->begin(), E = BB->end();
        II != E; ++II) {
-    if (isa<PHINode>(II)) continue;           // PHI nodes don't count.
+    if (isInstructionFree(II, TD))
+      continue;
 
     // Special handling for calls.
     if (isa<CallInst>(II) || isa<InvokeInst>(II)) {
-      if (const IntrinsicInst *IntrinsicI = dyn_cast<IntrinsicInst>(II)) {
-        switch (IntrinsicI->getIntrinsicID()) {
-        default: break;
-        case Intrinsic::dbg_declare:
-        case Intrinsic::dbg_value:
-        case Intrinsic::invariant_start:
-        case Intrinsic::invariant_end:
-        case Intrinsic::lifetime_start:
-        case Intrinsic::lifetime_end:
-        case Intrinsic::objectsize:
-        case Intrinsic::ptr_annotation:
-        case Intrinsic::var_annotation:
-          // These intrinsics don't count as size.
-          continue;
-        }
-      }
-
       ImmutableCallSite CS(cast<Instruction>(II));
 
       if (const Function *F = CS.getCalledFunction()) {
@@ -115,28 +145,6 @@ void CodeMetrics::analyzeBasicBlock(const BasicBlock *BB,
     if (isa<ExtractElementInst>(II) || II->getType()->isVectorTy())
       ++NumVectorInsts;
 
-    if (const CastInst *CI = dyn_cast<CastInst>(II)) {
-      // Noop casts, including ptr <-> int,  don't count.
-      if (CI->isLosslessCast() || isa<IntToPtrInst>(CI) ||
-          isa<PtrToIntInst>(CI))
-        continue;
-      // trunc to a native type is free (assuming the target has compare and
-      // shift-right of the same width).
-      if (isa<TruncInst>(CI) && TD &&
-          TD->isLegalInteger(TD->getTypeSizeInBits(CI->getType())))
-        continue;
-      // Result of a cmp instruction is often extended (to be used by other
-      // cmp instructions, logical or return instructions). These are usually
-      // nop on most sane targets.
-      if (isa<CmpInst>(CI->getOperand(0)))
-        continue;
-    } else if (const GetElementPtrInst *GEPI = dyn_cast<GetElementPtrInst>(II)){
-      // If a GEP has all constant indices, it will probably be folded with
-      // a load/store.
-      if (GEPI->hasAllConstantIndices())
-        continue;
-    }
-
     ++NumInsts;
   }
 

lib/Transforms/IPO/InlineAlways.cpp

@@ -59,10 +59,7 @@ namespace {
       // We still have to check the inline cost in case there are reasons to
       // not inline which trump the always-inline attribute such as setjmp and
       // indirectbr.
-      return CA.getInlineCost(CS);
-    }
-    float getInlineFudgeFactor(CallSite CS) {
-      return CA.getInlineFudgeFactor(CS);
+      return CA.getInlineCost(CS, getInlineThreshold(CS));
     }
     void resetCachedCostInfo(Function *Caller) {
       CA.resetCachedCostInfo(Caller);

lib/Transforms/IPO/InlineSimple.cpp

@@ -40,10 +40,7 @@ namespace {
     }
     static char ID; // Pass identification, replacement for typeid
     InlineCost getInlineCost(CallSite CS) {
-      return CA.getInlineCost(CS);
-    }
-    float getInlineFudgeFactor(CallSite CS) {
-      return CA.getInlineFudgeFactor(CS);
+      return CA.getInlineCost(CS, getInlineThreshold(CS));
     }
     void resetCachedCostInfo(Function *Caller) {
       CA.resetCachedCostInfo(Caller);

lib/Transforms/IPO/Inliner.cpp

@@ -231,14 +231,10 @@ bool Inliner::shouldInline(CallSite CS) {
     return false;
   }
   
-  int Cost = IC.getValue();
   Function *Caller = CS.getCaller();
-  int CurrentThreshold = getInlineThreshold(CS);
-  float FudgeFactor = getInlineFudgeFactor(CS);
-  int AdjThreshold = (int)(CurrentThreshold * FudgeFactor);
-  if (Cost >= AdjThreshold) {
-    DEBUG(dbgs() << "    NOT Inlining: cost=" << Cost
-          << ", thres=" << AdjThreshold
+  if (!IC) {
+    DEBUG(dbgs() << "    NOT Inlining: cost=" << IC.getCost()
+          << ", thres=" << (IC.getCostDelta() + IC.getCost())
           << ", Call: " << *CS.getInstruction() << "\n");
     return false;
   }
@@ -255,10 +251,15 @@ bool Inliner::shouldInline(CallSite CS) {
   // are used. Thus we will always have the opportunity to make local inlining
   // decisions. Importantly the linkonce-ODR linkage covers inline functions
   // and templates in C++.
+  //
+  // FIXME: All of this logic should be sunk into getInlineCost. It relies on
+  // the internal implementation of the inline cost metrics rather than
+  // treating them as truly abstract units etc.
   if (Caller->hasLocalLinkage() ||
       Caller->getLinkage() == GlobalValue::LinkOnceODRLinkage) {
     int TotalSecondaryCost = 0;
-    bool outerCallsFound = false;
+    // The candidate cost to be imposed upon the current function.
+    int CandidateCost = IC.getCost() - (InlineConstants::CallPenalty + 1);
     // This bool tracks what happens if we do NOT inline C into B.
     bool callerWillBeRemoved = Caller->hasLocalLinkage();
     // This bool tracks what happens if we DO inline C into B.
@@ -276,26 +277,19 @@ bool Inliner::shouldInline(CallSite CS) {
       }
 
       InlineCost IC2 = getInlineCost(CS2);
-      if (IC2.isNever())
+      if (!IC2) {
         callerWillBeRemoved = false;
-      if (IC2.isAlways() || IC2.isNever())
+        continue;
+      }
+      if (IC2.isAlways())
         continue;
 
-      outerCallsFound = true;
-      int Cost2 = IC2.getValue();
-      int CurrentThreshold2 = getInlineThreshold(CS2);
-      float FudgeFactor2 = getInlineFudgeFactor(CS2);
-
-      if (Cost2 >= (int)(CurrentThreshold2 * FudgeFactor2))
-        callerWillBeRemoved = false;
-
-      // See if we have this case.  We subtract off the penalty
-      // for the call instruction, which we would be deleting.
-      if (Cost2 < (int)(CurrentThreshold2 * FudgeFactor2) &&
-          Cost2 + Cost - (InlineConstants::CallPenalty + 1) >= 
-                (int)(CurrentThreshold2 * FudgeFactor2)) {
+      // See if inlining or original callsite would erase the cost delta of
+      // this callsite. We subtract off the penalty for the call instruction,
+      // which we would be deleting.
+      if (IC2.getCostDelta() <= CandidateCost) {
         inliningPreventsSomeOuterInline = true;
-        TotalSecondaryCost += Cost2;
+        TotalSecondaryCost += IC2.getCost();
       }
     }
     // If all outer calls to Caller would get inlined, the cost for the last
@@ -305,17 +299,16 @@ bool Inliner::shouldInline(CallSite CS) {
     if (callerWillBeRemoved && Caller->use_begin() != Caller->use_end())
       TotalSecondaryCost += InlineConstants::LastCallToStaticBonus;
 
-    if (outerCallsFound && inliningPreventsSomeOuterInline &&
-        TotalSecondaryCost < Cost) {
-      DEBUG(dbgs() << "    NOT Inlining: " << *CS.getInstruction() << 
-           " Cost = " << Cost << 
+    if (inliningPreventsSomeOuterInline && TotalSecondaryCost < IC.getCost()) {
+      DEBUG(dbgs() << "    NOT Inlining: " << *CS.getInstruction() <<
+           " Cost = " << IC.getCost() <<
            ", outer Cost = " << TotalSecondaryCost << '\n');
       return false;
     }
   }
 
-  DEBUG(dbgs() << "    Inlining: cost=" << Cost
-        << ", thres=" << AdjThreshold
+  DEBUG(dbgs() << "    Inlining: cost=" << IC.getCost()
+        << ", thres=" << (IC.getCostDelta() + IC.getCost())
         << ", Call: " << *CS.getInstruction() << '\n');
   return true;
 }

test/Transforms/Inline/alloca-bonus.ll

@@ -1,5 +1,7 @@
 ; RUN: opt -inline < %s -S -o - -inline-threshold=8 | FileCheck %s
 
+target datalayout = "p:32:32"
+
 declare void @llvm.lifetime.start(i64 %size, i8* nocapture %ptr)
 
 @glbl = external global i32
@@ -15,8 +17,8 @@ define void @outer1() {
 define void @inner1(i32 *%ptr) {
   %A = load i32* %ptr
   store i32 0, i32* %ptr
-  %C = getelementptr i32* %ptr, i32 0
-  %D = getelementptr i32* %ptr, i32 1
+  %C = getelementptr inbounds i32* %ptr, i32 0
+  %D = getelementptr inbounds i32* %ptr, i32 1
   %E = bitcast i32* %ptr to i8*
   %F = select i1 false, i32* %ptr, i32* @glbl
   call void @llvm.lifetime.start(i64 0, i8* %E)
@@ -35,8 +37,8 @@ define void @outer2() {
 define void @inner2(i32 *%ptr) {
   %A = load i32* %ptr
   store i32 0, i32* %ptr
-  %C = getelementptr i32* %ptr, i32 0
-  %D = getelementptr i32* %ptr, i32 %A
+  %C = getelementptr inbounds i32* %ptr, i32 0
+  %D = getelementptr inbounds i32* %ptr, i32 %A
   %E = bitcast i32* %ptr to i8*
   %F = select i1 false, i32* %ptr, i32* @glbl
   call void @llvm.lifetime.start(i64 0, i8* %E)
@@ -93,7 +95,7 @@ define void @outer4(i32 %A) {
 ; %B poisons this call, scalar-repl can't handle that instruction. However, we
 ; still want to detect that the icmp and branch *can* be handled.
 define void @inner4(i32 *%ptr, i32 %A) {
-  %B = getelementptr i32* %ptr, i32 %A
+  %B = getelementptr inbounds i32* %ptr, i32 %A
   %C = icmp eq i32* %ptr, null
   br i1 %C, label %bb.true, label %bb.false
 bb.true:
@@ -122,3 +124,32 @@ bb.true:
 bb.false:
   ret void
 }
+
+define void @outer5() {
+; CHECK: @outer5
+; CHECK-NOT: call void @inner5
+  %ptr = alloca i32
+  call void @inner5(i1 false, i32* %ptr)
+  ret void
+}
+
+; %D poisons this call, scalar-repl can't handle that instruction. However, if
+; the flag is set appropriately, the poisoning instruction is inside of dead
+; code, and so shouldn't be counted.
+define void @inner5(i1 %flag, i32 *%ptr) {
+  %A = load i32* %ptr
+  store i32 0, i32* %ptr
+  %C = getelementptr inbounds i32* %ptr, i32 0
+  br i1 %flag, label %if.then, label %exit
+
+if.then:
+  %D = getelementptr inbounds i32* %ptr, i32 %A
+  %E = bitcast i32* %ptr to i8*
+  %F = select i1 false, i32* %ptr, i32* @glbl
+  call void @llvm.lifetime.start(i64 0, i8* %E)
+  ret void
+
+exit:
+  ret void
+}
+

test/Transforms/Inline/dynamic_alloca_test.ll

@@ -4,6 +4,11 @@
 ; already have dynamic allocas.
 
 ; RUN: opt < %s -inline -S | FileCheck %s
+;
+; FIXME: This test is xfailed because the inline cost rewrite disabled *all*
+; inlining of functions which contain a dynamic alloca. It should be re-enabled
+; once that functionality is restored.
+; XFAIL: *
 
 declare void @ext(i32*)
 

test/Transforms/Inline/inline_constprop.ll

@@ -1,4 +1,4 @@
-; RUN: opt < %s -inline -S | FileCheck %s
+; RUN: opt < %s -inline -inline-threshold=20 -S | FileCheck %s
 
 define internal i32 @callee1(i32 %A, i32 %B) {
   %C = sdiv i32 %A, %B
@@ -14,17 +14,18 @@ define i32 @caller1() {
 }
 
 define i32 @caller2() {
+; Check that we can constant-prop through instructions after inlining callee21
+; to get constants in the inlined callsite to callee22.
+; FIXME: Currently, the threshold is fixed at 20 because we don't perform
+; *recursive* cost analysis to realize that the nested call site will definitely
+; inline and be cheap. We should eventually do that and lower the threshold here
+; to 1.
+;
 ; CHECK: @caller2
 ; CHECK-NOT: call void @callee2
 ; CHECK: ret
 
-; We contrive to make this hard for *just* the inline pass to do in order to
-; simulate what can actually happen with large, complex functions getting
-; inlined.
-  %a = add i32 42, 0
-  %b = add i32 48, 0
-
-  %x = call i32 @callee21(i32 %a, i32 %b)
+  %x = call i32 @callee21(i32 42, i32 48)
   ret i32 %x
 }
 
@@ -41,49 +42,71 @@ define i32 @callee22(i32 %x) {
   br i1 %icmp, label %bb.true, label %bb.false
 bb.true:
   ; This block musn't be counted in the inline cost.
-  %ptr = call i8* @getptr()
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
-  load volatile i8* %ptr
+  %x1 = add i32 %x, 1
+  %x2 = add i32 %x1, 1
+  %x3 = add i32 %x2, 1
+  %x4 = add i32 %x3, 1
+  %x5 = add i32 %x4, 1
+  %x6 = add i32 %x5, 1
+  %x7 = add i32 %x6, 1
+  %x8 = add i32 %x7, 1
 
-  ret i32 %x
+  ret i32 %x8
 bb.false:
   ret i32 %x
 }
+
+define i32 @caller3() {
+; Check that even if the expensive path is hidden behind several basic blocks,
+; it doesn't count toward the inline cost when constant-prop proves those paths
+; dead.
+;
+; CHECK: @caller3
+; CHECK-NOT: call
+; CHECK: ret i32 6
+
+entry:
+  %x = call i32 @callee3(i32 42, i32 48)
+  ret i32 %x
+}
+
+define i32 @callee3(i32 %x, i32 %y) {
+  %sub = sub i32 %y, %x
+  %icmp = icmp ugt i32 %sub, 42
+  br i1 %icmp, label %bb.true, label %bb.false
+
+bb.true:
+  %icmp2 = icmp ult i32 %sub, 64
+  br i1 %icmp2, label %bb.true.true, label %bb.true.false
+
+bb.true.true:
+  ; This block musn't be counted in the inline cost.
+  %x1 = add i32 %x, 1
+  %x2 = add i32 %x1, 1
+  %x3 = add i32 %x2, 1
+  %x4 = add i32 %x3, 1
+  %x5 = add i32 %x4, 1
+  %x6 = add i32 %x5, 1
+  %x7 = add i32 %x6, 1
+  %x8 = add i32 %x7, 1
+  br label %bb.merge
+
+bb.true.false:
+  ; This block musn't be counted in the inline cost.
+  %y1 = add i32 %y, 1
+  %y2 = add i32 %y1, 1
+  %y3 = add i32 %y2, 1
+  %y4 = add i32 %y3, 1
+  %y5 = add i32 %y4, 1
+  %y6 = add i32 %y5, 1
+  %y7 = add i32 %y6, 1
+  %y8 = add i32 %y7, 1
+  br label %bb.merge
+
+bb.merge:
+  %result = phi i32 [ %x8, %bb.true.true ], [ %y8, %bb.true.false ]
+  ret i32 %result
+
+bb.false:
+  ret i32 %sub
+}

test/Transforms/Inline/noinline-recursive-fn.ll

@@ -71,3 +71,40 @@ entry:
   call void @f2(i32 123, i8* bitcast (void (i32, i8*, i8*)* @f1 to i8*), i8* bitcast (void (i32, i8*, i8*)* @f2 to i8*)) nounwind ssp
   ret void
 }
+
+
+; Check that a recursive function, when called with a constant that makes the
+; recursive path dead code can actually be inlined.
+define i32 @fib(i32 %i) {
+entry:
+  %is.zero = icmp eq i32 %i, 0
+  br i1 %is.zero, label %zero.then, label %zero.else
+
+zero.then:
+  ret i32 0
+
+zero.else:
+  %is.one = icmp eq i32 %i, 1
+  br i1 %is.one, label %one.then, label %one.else
+
+one.then:
+  ret i32 1
+
+one.else:
+  %i1 = sub i32 %i, 1
+  %f1 = call i32 @fib(i32 %i1)
+  %i2 = sub i32 %i, 2
+  %f2 = call i32 @fib(i32 %i2)
+  %f = add i32 %f1, %f2
+  ret i32 %f
+}
+
+define i32 @fib_caller() {
+; CHECK: @fib_caller
+; CHECK-NOT: call
+; CHECK: ret
+  %f1 = call i32 @fib(i32 0)
+  %f2 = call i32 @fib(i32 1)
+  %result = add i32 %f1, %f2
+  ret i32 %result
+}

test/Transforms/Inline/ptr-diff.ll

@@ -1,5 +1,7 @@
 ; RUN: opt -inline < %s -S -o - -inline-threshold=10 | FileCheck %s
 
+target datalayout = "p:32:32"
+
 define i32 @outer1() {
 ; CHECK: @outer1
 ; CHECK-NOT: call