Fix a crash in the assumption cache when inlining indirect function calls

[dotdash]

Summary

Instances of the AssumptionCache are per function, so we can't re-use
the same AssumptionCache instance when recursing in the CallAnalyzer to
analyze a different function. Instead we have to pass the
AssumptionCacheTracker to the CallAnalyzer so it can get the right
AssumptionCache on demand.

[dotdash]

Not sure if this still qualifies for 3.6 (I guess it's not strictly a regression since llvm.assume is new?), but it'd be nice if it would as we hit this bug with rustc when using llvm.assume.

[hfinkel]

Seems right to me. Chandler, I think you touched this last, thoughts?

[chandlerc]

Yes, this is obviously correct. Thanks for the fix!

Also, yes, this should go into 3.6.

lib/Analysis/IPA/InlineCost.cpp

class CallAnalyzer : public InstVisitor<CallAnalyzer, bool> {
 
   // DataLayout if available, or null.
   const DataLayout *const DL;
 
   /// The TargetTransformInfo available for this compilation.
   const TargetTransformInfo &TTI;
 
   /// The cache of @llvm.assume intrinsics.
-  AssumptionCache &AC;
+  AssumptionCacheTracker *ACT;
 
   // The called function.
   Function &F;
 
   int Threshold;
   int Cost;
 
   bool IsCallerRecursive;
class CallAnalyzer : public InstVisitor<CallAnalyzer, bool> {
   bool visitBranchInst(BranchInst &BI);
   bool visitSwitchInst(SwitchInst &SI);
   bool visitIndirectBrInst(IndirectBrInst &IBI);
   bool visitResumeInst(ResumeInst &RI);
   bool visitUnreachableInst(UnreachableInst &I);
 
 public:
   CallAnalyzer(const DataLayout *DL, const TargetTransformInfo &TTI,
-               AssumptionCache &AC, Function &Callee, int Threshold)
-      : DL(DL), TTI(TTI), AC(AC), F(Callee), Threshold(Threshold), Cost(0),
+               AssumptionCacheTracker *ACT, Function &Callee, int Threshold)
+      : DL(DL), TTI(TTI), ACT(ACT), F(Callee), Threshold(Threshold), Cost(0),
         IsCallerRecursive(false), IsRecursiveCall(false),
         ExposesReturnsTwice(false), HasDynamicAlloca(false),
         ContainsNoDuplicateCall(false), HasReturn(false), HasIndirectBr(false),
         AllocatedSize(0), NumInstructions(0), NumVectorInstructions(0),
         FiftyPercentVectorBonus(0), TenPercentVectorBonus(0), VectorBonus(0),
         NumConstantArgs(0), NumConstantOffsetPtrArgs(0), NumAllocaArgs(0),
         NumConstantPtrCmps(0), NumConstantPtrDiffs(0),
         NumInstructionsSimplified(0), SROACostSavings(0),
bool CallAnalyzer::visitCallSite(CallSite CS) {
   if (!F)
     return Base::visitCallSite(CS);
 
   // If we have a constant that we are calling as a function, we can peer
   // through it and see the function target. This happens not infrequently
   // during devirtualization and so we want to give it a hefty bonus for
   // inlining, but cap that bonus in the event that inlining wouldn't pan
   // out. Pretend to inline the function, with a custom threshold.
-  CallAnalyzer CA(DL, TTI, AC, *F, InlineConstants::IndirectCallThreshold);
+  CallAnalyzer CA(DL, TTI, ACT, *F, InlineConstants::IndirectCallThreshold);
   if (CA.analyzeCall(CS)) {
     // We were able to inline the indirect call! Subtract the cost from the
     // bonus we want to apply, but don't go below zero.
     Cost -= std::max(0, InlineConstants::IndirectCallThreshold - CA.getCost());
   }
 
   return Base::visitCallSite(CS);
 }
bool CallAnalyzer::analyzeCall(CallSite CS) {
   NumConstantArgs = SimplifiedValues.size();
   NumConstantOffsetPtrArgs = ConstantOffsetPtrs.size();
   NumAllocaArgs = SROAArgValues.size();
 
   // FIXME: If a caller has multiple calls to a callee, we end up recomputing
   // the ephemeral values multiple times (and they're completely determined by
   // the callee, so this is purely duplicate work).
   SmallPtrSet<const Value *, 32> EphValues;
-  CodeMetrics::collectEphemeralValues(&F, &AC, EphValues);
+  CodeMetrics::collectEphemeralValues(&F, &ACT->getAssumptionCache(F), EphValues);
 
   // The worklist of live basic blocks in the callee *after* inlining. We avoid
   // adding basic blocks of the callee which can be proven to be dead for this
   // particular call site in order to get more accurate cost estimates. This
   // requires a somewhat heavyweight iteration pattern: we need to walk the
   // basic blocks in a breadth-first order as we insert live successors. To
   // accomplish this, prioritizing for small iterations because we exit after
   // crossing our threshold, we use a small-size optimized SetVector.
InlineCost InlineCostAnalysis::getInlineCost(CallSite CS, Function *Callee,
   if (Callee->mayBeOverridden() ||
       Callee->hasFnAttribute(Attribute::NoInline) || CS.isNoInline())
     return llvm::InlineCost::getNever();
 
   DEBUG(llvm::dbgs() << "      Analyzing call of " << Callee->getName()
         << "...\n");
 
   CallAnalyzer CA(Callee->getDataLayout(), TTIWP->getTTI(*Callee),
-                  ACT->getAssumptionCache(*Callee), *Callee, Threshold);
+                  ACT, *Callee, Threshold);
   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())

test/Transforms/Inline/inline-indirect.ll

+; RUN: opt < %s -inline -disable-output 2>/dev/null
+; This test used to trigger an assertion in the assumption cache when
+; inlining the indirect call
+declare void @llvm.assume(i1)
+
+define void @foo() {
+  ret void
+}
+
+define void @bar(void ()*) {
+  call void @llvm.assume(i1 true)
+  call void %0();
+  ret void
+}
+
+define void @baz() {
+  call void @bar(void ()* @foo)
+  ret void
+}