[llvm] 9a03058 - [CSSPGO] Factor out common part for CSSPGO inline and AFDO inline
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 2 01:22:02 PST 2021
> On Feb 2, 2021, at 08:34, Wenlei He via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
>
> Author: Wenlei He
> Date: 2021-02-02T00:34:06-08:00
> New Revision: 9a03058d6322edb8abc803ba3e436cc62647d979
>
> URL: https://github.com/llvm/llvm-project/commit/9a03058d6322edb8abc803ba3e436cc62647d979
> DIFF: https://github.com/llvm/llvm-project/commit/9a03058d6322edb8abc803ba3e436cc62647d979.diff
>
> LOG: [CSSPGO] Factor out common part for CSSPGO inline and AFDO inline
>
> Refactoring SampleProfileLoader::inlineHotFunctions to use helpers from CSSPGO inlining and reduce similar code in the inlining loop, plus minor cleanup for AFDO path.
>
> Test Plan:
>
> Differential Revision: https://reviews.llvm.org/D95024
>
> Added:
>
>
> Modified:
> llvm/lib/Transforms/IPO/SampleProfile.cpp
> llvm/test/Transforms/SampleProfile/pseudo-probe-inline.ll
> llvm/test/Transforms/SampleProfile/remarks.ll
>
> Removed:
>
>
>
> ################################################################################
> diff --git a/llvm/lib/Transforms/IPO/SampleProfile.cpp b/llvm/lib/Transforms/IPO/SampleProfile.cpp
> index 665c4078f3ee..7865426dff16 100644
> --- a/llvm/lib/Transforms/IPO/SampleProfile.cpp
> +++ b/llvm/lib/Transforms/IPO/SampleProfile.cpp
> @@ -416,20 +416,18 @@ class SampleProfileLoader {
> findIndirectCallFunctionSamples(const Instruction &I, uint64_t &Sum) const;
> mutable DenseMap<const DILocation *, const FunctionSamples *> DILocation2SampleMap;
> const FunctionSamples *findFunctionSamples(const Instruction &I) const;
> - CallBase *tryPromoteIndirectCall(Function &F, StringRef CalleeName,
> - uint64_t &Sum, uint64_t Count, CallBase *I,
> - const char *&Reason);
> - bool inlineCallInstruction(CallBase &CB,
> - const FunctionSamples *CalleeSamples);
> + // Attempt to promote indirect call and also inline the promoted call
> + bool tryPromoteAndInlineCandidate(
> + Function &F, InlineCandidate &Candidate, uint64_t &Sum,
> + DenseSet<Instruction *> &PromotedInsns,
> + SmallVector<CallBase *, 8> *InlinedCallSites = nullptr);
> bool inlineHotFunctions(Function &F,
> DenseSet<GlobalValue::GUID> &InlinedGUIDs);
> - // Helper functions call-site prioritized BFS inliner
> - // Will change the main FDO inliner to be work list based directly in
> - // upstream, then merge this change with that and remove the duplication.
> InlineCost shouldInlineCandidate(InlineCandidate &Candidate);
> bool getInlineCandidate(InlineCandidate *NewCandidate, CallBase *CB);
> - bool tryInlineCandidate(InlineCandidate &Candidate,
> - SmallVector<CallBase *, 8> &InlinedCallSites);
> + bool
> + tryInlineCandidate(InlineCandidate &Candidate,
> + SmallVector<CallBase *, 8> *InlinedCallSites = nullptr);
> bool
> inlineHotFunctionsWithPriority(Function &F,
> DenseSet<GlobalValue::GUID> &InlinedGUIDs);
> @@ -1077,70 +1075,45 @@ SampleProfileLoader::findFunctionSamples(const Instruction &Inst) const {
> return it.first->second;
> }
>
> -CallBase *
> -SampleProfileLoader::tryPromoteIndirectCall(Function &F, StringRef CalleeName,
> - uint64_t &Sum, uint64_t Count,
> - CallBase *I, const char *&Reason) {
> - Reason = "Callee function not available";
> +/// Attempt to promote indirect call and also inline the promoted call.
> +///
> +/// \param F Caller function.
> +/// \param Candidate ICP and inline candidate.
> +/// \param Sum Sum of target counts for indirect call.
> +/// \param PromotedInsns Map to keep track of indirect call already processed.
> +/// \param Candidate ICP and inline candidate.
> +/// \param InlinedCallSite Output vector for new call sites exposed after
> +/// inlining.
> +bool SampleProfileLoader::tryPromoteAndInlineCandidate(
> + Function &F, InlineCandidate &Candidate, uint64_t &Sum,
> + DenseSet<Instruction *> &PromotedInsns,
> + SmallVector<CallBase *, 8> *InlinedCallSite) {
> + const char *Reason = "Callee function not available";
> // R->getValue() != &F is to prevent promoting a recursive call.
> // If it is a recursive call, we do not inline it as it could bloat
> // the code exponentially. There is way to better handle this, e.g.
> // clone the caller first, and inline the cloned caller if it is
> // recursive. As llvm does not inline recursive calls, we will
> // simply ignore it instead of handling it explicitly.
> - auto R = SymbolMap.find(CalleeName);
> + auto R = SymbolMap.find(Candidate.CalleeSamples->getFuncName());
> if (R != SymbolMap.end() && R->getValue() &&
> !R->getValue()->isDeclaration() && R->getValue()->getSubprogram() &&
> R->getValue()->hasFnAttribute("use-sample-profile") &&
> - R->getValue() != &F && isLegalToPromote(*I, R->getValue(), &Reason)) {
> + R->getValue() != &F &&
> + isLegalToPromote(*Candidate.CallInstr, R->getValue(), &Reason)) {
> auto *DI =
> - &pgo::promoteIndirectCall(*I, R->getValue(), Count, Sum, false, ORE);
> - Sum -= Count;
> - return DI;
> - }
> - return nullptr;
> -}
> -
> -bool SampleProfileLoader::inlineCallInstruction(
> - CallBase &CB, const FunctionSamples *CalleeSamples) {
> - if (ExternalInlineAdvisor) {
> - auto Advice = ExternalInlineAdvisor->getAdvice(CB);
> - if (!Advice->isInliningRecommended()) {
> - Advice->recordUnattemptedInlining();
> - return false;
> + &pgo::promoteIndirectCall(*Candidate.CallInstr, R->getValue(),
> + Candidate.CallsiteCount, Sum, false, ORE);
> + if (DI) {
> + Sum -= Candidate.CallsiteCount;
> + PromotedInsns.insert(Candidate.CallInstr);
> + Candidate.CallInstr = DI;
> + if (isa<CallInst>(DI) || isa<InvokeInst>(DI))
> + return tryInlineCandidate(Candidate, InlinedCallSite);
> }
> - // Dummy record, we don't use it for replay.
> - Advice->recordInlining();
> - }
> -
> - Function *CalledFunction = CB.getCalledFunction();
> - assert(CalledFunction);
> - DebugLoc DLoc = CB.getDebugLoc();
> - BasicBlock *BB = CB.getParent();
> - InlineParams Params = getInlineParams();
> - Params.ComputeFullInlineCost = true;
> - // Checks if there is anything in the reachable portion of the callee at
> - // this callsite that makes this inlining potentially illegal. Need to
> - // set ComputeFullInlineCost, otherwise getInlineCost may return early
> - // when cost exceeds threshold without checking all IRs in the callee.
> - // The acutal cost does not matter because we only checks isNever() to
> - // see if it is legal to inline the callsite.
> - InlineCost Cost =
> - getInlineCost(CB, Params, GetTTI(*CalledFunction), GetAC, GetTLI);
> - if (Cost.isNever()) {
> - ORE->emit(OptimizationRemarkAnalysis(CSINLINE_DEBUG, "InlineFail", DLoc, BB)
> - << "incompatible inlining");
> - return false;
> - }
> - InlineFunctionInfo IFI(nullptr, GetAC);
> - if (InlineFunction(CB, IFI).isSuccess()) {
> - // The call to InlineFunction erases I, so we can't pass it here.
> - emitInlinedInto(*ORE, DLoc, BB, *CalledFunction, *BB->getParent(), Cost,
> - true, CSINLINE_DEBUG);
> - if (ProfileIsCS)
> - ContextTracker->markContextSamplesInlined(CalleeSamples);
> - ++NumCSInlined;
> - return true;
> + } else {
> + LLVM_DEBUG(dbgs() << "\nFailed to promote indirect call to "
> + << CalleeFunctionName << " because " << Reason << "\n”)
It looks like this change may cause the following build failure with assertions enabled
llvm-project/llvm/lib/Transforms/IPO/SampleProfile.cpp:1116:26: error: use of undeclared identifier 'CalleeFunctionName'
<< CalleeFunctionName << " because " << Reason << "\n");
^
1 error generated.
More information about the llvm-commits
mailing list