From f2f667dd6af5f2936175f567a3854cbba748a7a4 Mon Sep 17 00:00:00 2001 From: Sam Elliott Date: Mon, 21 Aug 2017 16:45:47 +0000 Subject: [PATCH] Emit only A Single Opt Remark When Inlining Summary: This updates the Inliner to only add a single Optimization Remark when Inlining, rather than an Analysis Remark and an Optimization Remark. Fixes https://bugs.llvm.org/show_bug.cgi?id=33786 Reviewers: anemet, davidxl, chandlerc Reviewed By: anemet Subscribers: haicheng, fhahn, mehdi_amini, dblaikie, llvm-commits, eraman Differential Revision: https://reviews.llvm.org/D36054 llvm-svn: 311349 --- include/llvm/Analysis/InlineCost.h | 6 ++ lib/Transforms/IPO/Inliner.cpp | 85 ++++++++++++------- ...diagnostic-handler-remarks-with-hotness.ll | 5 ++ .../X86/diagnostic-handler-remarks.ll | 5 ++ ...diagnostic-handler-remarks-with-hotness.ll | 5 ++ test/LTO/X86/diagnostic-handler-remarks.ll | 5 ++ ...diagnostic-handler-remarks-with-hotness.ll | 10 +++ .../ThinLTO/X86/diagnostic-handler-remarks.ll | 10 +++ .../optimization-remarks-passed-yaml.ll | 33 +++---- .../optimization-remarks-with-hotness.ll | 6 +- .../Inline/optimization-remarks-yaml.ll | 23 +++++ .../Transforms/Inline/optimization-remarks.ll | 12 ++- 12 files changed, 151 insertions(+), 54 deletions(-) diff --git a/include/llvm/Analysis/InlineCost.h b/include/llvm/Analysis/InlineCost.h index a4e13234a35..b76f0d920cc 100644 --- a/include/llvm/Analysis/InlineCost.h +++ b/include/llvm/Analysis/InlineCost.h @@ -105,6 +105,12 @@ public: return Cost; } + /// \brief Get the threshold against which the cost was computed + int getThreshold() const { + assert(isVariable() && "Invalid access of InlineCost"); + return Threshold; + } + /// \brief Get the cost delta from the threshold for inlining. /// Only valid if the cost is of the variable kind. Returns a negative /// value if the cost is too high to inline. diff --git a/lib/Transforms/IPO/Inliner.cpp b/lib/Transforms/IPO/Inliner.cpp index aca67d4472b..10a83f48ff4 100644 --- a/lib/Transforms/IPO/Inliner.cpp +++ b/lib/Transforms/IPO/Inliner.cpp @@ -335,10 +335,12 @@ shouldBeDeferred(Function *Caller, CallSite CS, InlineCost IC, return false; } -/// Return true if the inliner should attempt to inline at the given CallSite. -static bool shouldInline(CallSite CS, - function_ref GetInlineCost, - OptimizationRemarkEmitter &ORE) { +/// Return the cost only if the inliner should attempt to inline at the given +/// CallSite. If we return the cost, we will emit an optimisation remark later +/// using that cost, so we won't do so from this function. +static Optional +shouldInline(CallSite CS, function_ref GetInlineCost, + OptimizationRemarkEmitter &ORE) { using namespace ore; InlineCost IC = GetInlineCost(CS); Instruction *Call = CS.getInstruction(); @@ -348,10 +350,7 @@ static bool shouldInline(CallSite CS, if (IC.isAlways()) { DEBUG(dbgs() << " Inlining: cost=always" << ", Call: " << *CS.getInstruction() << "\n"); - ORE.emit(OptimizationRemarkAnalysis(DEBUG_TYPE, "AlwaysInline", Call) - << NV("Callee", Callee) - << " should always be inlined (cost=always)"); - return true; + return IC; } if (IC.isNever()) { @@ -361,19 +360,19 @@ static bool shouldInline(CallSite CS, << NV("Callee", Callee) << " not inlined into " << NV("Caller", Caller) << " because it should never be inlined (cost=never)"); - return false; + return None; } if (!IC) { DEBUG(dbgs() << " NOT Inlining: cost=" << IC.getCost() - << ", thres=" << (IC.getCostDelta() + IC.getCost()) + << ", thres=" << IC.getThreshold() << ", Call: " << *CS.getInstruction() << "\n"); ORE.emit(OptimizationRemarkMissed(DEBUG_TYPE, "TooCostly", Call) << NV("Callee", Callee) << " not inlined into " << NV("Caller", Caller) << " because too costly to inline (cost=" - << NV("Cost", IC.getCost()) << ", threshold=" - << NV("Threshold", IC.getCostDelta() + IC.getCost()) << ")"); - return false; + << NV("Cost", IC.getCost()) + << ", threshold=" << NV("Threshold", IC.getThreshold()) << ")"); + return None; } int TotalSecondaryCost = 0; @@ -386,18 +385,16 @@ static bool shouldInline(CallSite CS, << "Not inlining. Cost of inlining " << NV("Callee", Callee) << " increases the cost of inlining " << NV("Caller", Caller) << " in other contexts"); - return false; + + // IC does not bool() to false, so get an InlineCost that will. + // This will not be inspected to make an error message. + return None; } DEBUG(dbgs() << " Inlining: cost=" << IC.getCost() - << ", thres=" << (IC.getCostDelta() + IC.getCost()) + << ", thres=" << IC.getThreshold() << ", Call: " << *CS.getInstruction() << '\n'); - ORE.emit(OptimizationRemarkAnalysis(DEBUG_TYPE, "CanBeInlined", Call) - << NV("Callee", Callee) << " can be inlined into " - << NV("Caller", Caller) << " with cost=" << NV("Cost", IC.getCost()) - << " (threshold=" - << NV("Threshold", IC.getCostDelta() + IC.getCost()) << ")"); - return true; + return IC; } /// Return true if the specified inline history ID @@ -545,9 +542,10 @@ inlineCallsImpl(CallGraphSCC &SCC, CallGraph &CG, // just become a regular analysis dependency. OptimizationRemarkEmitter ORE(Caller); + Optional OIC = shouldInline(CS, GetInlineCost, ORE); // If the policy determines that we should inline this function, // delete the call instead. - if (!shouldInline(CS, GetInlineCost, ORE)) + if (!OIC) continue; // If this call site is dead and it is to a readonly function, we should @@ -562,7 +560,7 @@ inlineCallsImpl(CallGraphSCC &SCC, CallGraph &CG, ++NumCallsDeleted; } else { // Get DebugLoc to report. CS will be invalid after Inliner. - DebugLoc DLoc = Instr->getDebugLoc(); + DebugLoc DLoc = CS->getDebugLoc(); BasicBlock *Block = CS.getParent(); // Attempt to inline the function. @@ -578,10 +576,17 @@ inlineCallsImpl(CallGraphSCC &SCC, CallGraph &CG, } ++NumInlined; - // Report the inline decision. - ORE.emit(OptimizationRemark(DEBUG_TYPE, "Inlined", DLoc, Block) - << NV("Callee", Callee) << " inlined into " - << NV("Caller", Caller)); + if (OIC->isAlways()) + ORE.emit(OptimizationRemark(DEBUG_TYPE, "AlwaysInline", DLoc, Block) + << NV("Callee", Callee) << " inlined into " + << NV("Caller", Caller) << " with cost=always"); + else + ORE.emit(OptimizationRemark(DEBUG_TYPE, "Inlined", DLoc, Block) + << NV("Callee", Callee) << " inlined into " + << NV("Caller", Caller) + << " with cost=" << NV("Cost", OIC->getCost()) + << " (threshold=" << NV("Threshold", OIC->getThreshold()) + << ")"); // If inlining this function gave us any new call sites, throw them // onto our worklist to process. They are useful inline candidates. @@ -885,8 +890,9 @@ PreservedAnalyses InlinerPass::run(LazyCallGraph::SCC &InitialC, continue; } + Optional OIC = shouldInline(CS, GetInlineCost, ORE); // Check whether we want to inline this callsite. - if (!shouldInline(CS, GetInlineCost, ORE)) + if (!OIC) continue; // Setup the data structure used to plumb customization into the @@ -896,11 +902,32 @@ PreservedAnalyses InlinerPass::run(LazyCallGraph::SCC &InitialC, &FAM.getResult(*(CS.getCaller())), &FAM.getResult(Callee)); - if (!InlineFunction(CS, IFI)) + // Get DebugLoc to report. CS will be invalid after Inliner. + DebugLoc DLoc = CS->getDebugLoc(); + BasicBlock *Block = CS.getParent(); + + using namespace ore; + if (!InlineFunction(CS, IFI)) { + ORE.emit( + OptimizationRemarkMissed(DEBUG_TYPE, "NotInlined", DLoc, Block) + << NV("Callee", &Callee) << " will not be inlined into " + << NV("Caller", &F)); continue; + } DidInline = true; InlinedCallees.insert(&Callee); + if (OIC->isAlways()) + ORE.emit(OptimizationRemark(DEBUG_TYPE, "AlwaysInline", DLoc, Block) + << NV("Callee", &Callee) << " inlined into " + << NV("Caller", &F) << " with cost=always"); + else + ORE.emit( + OptimizationRemark(DEBUG_TYPE, "Inlined", DLoc, Block) + << NV("Callee", &Callee) << " inlined into " << NV("Caller", &F) + << " with cost=" << NV("Cost", OIC->getCost()) + << " (threshold=" << NV("Threshold", OIC->getThreshold()) << ")"); + // Add any new callsites to defined functions to the worklist. if (!IFI.InlinedCallSites.empty()) { int NewHistoryID = InlineHistory.size(); diff --git a/test/LTO/Resolution/X86/diagnostic-handler-remarks-with-hotness.ll b/test/LTO/Resolution/X86/diagnostic-handler-remarks-with-hotness.ll index 2469570c26b..2daba928e7e 100644 --- a/test/LTO/Resolution/X86/diagnostic-handler-remarks-with-hotness.ll +++ b/test/LTO/Resolution/X86/diagnostic-handler-remarks-with-hotness.ll @@ -17,6 +17,11 @@ ; YAML-NEXT: - Callee: tinkywinky ; YAML-NEXT: - String: ' inlined into ' ; YAML-NEXT: - Caller: main +; YAML-NEXT: - String: ' with cost=' +; YAML-NEXT: - Cost: '-15000' +; YAML-NEXT: - String: ' (threshold=' +; YAML-NEXT: - Threshold: '337' +; YAML-NEXT: - String: ')' ; YAML-NEXT: ... target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" diff --git a/test/LTO/Resolution/X86/diagnostic-handler-remarks.ll b/test/LTO/Resolution/X86/diagnostic-handler-remarks.ll index eb1bca3670c..c20cffa42c9 100644 --- a/test/LTO/Resolution/X86/diagnostic-handler-remarks.ll +++ b/test/LTO/Resolution/X86/diagnostic-handler-remarks.ll @@ -15,6 +15,11 @@ ; YAML-NEXT: - Callee: tinkywinky ; YAML-NEXT: - String: ' inlined into ' ; YAML-NEXT: - Caller: main +; YAML-NEXT: - String: ' with cost=' +; YAML-NEXT: - Cost: '-15000' +; YAML-NEXT: - String: ' (threshold=' +; YAML-NEXT: - Threshold: '337' +; YAML-NEXT: - String: ')' ; YAML-NEXT: ... target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" diff --git a/test/LTO/X86/diagnostic-handler-remarks-with-hotness.ll b/test/LTO/X86/diagnostic-handler-remarks-with-hotness.ll index 5d0a9b0a4e2..81116996ae2 100644 --- a/test/LTO/X86/diagnostic-handler-remarks-with-hotness.ll +++ b/test/LTO/X86/diagnostic-handler-remarks-with-hotness.ll @@ -17,6 +17,11 @@ ; YAML-NEXT: - Callee: foo ; YAML-NEXT: - String: ' inlined into ' ; YAML-NEXT: - Caller: main +; YAML-NEXT: - String: ' with cost=' +; YAML-NEXT: - Cost: '-15000' +; YAML-NEXT: - String: ' (threshold=' +; YAML-NEXT: - Threshold: '337' +; YAML-NEXT: - String: ')' ; YAML-NEXT: ... target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128" diff --git a/test/LTO/X86/diagnostic-handler-remarks.ll b/test/LTO/X86/diagnostic-handler-remarks.ll index 82627fd24ab..b6d02675bf1 100644 --- a/test/LTO/X86/diagnostic-handler-remarks.ll +++ b/test/LTO/X86/diagnostic-handler-remarks.ll @@ -53,6 +53,11 @@ ; YAML-NEXT: - Callee: foo ; YAML-NEXT: - String: ' inlined into ' ; YAML-NEXT: - Caller: main +; YAML-NEXT: - String: ' with cost=' +; YAML-NEXT: - Cost: '-15000' +; YAML-NEXT: - String: ' (threshold=' +; YAML-NEXT: - Threshold: '337' +; YAML-NEXT: - String: ')' ; YAML-NEXT: ... target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128" diff --git a/test/ThinLTO/X86/diagnostic-handler-remarks-with-hotness.ll b/test/ThinLTO/X86/diagnostic-handler-remarks-with-hotness.ll index e9dc584e756..0f0e26e53ca 100644 --- a/test/ThinLTO/X86/diagnostic-handler-remarks-with-hotness.ll +++ b/test/ThinLTO/X86/diagnostic-handler-remarks-with-hotness.ll @@ -25,6 +25,11 @@ ; YAML1-NEXT: - Callee: foo ; YAML1-NEXT: - String: ' inlined into ' ; YAML1-NEXT: - Caller: main +; YAML1-NEXT: - String: ' with cost=' +; YAML1-NEXT: - Cost: '-30' +; YAML1-NEXT: - String: ' (threshold=' +; YAML1-NEXT: - Threshold: '337' +; YAML1-NEXT: - String: ')' ; YAML1-NEXT: ... @@ -38,6 +43,11 @@ ; YAML2-NEXT: - Callee: bar ; YAML2-NEXT: - String: ' inlined into ' ; YAML2-NEXT: - Caller: foo +; YAML2-NEXT: - String: ' with cost=' +; YAML2-NEXT: - Cost: '-30' +; YAML2-NEXT: - String: ' (threshold=' +; YAML2-NEXT: - Threshold: '337' +; YAML2-NEXT: - String: ')' ; YAML2-NEXT: ... diff --git a/test/ThinLTO/X86/diagnostic-handler-remarks.ll b/test/ThinLTO/X86/diagnostic-handler-remarks.ll index 3880b6f1138..2fd2cf8102b 100644 --- a/test/ThinLTO/X86/diagnostic-handler-remarks.ll +++ b/test/ThinLTO/X86/diagnostic-handler-remarks.ll @@ -22,6 +22,11 @@ ; YAML1-NEXT: - Callee: foo ; YAML1-NEXT: - String: ' inlined into ' ; YAML1-NEXT: - Caller: main +; YAML1-NEXT: - String: ' with cost=' +; YAML1-NEXT: - Cost: '-30' +; YAML1-NEXT: - String: ' (threshold=' +; YAML1-NEXT: - Threshold: '337' +; YAML1-NEXT: - String: ')' ; YAML1-NEXT: ... @@ -35,6 +40,11 @@ ; YAML2-NEXT: - Callee: bar ; YAML2-NEXT: - String: ' inlined into ' ; YAML2-NEXT: - Caller: foo +; YAML2-NEXT: - String: ' with cost=' +; YAML2-NEXT: - Cost: '-30' +; YAML2-NEXT: - String: ' (threshold=' +; YAML2-NEXT: - Threshold: '337' +; YAML2-NEXT: - String: ')' ; YAML2-NEXT: ... diff --git a/test/Transforms/Inline/optimization-remarks-passed-yaml.ll b/test/Transforms/Inline/optimization-remarks-passed-yaml.ll index f532a164857..11325b3c623 100644 --- a/test/Transforms/Inline/optimization-remarks-passed-yaml.ll +++ b/test/Transforms/Inline/optimization-remarks-passed-yaml.ll @@ -3,6 +3,11 @@ ; RUN: -pass-remarks-with-hotness 2>&1 | FileCheck %s ; RUN: cat %t | FileCheck -check-prefix=YAML %s +; RUN: opt < %s -S -passes=inline -pass-remarks-output=%t -pass-remarks=inline \ +; RUN: -pass-remarks-missed=inline -pass-remarks-analysis=inline \ +; RUN: -pass-remarks-with-hotness 2>&1 | FileCheck %s +; RUN: cat %t | FileCheck -check-prefix=YAML %s + ; Check the YAML file for inliner-generated passed and analysis remarks. This ; is the input: @@ -12,28 +17,9 @@ ; 4 return foo(); ; 5 } -; CHECK: remark: /tmp/s.c:4:10: foo can be inlined into bar with cost={{[0-9\-]+}} (threshold={{[0-9]+}}) (hotness: 30) -; CHECK-NEXT: remark: /tmp/s.c:4:10: foo inlined into bar (hotness: 30) +; CHECK: remark: /tmp/s.c:4:10: foo inlined into bar with cost={{[0-9\-]+}} (threshold={{[0-9]+}}) (hotness: 30) -; YAML: --- !Analysis -; YAML-NEXT: Pass: inline -; YAML-NEXT: Name: CanBeInlined -; YAML-NEXT: DebugLoc: { File: /tmp/s.c, Line: 4, Column: 10 } -; YAML-NEXT: Function: bar -; YAML-NEXT: Hotness: 30 -; YAML-NEXT: Args: -; YAML-NEXT: - Callee: foo -; YAML-NEXT: DebugLoc: { File: /tmp/s.c, Line: 1, Column: 0 } -; YAML-NEXT: - String: ' can be inlined into ' -; YAML-NEXT: - Caller: bar -; YAML-NEXT: DebugLoc: { File: /tmp/s.c, Line: 3, Column: 0 } -; YAML-NEXT: - String: ' with cost=' -; YAML-NEXT: - Cost: '{{[0-9\-]+}}' -; YAML-NEXT: - String: ' (threshold=' -; YAML-NEXT: - Threshold: '{{[0-9]+}}' -; YAML-NEXT: - String: ')' -; YAML-NEXT: ... -; YAML-NEXT: --- !Passed +; YAML: --- !Passed ; YAML-NEXT: Pass: inline ; YAML-NEXT: Name: Inlined ; YAML-NEXT: DebugLoc: { File: /tmp/s.c, Line: 4, Column: 10 } @@ -45,6 +31,11 @@ ; YAML-NEXT: - String: ' inlined into ' ; YAML-NEXT: - Caller: bar ; YAML-NEXT: DebugLoc: { File: /tmp/s.c, Line: 3, Column: 0 } +; YAML-NEXT: - String: ' with cost=' +; YAML-NEXT: - Cost: '{{[0-9\-]+}}' +; YAML-NEXT: - String: ' (threshold=' +; YAML-NEXT: - Threshold: '{{[0-9]+}}' +; YAML-NEXT: - String: ')' ; YAML-NEXT: ... ; ModuleID = '/tmp/s.c' diff --git a/test/Transforms/Inline/optimization-remarks-with-hotness.ll b/test/Transforms/Inline/optimization-remarks-with-hotness.ll index 1d6d135bdda..3614c3b5230 100644 --- a/test/Transforms/Inline/optimization-remarks-with-hotness.ll +++ b/test/Transforms/Inline/optimization-remarks-with-hotness.ll @@ -1,9 +1,11 @@ ; RUN: opt < %s -inline -pass-remarks=inline -pass-remarks-missed=inline \ ; RUN: -pass-remarks-analysis=inline -pass-remarks-with-hotness -S 2>&1 \ ; RUN: | FileCheck %s +; RUN: opt < %s -passes=inline -pass-remarks=inline -pass-remarks-missed=inline \ +; RUN: -pass-remarks-analysis=inline -pass-remarks-with-hotness -S 2>&1 \ +; RUN: | FileCheck %s -; CHECK: foo should always be inlined (cost=always) (hotness: 30) -; CHECK: foo inlined into bar (hotness: 30) +; CHECK: foo inlined into bar with cost=always (hotness: 30) ; CHECK: foz not inlined into bar because it should never be inlined (cost=never) (hotness: 30) ; Function Attrs: alwaysinline nounwind uwtable diff --git a/test/Transforms/Inline/optimization-remarks-yaml.ll b/test/Transforms/Inline/optimization-remarks-yaml.ll index 16783634484..df7fecbc195 100644 --- a/test/Transforms/Inline/optimization-remarks-yaml.ll +++ b/test/Transforms/Inline/optimization-remarks-yaml.ll @@ -17,6 +17,26 @@ ; The remarks output file should be empty. ; RUN: test ! -s %t.threshold +; NewPM: +; RUN: opt < %s -S -passes=inline -pass-remarks-missed=inline \ +; RUN: -pass-remarks-with-hotness -pass-remarks-hotness-threshold 15 \ +; RUN: -pass-remarks-output=%t 2>&1 | FileCheck %s -check-prefix=CHECK_NEW +; RUN: test ! -s %t +; RUN: opt < %s -S -passes=inline -pass-remarks-with-hotness -pass-remarks-output=%t +; RUN: test ! -s %t +; +; Verify that remarks that don't meet the hotness threshold are not output. +; RUN: opt < %s -S -passes=inline -pass-remarks-missed=inline \ +; RUN: -pass-remarks-with-hotness -pass-remarks-hotness-threshold 100 \ +; RUN: -pass-remarks-output=%t.threshold 2>&1 | \ +; RUN: FileCheck -check-prefix=THRESHOLD %s +; RUN: test ! -s %t.threshold +; RUN: opt < %s -S -passes=inline \ +; RUN: -pass-remarks-with-hotness -pass-remarks-hotness-threshold 100 \ +; RUN: -pass-remarks-output=%t.threshold +; The remarks output file should be empty. +; RUN: test ! -s %t.threshold + ; Check the YAML file generated for inliner remarks for this program: ; ; 1 int foo(); @@ -59,6 +79,9 @@ ; No remarks should be output, since none meet the threshold. ; THRESHOLD-NOT: remark +; NewPM does not output this kind of "missed" remark. +; CHECK_NEW-NOT: remark + ; ModuleID = '/tmp/s.c' source_filename = "/tmp/s.c" target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128" diff --git a/test/Transforms/Inline/optimization-remarks.ll b/test/Transforms/Inline/optimization-remarks.ll index 61e270cff76..3f8332134e0 100644 --- a/test/Transforms/Inline/optimization-remarks.ll +++ b/test/Transforms/Inline/optimization-remarks.ll @@ -5,10 +5,18 @@ ; RUN: -pass-remarks-analysis=inline -pass-remarks-with-hotness -S 2>&1 | \ ; RUN: FileCheck -check-prefix=CHECK -check-prefix=HOTNESS %s +; RUN: opt < %s -passes=inline -pass-remarks=inline -pass-remarks-missed=inline \ +; RUN: -pass-remarks-analysis=inline -S 2>&1 | \ +; RUN: FileCheck -check-prefix=CHECK -check-prefix=NO_HOTNESS %s +; RUN: opt < %s -passes=inline -pass-remarks=inline -pass-remarks-missed=inline \ +; RUN: -pass-remarks-analysis=inline -pass-remarks-with-hotness -S 2>&1 | \ +; RUN: FileCheck -check-prefix=CHECK -check-prefix=HOTNESS_NEW %s + ; HOTNESS: fox will not be inlined into bar because its definition is unavailable ; NO_HOTNESS-NOT: fox will not be inlined into bar because its definition is unavailable -; CHECK: foo should always be inlined (cost=always) -; CHECK: foo inlined into bar +; NewPM's inliner does not emit the following remark: +; HOTNESS_NEW-NOT: fox will not be inlined into bar because its definition is unavailable +; CHECK: foo inlined into bar with cost=always ; CHECK: foz not inlined into bar because it should never be inlined (cost=never) ; Function Attrs: alwaysinline nounwind uwtable