From 243ea6a7d6d90eb52d3e30cb4a2b73d3fd428f88 Mon Sep 17 00:00:00 2001 From: Diego Novillo Date: Mon, 23 Nov 2015 20:12:21 +0000 Subject: [PATCH] SamplePGO - Add coverage tracking for samples. The existing coverage tracker counts the number of records that were used from the input profile. An alternative view of coverage is to check how many available samples were applied. This way, if the profile contains several records with few samples, it doesn't really matter much that they were not applied. The more interesting records to apply are the ones that contribute many samples. llvm-svn: 253912 --- llvm/lib/Transforms/IPO/SampleProfile.cpp | 111 ++++++++++++++---- .../SampleProfile/cov-zero-samples.ll | 2 +- .../SampleProfile/coverage-warning.ll | 3 +- .../SampleProfile/inline-coverage.ll | 7 +- 4 files changed, 94 insertions(+), 29 deletions(-) diff --git a/llvm/lib/Transforms/IPO/SampleProfile.cpp b/llvm/lib/Transforms/IPO/SampleProfile.cpp index de4170692fe0..843453775fb0 100644 --- a/llvm/lib/Transforms/IPO/SampleProfile.cpp +++ b/llvm/lib/Transforms/IPO/SampleProfile.cpp @@ -63,8 +63,12 @@ static cl::opt SampleProfileMaxPropagateIterations( "sample-profile-max-propagate-iterations", cl::init(100), cl::desc("Maximum number of iterations to go through when propagating " "sample block/edge weights through the CFG.")); -static cl::opt SampleProfileCoverage( - "sample-profile-check-coverage", cl::init(0), cl::value_desc("N"), +static cl::opt SampleProfileRecordCoverage( + "sample-profile-check-record-coverage", cl::init(0), cl::value_desc("N"), + cl::desc("Emit a warning if less than N% of records in the input profile " + "are matched to the IR.")); +static cl::opt SampleProfileSampleCoverage( + "sample-profile-check-sample-coverage", cl::init(0), cl::value_desc("N"), cl::desc("Emit a warning if less than N% of samples in the input profile " "are matched to the IR.")); @@ -181,14 +185,19 @@ protected: class SampleCoverageTracker { public: - SampleCoverageTracker() : SampleCoverage() {} + SampleCoverageTracker() : SampleCoverage(), TotalUsedSamples(0) {} - bool markSamplesUsed(const FunctionSamples *Samples, uint32_t LineOffset, - uint32_t Discriminator); + bool markSamplesUsed(const FunctionSamples *FS, uint32_t LineOffset, + uint32_t Discriminator, uint64_t Samples); unsigned computeCoverage(unsigned Used, unsigned Total) const; - unsigned countUsedSamples(const FunctionSamples *Samples) const; - unsigned countBodySamples(const FunctionSamples *Samples) const; - void clear() { SampleCoverage.clear(); } + unsigned countUsedRecords(const FunctionSamples *FS) const; + unsigned countBodyRecords(const FunctionSamples *FS) const; + uint64_t getTotalUsedSamples() const { return TotalUsedSamples; } + uint64_t countBodySamples(const FunctionSamples *FS) const; + void clear() { + SampleCoverage.clear(); + TotalUsedSamples = 0; + } private: typedef DenseMap BodySampleCoverageMap; @@ -205,6 +214,19 @@ private: /// another map that counts how many times the sample record at the /// given location has been used. FunctionSamplesCoverageMap SampleCoverage; + + /// Number of samples used from the profile. + /// + /// When a sampling record is used for the first time, the samples from + /// that record are added to this accumulator. Coverage is later computed + /// based on the total number of samples available in this function and + /// its callsites. + /// + /// Note that this accumulator tracks samples used from a single function + /// and all the inlined callsites. Strictly, we should have a map of counters + /// keyed by FunctionSamples pointers, but these stats are cleared after + /// every function, so we just need to keep a single counter. + uint64_t TotalUsedSamples; }; SampleCoverageTracker CoverageTracker; @@ -214,30 +236,34 @@ SampleCoverageTracker CoverageTracker; /// (LineOffset, Discriminator). /// /// \returns true if this is the first time we mark the given record. -bool SampleCoverageTracker::markSamplesUsed(const FunctionSamples *Samples, +bool SampleCoverageTracker::markSamplesUsed(const FunctionSamples *FS, uint32_t LineOffset, - uint32_t Discriminator) { + uint32_t Discriminator, + uint64_t Samples) { LineLocation Loc(LineOffset, Discriminator); - unsigned &Count = SampleCoverage[Samples][Loc]; - return ++Count == 1; + unsigned &Count = SampleCoverage[FS][Loc]; + bool FirstTime = (++Count == 1); + if (FirstTime) + TotalUsedSamples += Samples; + return FirstTime; } /// Return the number of sample records that were applied from this profile. unsigned -SampleCoverageTracker::countUsedSamples(const FunctionSamples *Samples) const { - auto I = SampleCoverage.find(Samples); +SampleCoverageTracker::countUsedRecords(const FunctionSamples *FS) const { + auto I = SampleCoverage.find(FS); - // The size of the coverage map for Samples represents the number of records + // The size of the coverage map for FS represents the number of records // that were marked used at least once. unsigned Count = (I != SampleCoverage.end()) ? I->second.size() : 0; // If there are inlined callsites in this function, count the samples found // in the respective bodies. However, do not bother counting callees with 0 // total samples, these are callees that were never invoked at runtime. - for (const auto &I : Samples->getCallsiteSamples()) { + for (const auto &I : FS->getCallsiteSamples()) { const FunctionSamples *CalleeSamples = &I.second; if (CalleeSamples->getTotalSamples() > 0) - Count += countUsedSamples(CalleeSamples); + Count += countUsedRecords(CalleeSamples); } return Count; @@ -249,19 +275,40 @@ SampleCoverageTracker::countUsedSamples(const FunctionSamples *Samples) const { /// with 0 samples indicate inlined function calls that were never actually /// invoked at runtime. Ignore these callsites for coverage purposes. unsigned -SampleCoverageTracker::countBodySamples(const FunctionSamples *Samples) const { - unsigned Count = Samples->getBodySamples().size(); +SampleCoverageTracker::countBodyRecords(const FunctionSamples *FS) const { + unsigned Count = FS->getBodySamples().size(); // Count all the callsites with non-zero samples. - for (const auto &I : Samples->getCallsiteSamples()) { + for (const auto &I : FS->getCallsiteSamples()) { const FunctionSamples *CalleeSamples = &I.second; if (CalleeSamples->getTotalSamples() > 0) - Count += countBodySamples(CalleeSamples); + Count += countBodyRecords(CalleeSamples); } return Count; } +/// Return the number of samples collected in the body of this profile. +/// +/// The count includes all the samples in inlined callees. However, callsites +/// with 0 samples indicate inlined function calls that were never actually +/// invoked at runtime. Ignore these callsites for coverage purposes. +uint64_t +SampleCoverageTracker::countBodySamples(const FunctionSamples *FS) const { + uint64_t Total = 0; + for (const auto &I : FS->getBodySamples()) + Total += I.second.getSamples(); + + // Count all the callsites with non-zero samples. + for (const auto &I : FS->getCallsiteSamples()) { + const FunctionSamples *CalleeSamples = &I.second; + if (CalleeSamples->getTotalSamples() > 0) + Total += countBodySamples(CalleeSamples); + } + + return Total; +} + /// Return the fraction of sample records used in this profile. /// /// The returned value is an unsigned integer in the range 0-100 indicating @@ -361,7 +408,7 @@ SampleProfileLoader::getInstWeight(const Instruction &Inst) const { ErrorOr R = FS->findSamplesAt(LineOffset, Discriminator); if (R) { bool FirstMark = - CoverageTracker.markSamplesUsed(FS, LineOffset, Discriminator); + CoverageTracker.markSamplesUsed(FS, LineOffset, Discriminator, R.get()); if (FirstMark) { const Function *F = Inst.getParent()->getParent(); LLVMContext &Ctx = F->getContext(); @@ -1027,11 +1074,11 @@ bool SampleProfileLoader::emitAnnotations(Function &F) { } // If coverage checking was requested, compute it now. - if (SampleProfileCoverage) { - unsigned Used = CoverageTracker.countUsedSamples(Samples); - unsigned Total = CoverageTracker.countBodySamples(Samples); + if (SampleProfileRecordCoverage) { + unsigned Used = CoverageTracker.countUsedRecords(Samples); + unsigned Total = CoverageTracker.countBodyRecords(Samples); unsigned Coverage = CoverageTracker.computeCoverage(Used, Total); - if (Coverage < SampleProfileCoverage) { + if (Coverage < SampleProfileRecordCoverage) { F.getContext().diagnose(DiagnosticInfoSampleProfile( getDISubprogram(&F)->getFilename(), getFunctionLoc(F), Twine(Used) + " of " + Twine(Total) + " available profile records (" + @@ -1040,6 +1087,18 @@ bool SampleProfileLoader::emitAnnotations(Function &F) { } } + if (SampleProfileSampleCoverage) { + uint64_t Used = CoverageTracker.getTotalUsedSamples(); + uint64_t Total = CoverageTracker.countBodySamples(Samples); + unsigned Coverage = CoverageTracker.computeCoverage(Used, Total); + if (Coverage < SampleProfileSampleCoverage) { + F.getContext().diagnose(DiagnosticInfoSampleProfile( + getDISubprogram(&F)->getFilename(), getFunctionLoc(F), + Twine(Used) + " of " + Twine(Total) + " available profile samples (" + + Twine(Coverage) + "%) were applied", + DS_Warning)); + } + } return Changed; } diff --git a/llvm/test/Transforms/SampleProfile/cov-zero-samples.ll b/llvm/test/Transforms/SampleProfile/cov-zero-samples.ll index f7c5f82bb3eb..d81e6438ee01 100644 --- a/llvm/test/Transforms/SampleProfile/cov-zero-samples.ll +++ b/llvm/test/Transforms/SampleProfile/cov-zero-samples.ll @@ -1,4 +1,4 @@ -; RUN: opt < %s -sample-profile -sample-profile-file=%S/Inputs/cov-zero-samples.prof -sample-profile-check-coverage=100 -pass-remarks=sample-profile -o /dev/null 2>&1 | FileCheck %s +; RUN: opt < %s -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 ; ; CHECK: remark: cov-zero-samples.cc:9:25: 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/llvm/test/Transforms/SampleProfile/coverage-warning.ll b/llvm/test/Transforms/SampleProfile/coverage-warning.ll index 7b082b8ba674..14a2710b0810 100644 --- a/llvm/test/Transforms/SampleProfile/coverage-warning.ll +++ b/llvm/test/Transforms/SampleProfile/coverage-warning.ll @@ -1,9 +1,10 @@ -; RUN: opt < %s -sample-profile -sample-profile-file=%S/Inputs/coverage-warning.prof -sample-profile-check-coverage=90 -o /dev/null 2>&1 | FileCheck %s +; RUN: opt < %s -sample-profile -sample-profile-file=%S/Inputs/coverage-warning.prof -sample-profile-check-record-coverage=90 -sample-profile-check-sample-coverage=100 -o /dev/null 2>&1 | FileCheck %s define i32 @foo(i32 %i) !dbg !4 { ; The profile has samples for line locations that are no longer present. ; Coverage does not reach 90%, so we should get this warning: ; ; CHECK: warning: coverage-warning.c:1: 2 of 3 available profile records (66%) were applied +; CHECK: warning: coverage-warning.c:1: 29000 of 30700 available profile samples (94%) were applied entry: %retval = alloca i32, align 4 %i.addr = alloca i32, align 4 diff --git a/llvm/test/Transforms/SampleProfile/inline-coverage.ll b/llvm/test/Transforms/SampleProfile/inline-coverage.ll index 0b97b560a4f3..7248540b4f7c 100644 --- a/llvm/test/Transforms/SampleProfile/inline-coverage.ll +++ b/llvm/test/Transforms/SampleProfile/inline-coverage.ll @@ -1,4 +1,4 @@ -; RUN: opt < %s -sample-profile -sample-profile-file=%S/Inputs/inline-coverage.prof -sample-profile-check-coverage=100 -pass-remarks=sample-profile 2>&1 | FileCheck %s +; RUN: opt < %s -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 ; ; Original code: ; @@ -25,6 +25,11 @@ ; There is one sample record with 0 samples at offset 4 in main() that we never ; use: ; CHECK: warning: coverage.cc:7: 4 of 5 available profile records (80%) were applied +; +; Since the unused sample record contributes no samples, sample coverage should +; be 100%. Note that we get this warning because we are requesting an impossible +; 110% coverage check. +; CHECK: warning: coverage.cc:7: 78834 of 78834 available profile samples (100%) were applied define i64 @_Z3fool(i64 %i) !dbg !4 { entry: