[NFC][InlineCost] Simplify internal inlining cost interface

[mtrofin]

Summary

All the use cases of CallAnalyzer use the same call site parameter to
both construct the CallAnalyzer, and then pass to the analysis member.
This change removes this duplication.

[mtrofin]

There appear to be unrelated changes due to clang-format. I created https://reviews.llvm.org/D71646 to capture just those changes, and I plan to submit that first, then this one.

[Jim]

It looks like some of changes are the same as this patch https://reviews.llvm.org/D71646.
Please based on https://reviews.llvm.org/D71646 this patch to make change.

[mtrofin]

rebase & rename

[mtrofin]

removed unrelated formatting in comments

[davidxl]

There is comment like this:

/// The candidate callsite being analyzed. Please do not use this to do
/// analysis in the caller function; we want the inline cost query to be
/// easily cacheable. Instead, use the cover function paramHasAttr.
CallBase &CandidateCall;

Can you find out the history of this field?

On the other hand, if a different callsite is passed to analyzeCall other than CandidateCall, it will produce wrong results.

[mtrofin]

In D71645#1791582, @davidxl wrote:

There is comment like this:

/// The candidate callsite being analyzed. Please do not use this to do
/// analysis in the caller function; we want the inline cost query to be
/// easily cacheable. Instead, use the cover function paramHasAttr.
CallBase &CandidateCall;

Can you find out the history of this field?

It seems it was introduced here: http://reviews.llvm.org/D9129. analyzeCall was already taking a CallSite parameter.

On the other hand, if a different callsite is passed to analyzeCall other than CandidateCall, it will produce wrong results.

That's the thing: the API is local to this file, and there is no case when we pass a different parameter to analyzeCall, than the parameter used to construct the CallAnalyzer object.

[davidxl]

lgtm

llvm/lib/Analysis/InlineCost.cpp

CallAnalyzer(const TargetTransformInfo &TTI,
                bool BoostIndirect = true)
       : TTI(TTI), GetAssumptionCache(GetAssumptionCache), GetBFI(GetBFI),
         PSI(PSI), F(Callee), DL(F.getParent()->getDataLayout()), ORE(ORE),
         CandidateCall(Call), Params(Params), Threshold(Params.DefaultThreshold),
         ComputeFullInlineCost(OptComputeFullInlineCost ||
                               Params.ComputeFullInlineCost || ORE),
         BoostIndirectCalls(BoostIndirect), EnableLoadElimination(true) {}
 
-  InlineResult analyzeCall(CallBase &Call);
+  InlineResult analyze();
 
   int getThreshold() { return Threshold; }
   int getCost() { return Cost; }
 
   // Keep a bunch of stats about the cost savings found so we can print them
   // out when debugging.
   unsigned NumConstantArgs = 0;
   unsigned NumConstantOffsetPtrArgs = 0;
