From 91ffe00c5797daa27c9dac33d051b598a78f2440 Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Mon, 14 Aug 2017 21:25:00 +0000 Subject: [PATCH] [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 --- lib/Analysis/InlineCost.cpp | 124 +++++++++++++++--------------- test/Transforms/Inline/pr26698.ll | 4 +- 2 files changed, 64 insertions(+), 64 deletions(-) diff --git a/lib/Analysis/InlineCost.cpp b/lib/Analysis/InlineCost.cpp index ffb49fe30fc..6f5770b1cb2 100644 --- 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 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 12a40c100e9..6d5873ff1bc 100644 --- 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" -- 2.50.1