From f524386039f3808e5689a3da79e57fe6d0d12aa6 Mon Sep 17 00:00:00 2001 From: Eli Friedman Date: Fri, 11 Aug 2017 21:12:04 +0000 Subject: [PATCH] [OptDiag] Updating Remarks in SampleProfile Updating remark API to newer OptimizationDiagnosticInfo API. This allows remarks to show up in diagnostic yaml file, and enables use of opt-viewer tool. Hotness information for remarks (L505 and L751) do not display hotness information, most likely due to profile information not being propagated yet. Unsure if this is the desired outcome. Patch by Tarun Rajendran. Differential Revision: https://reviews.llvm.org/D36127 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@310763 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/IR/DiagnosticInfo.h | 1 + include/llvm/Transforms/Instrumentation.h | 2 +- lib/IR/DiagnosticInfo.cpp | 10 +++ lib/Transforms/IPO/SampleProfile.cpp | 75 +++++++++++-------- .../SampleProfile/cov-zero-samples.ll | 4 +- .../SampleProfile/inline-coverage.ll | 4 +- test/Transforms/SampleProfile/remarks.ll | 42 ++++++++++- 7 files changed, 101 insertions(+), 37 deletions(-) diff --git a/include/llvm/IR/DiagnosticInfo.h b/include/llvm/IR/DiagnosticInfo.h index 2a38071be5e..024b81a91cf 100644 --- a/include/llvm/IR/DiagnosticInfo.h +++ b/include/llvm/IR/DiagnosticInfo.h @@ -425,6 +425,7 @@ public: Argument(StringRef Key, unsigned N); Argument(StringRef Key, uint64_t N); Argument(StringRef Key, bool B) : Key(Key), Val(B ? "true" : "false") {} + Argument(StringRef Key, DebugLoc dl); }; /// \p PassName is the name of the pass emitting this diagnostic. \p diff --git a/include/llvm/Transforms/Instrumentation.h b/include/llvm/Transforms/Instrumentation.h index 71aed33db72..41f209ebae9 100644 --- a/include/llvm/Transforms/Instrumentation.h +++ b/include/llvm/Transforms/Instrumentation.h @@ -111,7 +111,7 @@ bool isLegalToPromote(Instruction *Inst, Function *F, const char **Reason); Instruction *promoteIndirectCall(Instruction *Inst, Function *F, uint64_t Count, uint64_t TotalCount, bool AttachProfToDirectCall, - OptimizationRemarkEmitter *ORE = nullptr); + OptimizationRemarkEmitter *ORE); /// Options for the frontend instrumentation based profiling pass. struct InstrProfOptions { diff --git a/lib/IR/DiagnosticInfo.cpp b/lib/IR/DiagnosticInfo.cpp index eeb6ce3dd50..99f6a33dc40 100644 --- a/lib/IR/DiagnosticInfo.cpp +++ b/lib/IR/DiagnosticInfo.cpp @@ -233,6 +233,16 @@ DiagnosticInfoOptimizationBase::Argument::Argument(StringRef Key, unsigned N) DiagnosticInfoOptimizationBase::Argument::Argument(StringRef Key, uint64_t N) : Key(Key), Val(utostr(N)) {} +DiagnosticInfoOptimizationBase::Argument::Argument(StringRef Key, DebugLoc Loc) + : Key(Key), Loc(Loc) { + if (Loc) { + Val = (Loc->getFilename() + ":" + Twine(Loc.getLine()) + ":" + + Twine(Loc.getCol())).str(); + } else { + Val = ""; + } +} + void DiagnosticInfoOptimizationBase::print(DiagnosticPrinter &DP) const { DP << getLocationStr() << ": " << getMsg(); if (Hotness) diff --git a/lib/Transforms/IPO/SampleProfile.cpp b/lib/Transforms/IPO/SampleProfile.cpp index c8d07e63fa2..246680f04c4 100644 --- a/lib/Transforms/IPO/SampleProfile.cpp +++ b/lib/Transforms/IPO/SampleProfile.cpp @@ -29,6 +29,7 @@ #include "llvm/ADT/StringRef.h" #include "llvm/Analysis/AssumptionCache.h" #include "llvm/Analysis/LoopInfo.h" +#include "llvm/Analysis/OptimizationDiagnosticInfo.h" #include "llvm/Analysis/PostDominators.h" #include "llvm/IR/Constants.h" #include "llvm/IR/DebugInfo.h" @@ -148,16 +149,16 @@ public: SampleProfileLoader(StringRef Name = SampleProfileFile) : DT(nullptr), PDT(nullptr), LI(nullptr), ACT(nullptr), Reader(), Samples(nullptr), Filename(Name), ProfileIsValid(false), - TotalCollectedSamples(0) {} + TotalCollectedSamples(0), ORE(nullptr) {} bool doInitialization(Module &M); - bool runOnModule(Module &M); + bool runOnModule(Module &M, ModuleAnalysisManager *AM); void setACT(AssumptionCacheTracker *A) { ACT = A; } void dump() { Reader->dump(); } protected: - bool runOnFunction(Function &F); + bool runOnFunction(Function &F, ModuleAnalysisManager *AM); unsigned getFunctionLoc(Function &F); bool emitAnnotations(Function &F); ErrorOr getInstWeight(const Instruction &I); @@ -249,6 +250,9 @@ protected: /// This is the sum of all the samples collected in all the functions executed /// at runtime. uint64_t TotalCollectedSamples; + + /// \brief Optimization Remark Emitter used to emit diagnostic remarks. + OptimizationRemarkEmitter *ORE; }; class SampleProfileLoaderLegacyPass : public ModulePass { @@ -497,13 +501,17 @@ ErrorOr SampleProfileLoader::getInstWeight(const Instruction &Inst) { bool FirstMark = CoverageTracker.markSamplesUsed(FS, LineOffset, Discriminator, R.get()); if (FirstMark) { - const Function *F = Inst.getParent()->getParent(); - LLVMContext &Ctx = F->getContext(); - emitOptimizationRemark( - Ctx, DEBUG_TYPE, *F, DLoc, - Twine("Applied ") + Twine(*R) + - " samples from profile (offset: " + Twine(LineOffset) + - ((Discriminator) ? Twine(".") + Twine(Discriminator) : "") + ")"); + if (Discriminator) + ORE->emit(OptimizationRemarkAnalysis(DEBUG_TYPE, "AppliedSamples", &Inst) + << "Applied " << ore::NV("NumSamples", *R) + << " samples from profile (offset: " + << ore::NV("LineOffset", LineOffset) << "." + << ore::NV("Discriminator", Discriminator) << ")"); + else + ORE->emit(OptimizationRemarkAnalysis(DEBUG_TYPE, "AppliedSamples", &Inst) + << "Applied " << ore::NV("NumSamples", *R) + << " samples from profile (offset: " + << ore::NV("LineOffset", LineOffset) << ")"); } DEBUG(dbgs() << " " << DLoc.getLine() << "." << DIL->getBaseDiscriminator() << ":" << Inst @@ -669,7 +677,6 @@ bool SampleProfileLoader::inlineHotFunctions( Function &F, DenseSet &ImportGUIDs) { DenseSet PromotedInsns; bool Changed = false; - LLVMContext &Ctx = F.getContext(); std::function GetAssumptionCache = [&]( Function &F) -> AssumptionCache & { return ACT->getAssumptionCache(F); }; while (true) { @@ -720,7 +727,7 @@ bool SampleProfileLoader::inlineHotFunctions( // We set the probability to 80% taken to indicate that the static // call is likely taken. DI = dyn_cast( - promoteIndirectCall(I, CalledFunction, 80, 100, false) + promoteIndirectCall(I, CalledFunction, 80, 100, false, ORE) ->stripPointerCasts()); PromotedInsns.insert(I); } else { @@ -737,12 +744,14 @@ bool SampleProfileLoader::inlineHotFunctions( continue; } DebugLoc DLoc = I->getDebugLoc(); + BasicBlock *BB = I->getParent(); if (InlineFunction(CallSite(DI), IFI)) { LocalChanged = true; - emitOptimizationRemark(Ctx, DEBUG_TYPE, F, DLoc, - Twine("inlined hot callee '") + - CalledFunction->getName() + "' into '" + - F.getName() + "'"); + // The call to InlineFunction erases DI, so we can't pass it here. + ORE->emit(OptimizationRemark(DEBUG_TYPE, "HotInline", DLoc, BB) + << "inlined hot callee '" + << ore::NV("Callee", CalledFunction) << "' into '" + << ore::NV("Caller", &F) << "'"); } } if (LocalChanged) { @@ -1213,7 +1222,7 @@ void SampleProfileLoader::propagateWeights(Function &F) { << ".\n"); SmallVector Weights; uint32_t MaxWeight = 0; - DebugLoc MaxDestLoc; + Instruction *MaxDestInst; for (unsigned I = 0; I < TI->getNumSuccessors(); ++I) { BasicBlock *Succ = TI->getSuccessor(I); Edge E = std::make_pair(BB, Succ); @@ -1232,7 +1241,7 @@ void SampleProfileLoader::propagateWeights(Function &F) { if (Weight != 0) { if (Weight > MaxWeight) { MaxWeight = Weight; - MaxDestLoc = Succ->getFirstNonPHIOrDbgOrLifetime()->getDebugLoc(); + MaxDestInst = Succ->getFirstNonPHIOrDbgOrLifetime(); } } } @@ -1247,13 +1256,9 @@ void SampleProfileLoader::propagateWeights(Function &F) { DEBUG(dbgs() << "SUCCESS. Found non-zero weights.\n"); TI->setMetadata(llvm::LLVMContext::MD_prof, MDB.createBranchWeights(Weights)); - emitOptimizationRemark( - Ctx, DEBUG_TYPE, F, MaxDestLoc, - Twine("most popular destination for conditional branches at ") + - ((BranchLoc) ? Twine(BranchLoc->getFilename() + ":" + - Twine(BranchLoc.getLine()) + ":" + - Twine(BranchLoc.getCol())) - : Twine(""))); + ORE->emit(OptimizationRemark(DEBUG_TYPE, "PopularDest", MaxDestInst) + << "most popular destination for conditional branches at " + << ore::NV("CondBranchesLoc", BranchLoc)); } else { DEBUG(dbgs() << "SKIPPED. All branch weights are zero.\n"); } @@ -1433,7 +1438,7 @@ ModulePass *llvm::createSampleProfileLoaderPass(StringRef Name) { return new SampleProfileLoaderLegacyPass(Name); } -bool SampleProfileLoader::runOnModule(Module &M) { +bool SampleProfileLoader::runOnModule(Module &M, ModuleAnalysisManager *AM) { if (!ProfileIsValid) return false; @@ -1465,7 +1470,7 @@ bool SampleProfileLoader::runOnModule(Module &M) { for (auto &F : M) if (!F.isDeclaration()) { clearFunctionData(); - retval |= runOnFunction(F); + retval |= runOnFunction(F, AM); } if (M.getProfileSummary() == nullptr) M.setProfileSummary(Reader->getSummary().getMD(M.getContext())); @@ -1475,11 +1480,21 @@ bool SampleProfileLoader::runOnModule(Module &M) { bool SampleProfileLoaderLegacyPass::runOnModule(Module &M) { // FIXME: pass in AssumptionCache correctly for the new pass manager. SampleLoader.setACT(&getAnalysis()); - return SampleLoader.runOnModule(M); + return SampleLoader.runOnModule(M, nullptr); } -bool SampleProfileLoader::runOnFunction(Function &F) { +bool SampleProfileLoader::runOnFunction(Function &F, ModuleAnalysisManager *AM) { F.setEntryCount(0); + std::unique_ptr OwnedORE; + if (AM) { + auto &FAM = + AM->getResult(*F.getParent()) + .getManager(); + ORE = &FAM.getResult(F); + } else { + OwnedORE = make_unique(&F); + ORE = OwnedORE.get(); + } Samples = Reader->getSamplesFor(F); if (Samples && !Samples->empty()) return emitAnnotations(F); @@ -1494,7 +1509,7 @@ PreservedAnalyses SampleProfileLoaderPass::run(Module &M, SampleLoader.doInitialization(M); - if (!SampleLoader.runOnModule(M)) + if (!SampleLoader.runOnModule(M, &AM)) return PreservedAnalyses::all(); return PreservedAnalyses::none(); diff --git a/test/Transforms/SampleProfile/cov-zero-samples.ll b/test/Transforms/SampleProfile/cov-zero-samples.ll index 5239d74fdc6..07b2d610612 100644 --- a/test/Transforms/SampleProfile/cov-zero-samples.ll +++ b/test/Transforms/SampleProfile/cov-zero-samples.ll @@ -1,5 +1,5 @@ -; RUN: opt < %s -instcombine -sample-profile -sample-profile-file=%S/Inputs/cov-zero-samples.prof -sample-profile-check-record-coverage=100 -pass-remarks=sample-profile -o /dev/null 2>&1 | FileCheck %s -; RUN: opt < %s -passes="function(instcombine),sample-profile" -sample-profile-file=%S/Inputs/cov-zero-samples.prof -sample-profile-check-record-coverage=100 -pass-remarks=sample-profile -o /dev/null 2>&1 | FileCheck %s +; RUN: opt < %s -instcombine -sample-profile -sample-profile-file=%S/Inputs/cov-zero-samples.prof -sample-profile-check-record-coverage=100 -pass-remarks=sample-profile -pass-remarks-analysis=sample-profile -o /dev/null 2>&1 | FileCheck %s +; RUN: opt < %s -passes="function(instcombine),sample-profile" -sample-profile-file=%S/Inputs/cov-zero-samples.prof -sample-profile-check-record-coverage=100 -pass-remarks=sample-profile -pass-remarks-analysis=sample-profile -o /dev/null 2>&1 | FileCheck %s ; ; CHECK: remark: cov-zero-samples.cc:9:29: Applied 404065 samples from profile (offset: 2.1) ; CHECK: remark: cov-zero-samples.cc:10:9: Applied 443089 samples from profile (offset: 3) diff --git a/test/Transforms/SampleProfile/inline-coverage.ll b/test/Transforms/SampleProfile/inline-coverage.ll index 080876a4647..0a77e1ca335 100644 --- a/test/Transforms/SampleProfile/inline-coverage.ll +++ b/test/Transforms/SampleProfile/inline-coverage.ll @@ -1,5 +1,5 @@ -; RUN: opt < %s -instcombine -sample-profile -sample-profile-file=%S/Inputs/inline-coverage.prof -sample-profile-check-record-coverage=100 -sample-profile-check-sample-coverage=110 -pass-remarks=sample-profile -o /dev/null 2>&1 | FileCheck %s -; RUN: opt < %s -passes="function(instcombine),sample-profile" -sample-profile-file=%S/Inputs/inline-coverage.prof -sample-profile-check-record-coverage=100 -sample-profile-check-sample-coverage=110 -pass-remarks=sample-profile -o /dev/null 2>&1 | FileCheck %s +; RUN: opt < %s -instcombine -sample-profile -sample-profile-file=%S/Inputs/inline-coverage.prof -sample-profile-check-record-coverage=100 -sample-profile-check-sample-coverage=110 -pass-remarks=sample-profile -pass-remarks-analysis=sample-profile -o /dev/null 2>&1 | FileCheck %s +; RUN: opt < %s -passes="function(instcombine),sample-profile" -sample-profile-file=%S/Inputs/inline-coverage.prof -sample-profile-check-record-coverage=100 -sample-profile-check-sample-coverage=110 -pass-remarks=sample-profile -pass-remarks-analysis=sample-profile -o /dev/null 2>&1 | FileCheck %s ; ; Original code: ; diff --git a/test/Transforms/SampleProfile/remarks.ll b/test/Transforms/SampleProfile/remarks.ll index dfb075ee00e..6285d69f9c7 100644 --- a/test/Transforms/SampleProfile/remarks.ll +++ b/test/Transforms/SampleProfile/remarks.ll @@ -1,6 +1,6 @@ -; RUN: opt < %s -sample-profile -sample-profile-file=%S/Inputs/remarks.prof -S -pass-remarks=sample-profile 2>&1 | FileCheck %s +; RUN: opt < %s -sample-profile -sample-profile-file=%S/Inputs/remarks.prof -S -pass-remarks=sample-profile -pass-remarks-output=%t.opt.yaml 2>&1 | FileCheck %s ; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/remarks.prof -S -pass-remarks=sample-profile 2>&1 | FileCheck %s -; +; RUN: FileCheck %s -check-prefix=YAML < %t.opt.yaml ; Original test case. ; ; 1 #include @@ -27,6 +27,44 @@ ; The predicate almost always chooses the 'else' branch. ; CHECK: remark: remarks.cc:9:15: most popular destination for conditional branches at remarks.cc:6:9 +; Checking to see if YAML file is generated and contains remarks +;YAML: --- !Passed +;YAML-NEXT: Pass: sample-profile +;YAML-NEXT: Name: HotInline +;YAML-NEXT: DebugLoc: { File: remarks.cc, Line: 13, Column: 21 } +;YAML-NEXT: Function: main +;YAML-NEXT: Args: +;YAML-NEXT: - String: 'inlined hot callee ''' +;YAML-NEXT: - Callee: _Z3foov +;YAML-NEXT: DebugLoc: { File: remarks.cc, Line: 3, Column: 0 } +;YAML-NEXT: - String: ''' into ''' +;YAML-NEXT: - Caller: main +;YAML-NEXT: DebugLoc: { File: remarks.cc, Line: 13, Column: 0 } +;YAML-NEXT: - String: '''' +;YAML-NEXT: ... +;YAML: --- !Analysis +;YAML-NEXT: Pass: sample-profile +;YAML-NEXT: Name: AppliedSamples +;YAML-NEXT: DebugLoc: { File: remarks.cc, Line: 5, Column: 8 } +;YAML-NEXT: Function: main +;YAML-NEXT: Args: +;YAML-NEXT: - String: 'Applied ' +;YAML-NEXT: - NumSamples: '18305' +;YAML-NEXT: - String: ' samples from profile (offset: ' +;YAML-NEXT: - LineOffset: '2' +;YAML-NEXT: - String: ')' +;YAML-NEXT: ... +;YAML: --- !Passed +;YAML-NEXT: Pass: sample-profile +;YAML-NEXT: Name: PopularDest +;YAML-NEXT: DebugLoc: { File: remarks.cc, Line: 6, Column: 9 } +;YAML-NEXT: Function: main +;YAML-NEXT: Args: +;YAML-NEXT: - String: 'most popular destination for conditional branches at ' +;YAML-NEXT: - CondBranchesLoc: 'remarks.cc:5:3' +;YAML-NEXT: DebugLoc: { File: remarks.cc, Line: 5, Column: 3 } +;YAML-NEXT: ... + ; Function Attrs: nounwind uwtable define i64 @_Z3foov() #0 !dbg !4 { entry: -- 2.50.1