From: Diego Novillo Date: Mon, 23 Nov 2015 20:12:21 +0000 (+0000) Subject: SamplePGO - Add coverage tracking for samples. X-Git-Url: http://plrg.eecs.uci.edu/git/?p=oota-llvm.git;a=commitdiff_plain;h=df79221070e72b2fc5e01dccaa5875602d7073d2 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. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@253912 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Transforms/IPO/SampleProfile.cpp b/lib/Transforms/IPO/SampleProfile.cpp index de4170692fe..843453775fb 100644 --- a/lib/Transforms/IPO/SampleProfile.cpp +++ b/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/test/Transforms/SampleProfile/cov-zero-samples.ll b/test/Transforms/SampleProfile/cov-zero-samples.ll index f7c5f82bb3e..d81e6438ee0 100644 --- a/test/Transforms/SampleProfile/cov-zero-samples.ll +++ b/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/test/Transforms/SampleProfile/coverage-warning.ll b/test/Transforms/SampleProfile/coverage-warning.ll index 7b082b8ba67..14a2710b081 100644 --- a/test/Transforms/SampleProfile/coverage-warning.ll +++ b/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/test/Transforms/SampleProfile/inline-coverage.ll b/test/Transforms/SampleProfile/inline-coverage.ll index 0b97b560a4f..7248540b4f7 100644 --- a/test/Transforms/SampleProfile/inline-coverage.ll +++ b/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: