[InlineCost] Refactor the checks for different analyses to be a bit more

localized to the code that uses those analyses.

Technically, this can change behavior as we no longer require the
existence of the ProfileSummaryInfo analysis to use local profile
information via BFI. We didn't actually require the PSI to have an
interesting profile though, so this only really impacts the behavior in
non-default pass pipelines.

IMO, this makes it substantially less surprising how everything works --
before an analysis that wasn't actually used had to exist to trigger
*any* profile aware inlining. I think the new organization makes it more
obvious where various checks for profile signals happen.

Differential Revision: https://reviews.llvm.org/D36710

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

diff --git a/lib/Analysis/InlineCost.cpp b/lib/Analysis/InlineCost.cpp
index ffb49fe30fcf6e1aceb9d307eb5f5562be67b568..6f5770b1cb278afeda86609b668d5504c83f0b43 100644 (file)
--- a/lib/Analysis/InlineCost.cpp
+++ b/lib/Analysis/InlineCost.cpp
@@ -659,15 +659,17 @@ bool CallAnalyzer::allowSizeGrowth(CallSite CS) {
 bool CallAnalyzer::isColdCallSite(CallSite CS, BlockFrequencyInfo *CallerBFI) {
   // If global profile summary is available, then callsite's coldness is
   // determined based on that.
-  if (PSI->hasProfileSummary())
+  if (PSI && PSI->hasProfileSummary())
     return PSI->isColdCallSite(CS, CallerBFI);
+
+  // Otherwise we need BFI to be available.
   if (!CallerBFI)
     return false;
 
-  // In the absence of global profile summary, determine if the callsite is cold
-  // relative to caller's entry. We could potentially cache the computation of
-  // scaled entry frequency, but the added complexity is not worth it unless
-  // this scaling shows up high in the profiles.
+  // Determine if the callsite is cold relative to caller's entry. We could
+  // potentially cache the computation of scaled entry frequency, but the added
+  // complexity is not worth it unless this scaling shows up high in the
+  // profiles.
   const BranchProbability ColdProb(ColdCallSiteRelFreq, 100);
   auto CallSiteBB = CS.getInstruction()->getParent();
   auto CallSiteFreq = CallerBFI->getBlockFreq(CallSiteBB);
@@ -679,28 +681,28 @@ bool CallAnalyzer::isColdCallSite(CallSite CS, BlockFrequencyInfo *CallerBFI) {
 Optional<int>
 CallAnalyzer::getHotCallSiteThreshold(CallSite CS,
                                       BlockFrequencyInfo *CallerBFI) {
+
   // If global profile summary is available, then callsite's hotness is
   // determined based on that.
+  if (PSI && PSI->hasProfileSummary() && PSI->isHotCallSite(CS, CallerBFI))
+    return Params.HotCallSiteThreshold;
 
-  auto HotCallSiteThreshold = Params.HotCallSiteThreshold;
-  if (PSI->hasProfileSummary() && PSI->isHotCallSite(CS, CallerBFI))
-    return HotCallSiteThreshold;
-  if (!CallerBFI)
-    return None;
-
-  HotCallSiteThreshold = Params.LocallyHotCallSiteThreshold;
-  if (!HotCallSiteThreshold)
+  // Otherwise we need BFI to be available and to have a locally hot callsite
+  // threshold.
+  if (!CallerBFI || !Params.LocallyHotCallSiteThreshold)
     return None;
 
-  // In the absence of global profile summary, determine if the callsite is hot
-  // relative to caller's entry. We could potentially cache the computation of
-  // scaled entry frequency, but the added complexity is not worth it unless
-  // this scaling shows up high in the profiles.
+  // Determine if the callsite is hot relative to caller's entry. We could
+  // potentially cache the computation of scaled entry frequency, but the added
+  // complexity is not worth it unless this scaling shows up high in the
+  // profiles.
   auto CallSiteBB = CS.getInstruction()->getParent();
   auto CallSiteFreq = CallerBFI->getBlockFreq(CallSiteBB).getFrequency();
   auto CallerEntryFreq = CallerBFI->getEntryFreq();
   if (CallSiteFreq >= CallerEntryFreq * HotCallSiteRelFreq)
-    return HotCallSiteThreshold;
+    return Params.LocallyHotCallSiteThreshold;
+
+  // Otherwise treat it normally.
   return None;
 }
 
@@ -773,50 +775,48 @@ void CallAnalyzer::updateThreshold(CallSite CS, Function &Callee) {
   if (!Caller->optForMinSize()) {
     if (Callee.hasFnAttribute(Attribute::InlineHint))
       Threshold = MaxIfValid(Threshold, Params.HintThreshold);
-    if (PSI) {
-      BlockFrequencyInfo *CallerBFI = GetBFI ? &((*GetBFI)(*Caller)) : nullptr;
-      // FIXME: After switching to the new passmanager, simplify the logic below
-      // by checking only the callsite hotness/coldness. The check for CallerBFI
-      // exists only because we do not have BFI available with the old PM.
-      //
-      // Use callee's hotness information only if we have no way of determining
-      // callsite's hotness information. Callsite hotness can be determined if
-      // sample profile is used (which adds hotness metadata to calls) or if
-      // caller's BlockFrequencyInfo is available.
-      if (CallerBFI || PSI->hasSampleProfile()) {
-        auto HotCallSiteThreshold = getHotCallSiteThreshold(CS, CallerBFI);
-        if (!Caller->optForSize() && HotCallSiteThreshold) {
-          DEBUG(dbgs() << "Hot callsite.\n");
-          // FIXME: This should update the threshold only if it exceeds the
-          // current threshold, but AutoFDO + ThinLTO currently relies on this
-          // behavior to prevent inlining of hot callsites during ThinLTO
-          // compile phase.
-          Threshold = HotCallSiteThreshold.getValue();
-        } else if (isColdCallSite(CS, CallerBFI)) {
-          DEBUG(dbgs() << "Cold callsite.\n");
-          // Do not apply bonuses for a cold callsite including the
-          // LastCallToStatic bonus. While this bonus might result in code size
-          // reduction, it can cause the size of a non-cold caller to increase
-          // preventing it from being inlined.
-          DisallowAllBonuses();
-          Threshold = MinIfValid(Threshold, Params.ColdCallSiteThreshold);
-        }
-      } else {
-        if (PSI->isFunctionEntryHot(&Callee)) {
-          DEBUG(dbgs() << "Hot callee.\n");
-          // If callsite hotness can not be determined, we may still know
-          // that the callee is hot and treat it as a weaker hint for threshold
-          // increase.
-          Threshold = MaxIfValid(Threshold, Params.HintThreshold);
-        } else if (PSI->isFunctionEntryCold(&Callee)) {
-          DEBUG(dbgs() << "Cold callee.\n");
-          // Do not apply bonuses for a cold callee including the
-          // LastCallToStatic bonus. While this bonus might result in code size
-          // reduction, it can cause the size of a non-cold caller to increase
-          // preventing it from being inlined.
-          DisallowAllBonuses();
-          Threshold = MinIfValid(Threshold, Params.ColdThreshold);
-        }
+
+    // FIXME: After switching to the new passmanager, simplify the logic below
+    // by checking only the callsite hotness/coldness as we will reliably
+    // have local profile information.
+    //
+    // Callsite hotness and coldness can be determined if sample profile is
+    // used (which adds hotness metadata to calls) or if caller's
+    // BlockFrequencyInfo is available.
+    BlockFrequencyInfo *CallerBFI = GetBFI ? &((*GetBFI)(*Caller)) : nullptr;
+    auto HotCallSiteThreshold = getHotCallSiteThreshold(CS, CallerBFI);
+    if (!Caller->optForSize() && HotCallSiteThreshold) {
+      DEBUG(dbgs() << "Hot callsite.\n");
+      // FIXME: This should update the threshold only if it exceeds the
+      // current threshold, but AutoFDO + ThinLTO currently relies on this
+      // behavior to prevent inlining of hot callsites during ThinLTO
+      // compile phase.
+      Threshold = HotCallSiteThreshold.getValue();
+    } else if (isColdCallSite(CS, CallerBFI)) {
+      DEBUG(dbgs() << "Cold callsite.\n");
+      // Do not apply bonuses for a cold callsite including the
+      // LastCallToStatic bonus. While this bonus might result in code size
+      // reduction, it can cause the size of a non-cold caller to increase
+      // preventing it from being inlined.
+      DisallowAllBonuses();
+      Threshold = MinIfValid(Threshold, Params.ColdCallSiteThreshold);
+    } else if (PSI) {
+      // Use callee's global profile information only if we have no way of
+      // determining this via callsite information.
+      if (PSI->isFunctionEntryHot(&Callee)) {
+        DEBUG(dbgs() << "Hot callee.\n");
+        // If callsite hotness can not be determined, we may still know
+        // that the callee is hot and treat it as a weaker hint for threshold
+        // increase.
+        Threshold = MaxIfValid(Threshold, Params.HintThreshold);
+      } else if (PSI->isFunctionEntryCold(&Callee)) {
+        DEBUG(dbgs() << "Cold callee.\n");
+        // Do not apply bonuses for a cold callee including the
+        // LastCallToStatic bonus. While this bonus might result in code size
+        // reduction, it can cause the size of a non-cold caller to increase
+        // preventing it from being inlined.
+        DisallowAllBonuses();
+        Threshold = MinIfValid(Threshold, Params.ColdThreshold);
       }
     }
   }

diff --git a/test/Transforms/Inline/pr26698.ll b/test/Transforms/Inline/pr26698.ll
index 12a40c100e91bd23565f45c7df2a0fafef3428ea..6d5873ff1bcb3250ea3e8dea85601f7779f145d9 100644 (file)
--- a/test/Transforms/Inline/pr26698.ll
+++ b/test/Transforms/Inline/pr26698.ll
@@ -1,5 +1,5 @@
-; RUN: opt -S -inline < %s | FileCheck %s
-; RUN: opt -S -passes='cgscc(inline)' < %s | FileCheck %s
+; RUN: opt -S -inline -inline-threshold=100 -inline-cold-callsite-threshold=100 < %s | FileCheck %s
+; RUN: opt -S -passes='cgscc(inline)' -inline-threshold=100 -inline-cold-callsite-threshold=100 < %s | FileCheck %s
 target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
 target triple = "i686-pc-windows-msvc18.0.0"