if (TTI.isLoweredToCall(F)) {
     // inlining, but cap that bonus in the event that inlining wouldn't pan out.
     // Pretend to inline the function, with a custom threshold.
     if (IsIndirectCall && BoostIndirectCalls) {
       auto IndirectCallParams = Params;
       IndirectCallParams.DefaultThreshold =
           InlineConstants::IndirectCallThreshold;
       CallAnalyzer CA(TTI, GetAssumptionCache, GetBFI, PSI, ORE, *F, Call,
                       IndirectCallParams, false);
-      if (CA.analyzeCall(Call)) {
+      if (CA.analyze()) {
         // We were able to inline the indirect call! Subtract the cost from the
         // threshold to get the bonus we want to apply, but don't go below zero.
         Cost -= std::max(0, CA.getThreshold() - CA.getCost());
       }
     } else
       // Otherwise simply add the cost for merely making the call.
       addCost(InlineConstants::CallPenalty);
   }

 
 /// Analyze a call site for potential inlining.
 ///
 /// Returns true if inlining this call is viable, and false if it is not
 /// viable. It computes the cost and adjusts the threshold based on numerous
 /// factors and heuristics. If this method returns false but the computed cost
 /// is below the computed threshold, then inlining was forcibly disabled by
 /// some artifact of the routine.
-InlineResult CallAnalyzer::analyzeCall(CallBase &Call) {
+InlineResult CallAnalyzer::analyze() {
   ++NumCallsAnalyzed;
 
   // Perform some tweaks to the cost and threshold based on the direct
   // callsite information.
 
   // We want to more aggressively inline vector-dense kernels, so up the
   // 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(Call, F);
+  updateThreshold(CandidateCall, F);
 
   // While Threshold depends on commandline options that can take negative
   // values, we want to enforce the invariant that the computed threshold and
   // bonuses are non-negative.
   assert(Threshold >= 0);
   assert(SingleBBBonus >= 0);
   assert(VectorBonus >= 0);
 
   // Speculatively apply all possible bonuses to Threshold. If cost exceeds
   // this Threshold any time, and cost cannot decrease, we can stop processing
   // the rest of the function body.
   Threshold += (SingleBBBonus + VectorBonus);
 
   // Give out bonuses for the callsite, as the instructions setting them up
   // will be gone after inlining.
-  addCost(-getCallsiteCost(Call, DL));
+  addCost(-getCallsiteCost(CandidateCall, DL));
 
   // If this function uses the coldcc calling convention, prefer not to inline
   // it.
   if (F.getCallingConv() == CallingConv::Cold)
     Cost += InlineConstants::ColdccPenalty;
 
   // Check if we're done. This can happen due to bonuses and penalties.
   if (Cost >= Threshold && !ComputeFullInlineCost)
     return "high cost";
 
   if (F.empty())
     return true;
 
-  Function *Caller = Call.getFunction();
+  Function *Caller = CandidateCall.getFunction();
   // Check if the caller function is recursive itself.
   for (User *U : Caller->users()) {
     CallBase *Call = dyn_cast<CallBase>(U);
     if (Call && Call->getFunction() == Caller) {
       IsCallerRecursive = true;
       break;
     }
   }
 
   // Populate our simplified values by mapping from function arguments to call
   // arguments with known important simplifications.
-  auto CAI = Call.arg_begin();
+  auto CAI = CandidateCall.arg_begin();
   for (Function::arg_iterator FAI = F.arg_begin(), FAE = F.arg_end();
        FAI != FAE; ++FAI, ++CAI) {
-    assert(CAI != Call.arg_end());
+    assert(CAI != CandidateCall.arg_end());
     if (Constant *C = dyn_cast<Constant>(CAI))
       SimplifiedValues[&*FAI] = C;
 
     Value *PtrArg = *CAI;
     if (ConstantInt *C = stripAndComputeInBoundsConstantOffsets(PtrArg)) {
       ConstantOffsetPtrs[&*FAI] = std::make_pair(PtrArg, C->getValue());
 
       // We can SROA any pointer arguments derived from alloca instructions.
for (unsigned Idx = 0; Idx != BBWorklist.size(); ++Idx) {
     // inlining.
     if (SingleBB && TI->getNumSuccessors() > 1) {
       // Take off the bonus we applied to the threshold.
       Threshold -= SingleBBBonus;
       SingleBB = false;
     }
   }
 
-  bool OnlyOneCallAndLocalLinkage =
-      F.hasLocalLinkage() && F.hasOneUse() && &F == Call.getCalledFunction();
+  bool OnlyOneCallAndLocalLinkage = F.hasLocalLinkage() && F.hasOneUse() &&
+                                    &F == CandidateCall.getCalledFunction();
   // If this is a noduplicate call, we can still inline as long as
   // inlining this would cause the removal of the caller (so the instruction
   // is not actually duplicated, just moved).
   if (!OnlyOneCallAndLocalLinkage && ContainsNoDuplicateCall)
     return "noduplicate";
 
   // Loops generally act a lot like calls in that they act like barriers to
   // movement, require a certain amount of setup, etc. So when optimising for
InlineCost llvm::getInlineCost(
   if (Call.isNoInline())
     return llvm::InlineCost::getNever("noinline call site attribute");
 
   LLVM_DEBUG(llvm::dbgs() << "      Analyzing call of " << Callee->getName()
                           << "... (caller:" << Caller->getName() << ")\n");
 
   CallAnalyzer CA(CalleeTTI, GetAssumptionCache, GetBFI, PSI, ORE, *Callee,
                   Call, Params);
-  InlineResult ShouldInline = CA.analyzeCall(Call);
+  InlineResult ShouldInline = CA.analyze();
 
   LLVM_DEBUG(CA.dump());
 
   // Check if there was a reason to force inlining or no inlining.
   if (!ShouldInline && CA.getCost() < CA.getThreshold())
     return InlineCost::getNever(ShouldInline.message);
   if (ShouldInline && CA.getCost() >= CA.getThreshold())
     return InlineCost::getAlways("empty function");
for (auto &II : *BI) {
       if (!ReturnsTwice && isa<CallInst>(Call) &&
           cast<CallInst>(Call)->canReturnTwice())
         return "exposes returns-twice attribute";
 
       if (Call->getCalledFunction())
         switch (Call->getCalledFunction()->getIntrinsicID()) {
         default:
           break;
           // Disallow inlining of @llvm.icall.branch.funnel because current
           // backend can't separate call targets from call arguments.
         case llvm::Intrinsic::icall_branch_funnel:
           return "disallowed inlining of @llvm.icall.branch.funnel";
           // Disallow inlining functions that call @llvm.localescape. Doing this
           // correctly would require major changes to the inliner.
         case llvm::Intrinsic::localescape:
           return "disallowed inlining of @llvm.localescape";
-        // Disallow inlining of functions that initialize VarArgs with va_start.
+          // Disallow inlining of functions that initialize VarArgs with
+          // va_start.
         case llvm::Intrinsic::vastart:
           return "contains VarArgs initialized with va_start";
         }
     }
   }
 
   return true;
 }