<div dir="ltr">Thanks!</div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Oct 29, 2015 at 5:51 AM, Daniel Jasper <span dir="ltr"><<a href="mailto:djasper@google.com" target="_blank">djasper@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On Wed, Oct 28, 2015 at 3:30 PM, Diego Novillo via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: dnovillo<br>
Date: Wed Oct 28 17:30:25 2015<br>
New Revision: 251568<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=251568&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=251568&view=rev</a><br>
Log:<br>
SamplePGO - Add flag to check sampling coverage.<br>
<br>
This adds the flag -mllvm -sample-profile-check-coverage=N to the<br>
SampleProfile pass. N is the percent of input sample records that the<br>
user expects to apply. If the pass does not use N% (or more) of the<br>
sample records in the input, it emits a warning.<br>
<br>
This is useful to detect some forms of stale profiles. If the code has<br>
drifted enough from the original profile, there will be records that do<br>
not match the IR anymore.<br>
<br>
This will not detect cases where a sample profile record for line L is<br>
referring to some other instructions that also used to be at line L.<br>
<br>
Added:<br>
llvm/trunk/test/Transforms/SampleProfile/Inputs/coverage-warning.prof<br>
llvm/trunk/test/Transforms/SampleProfile/coverage-warning.ll<br>
Modified:<br>
llvm/trunk/lib/Transforms/IPO/SampleProfile.cpp<br>
<br>
Modified: llvm/trunk/lib/Transforms/IPO/SampleProfile.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/SampleProfile.cpp?rev=251568&r1=251567&r2=251568&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/SampleProfile.cpp?rev=251568&r1=251567&r2=251568&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/IPO/SampleProfile.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/IPO/SampleProfile.cpp Wed Oct 28 17:30:25 2015<br>
@@ -63,6 +63,10 @@ static cl::opt<unsigned> SampleProfileMa<br>
"sample-profile-max-propagate-iterations", cl::init(100),<br>
cl::desc("Maximum number of iterations to go through when propagating "<br>
"sample block/edge weights through the CFG."));<br>
+static cl::opt<unsigned> SampleProfileCoverage(<br>
+ "sample-profile-check-coverage", cl::init(0), cl::value_desc("N"),<br>
+ cl::desc("Emit a warning if less than N% of samples in the input profile "<br>
+ "are matched to the IR."));<br>
<br>
namespace {<br>
typedef DenseMap<const BasicBlock *, uint64_t> BlockWeightMap;<br>
@@ -174,6 +178,64 @@ protected:<br>
/// \brief Flag indicating whether the profile input loaded successfully.<br>
bool ProfileIsValid;<br>
};<br>
+<br>
+class SampleCoverageTracker {<br>
+public:<br>
+ SampleCoverageTracker() : SampleCoverage() {}<br>
+<br>
+ void markSamplesUsed(const FunctionSamples *Samples, uint32_t LineOffset,<br>
+ uint32_t Discriminator);<br>
+ unsigned computeCoverage(const FunctionSamples *Samples) const;<br>
+ unsigned getNumUsedSamples(const FunctionSamples *Samples) const;<br>
+<br>
+private:<br>
+ typedef DenseMap<LineLocation, unsigned> BodySampleCoverageMap;<br>
+ typedef DenseMap<const FunctionSamples *, BodySampleCoverageMap><br>
+ FunctionSamplesCoverageMap;<br>
+<br>
+ /// Coverage map for sampling records.<br>
+ ///<br>
+ /// This map keeps a record of sampling records that have been matched to<br>
+ /// an IR instruction. This is used to detect some form of staleness in<br>
+ /// profiles (see flag -sample-profile-check-coverage).<br>
+ ///<br>
+ /// Each entry in the map corresponds to a FunctionSamples instance. This is<br>
+ /// another map that counts how many times the sample record at the<br>
+ /// given location has been used.<br>
+ FunctionSamplesCoverageMap SampleCoverage;<br>
+};<br>
+<br>
+SampleCoverageTracker CoverageTracker;<br>
+}<br>
+<br>
+/// Mark as used the sample record for the given function samples at<br>
+/// (LineOffset, Discriminator).<br>
+void SampleCoverageTracker::markSamplesUsed(const FunctionSamples *Samples,<br>
+ uint32_t LineOffset,<br>
+ uint32_t Discriminator) {<br>
+ BodySampleCoverageMap &Coverage = SampleCoverage[Samples];<br>
+ Coverage[LineLocation(LineOffset, Discriminator)]++;<br>
+}<br>
+<br>
+/// Return the number of sample records that were applied from this profile.<br>
+unsigned<br>
+SampleCoverageTracker::getNumUsedSamples(const FunctionSamples *Samples) const {<br>
+ auto I = SampleCoverage.find(Samples);<br>
+ return (I != SampleCoverage.end()) ? I->second.size() : 0;<br>
+}<br>
+<br>
+/// Return the fraction of sample records used in this profile.<br>
+///<br>
+/// The returned value is an unsigned integer in the range 0-100 indicating<br>
+/// the percentage of sample records that were used while applying this<br>
+/// profile to the associated function.<br>
+unsigned<br>
+SampleCoverageTracker::computeCoverage(const FunctionSamples *Samples) const {<br>
+ uint32_t NumTotalRecords = Samples->getBodySamples().size();<br>
+ uint32_t NumUsedRecords = getNumUsedSamples(Samples);<br>
+ assert(NumUsedRecords <= NumTotalRecords &&<br>
+ "number of used records cannot exceed the total number of records");<br>
+ return NumTotalRecords > 0 ? NumUsedRecords * 100 / NumTotalRecords : 100;<br>
}<br>
<br>
/// Clear all the per-function data used to load samples and propagate weights.<br>
@@ -257,13 +319,17 @@ SampleProfileLoader::getInstWeight(const<br>
unsigned Lineno = DLoc.getLine();<br>
unsigned HeaderLineno = DIL->getScope()->getSubprogram()->getLine();<br>
<br>
- ErrorOr<uint64_t> R = FS->findSamplesAt(getOffset(Lineno, HeaderLineno),<br>
- DIL->getDiscriminator());<br>
- if (R)<br>
+ uint32_t LineOffset = getOffset(Lineno, HeaderLineno);<br>
+ uint32_t Discriminator = DIL->getDiscriminator();<br>
+ ErrorOr<uint64_t> R = FS->findSamplesAt(LineOffset, Discriminator);<br>
+ if (R) {<br>
+ if (SampleProfileCoverage)<br>
+ CoverageTracker.markSamplesUsed(FS, LineOffset, Discriminator);<br>
DEBUG(dbgs() << " " << Lineno << "." << DIL->getDiscriminator() << ":"<br>
<< Inst << " (line offset: " << Lineno - HeaderLineno << "."<br>
<< DIL->getDiscriminator() << " - weight: " << R.get()<br>
<< ")\n");<br>
+ }<br>
return R;<br>
}<br>
<br>
@@ -311,6 +377,20 @@ bool SampleProfileLoader::computeBlockWe<br>
DEBUG(printBlockWeight(dbgs(), &BB));<br>
}<br>
<br>
+ if (SampleProfileCoverage) {<br>
+ unsigned Coverage = CoverageTracker.computeCoverage(Samples);<br>
+ if (Coverage < SampleProfileCoverage) {<br>
+ const char *Filename = getDISubprogram(&F)->getFilename().str().c_str();<br></blockquote><div><br></div></div></div><div>This creates a pointer into a temporary string. Fixed in r251623.</div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+ F.getContext().diagnose(DiagnosticInfoSampleProfile(<br>
+ Filename, getFunctionLoc(F),<br>
+ Twine(CoverageTracker.getNumUsedSamples(Samples)) + " of " +<br>
+ Twine(Samples->getBodySamples().size()) +<br>
+ " available profile records (" + Twine(Coverage) +<br>
+ "%) were applied",<br>
+ DS_Warning));<br>
+ }<br>
+ }<br>
+<br>
return Changed;<br>
}<br>
<br>
<br>
Added: llvm/trunk/test/Transforms/SampleProfile/Inputs/coverage-warning.prof<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SampleProfile/Inputs/coverage-warning.prof?rev=251568&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SampleProfile/Inputs/coverage-warning.prof?rev=251568&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/SampleProfile/Inputs/coverage-warning.prof (added)<br>
+++ llvm/trunk/test/Transforms/SampleProfile/Inputs/coverage-warning.prof Wed Oct 28 17:30:25 2015<br>
@@ -0,0 +1,5 @@<br>
+foo:30000:100<br>
+ 2: 28000<br>
+ 3: 1000<br>
+# This profile is stale. Function foo() does not have a line 8 anymore.<br>
+ 8: 1700<br>
<br>
Added: llvm/trunk/test/Transforms/SampleProfile/coverage-warning.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SampleProfile/coverage-warning.ll?rev=251568&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SampleProfile/coverage-warning.ll?rev=251568&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/SampleProfile/coverage-warning.ll (added)<br>
+++ llvm/trunk/test/Transforms/SampleProfile/coverage-warning.ll Wed Oct 28 17:30:25 2015<br>
@@ -0,0 +1,45 @@<br>
+; RUN: opt < %s -sample-profile -sample-profile-file=%S/Inputs/coverage-warning.prof -sample-profile-check-coverage=90 2>& 1 | FileCheck %s<br>
+define i32 @foo(i32 %i) {<br>
+; The profile has samples for line locations that are no longer present.<br>
+; Coverage does not reach 90%, so we should get this warning:<br>
+;<br>
+; CHECK: warning: coverage-warning.c:1: 2 of 3 available profile records (66%) were applied<br>
+entry:<br>
+ %retval = alloca i32, align 4<br>
+ %i.addr = alloca i32, align 4<br>
+ store i32 %i, i32* %i.addr, align 4<br>
+ %0 = load i32, i32* %i.addr, align 4, !dbg !9<br>
+ %cmp = icmp sgt i32 %0, 1000, !dbg !10<br>
+ br i1 %cmp, label %if.then, label %if.end, !dbg !9<br>
+<br>
+if.then: ; preds = %entry<br>
+ store i32 30, i32* %retval, align 4, !dbg !11<br>
+ br label %return, !dbg !11<br>
+<br>
+if.end: ; preds = %entry<br>
+ store i32 3, i32* %retval, align 4, !dbg !12<br>
+ br label %return, !dbg !12<br>
+<br>
+return: ; preds = %if.end, %if.then<br>
+ %1 = load i32, i32* %retval, align 4, !dbg !13<br>
+ ret i32 %1, !dbg !13<br>
+}<br>
+<br>
+!<a href="http://llvm.dbg.cu" rel="noreferrer" target="_blank">llvm.dbg.cu</a> = !{!0}<br>
+!llvm.module.flags = !{!6, !7}<br>
+!llvm.ident = !{!8}<br>
+<br>
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 3.8.0 (trunk 251524) (llvm/trunk 251531)", isOptimized: false, runtimeVersion: 0, emissionKind: 2, enums: !2, subprograms: !3)<br>
+!1 = !DIFile(filename: "coverage-warning.c", directory: ".")<br>
+!2 = !{}<br>
+!3 = !{!4}<br>
+!4 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 1, type: !5, isLocal: false, isDefinition: true, scopeLine: 1, flags: DIFlagPrototyped, isOptimized: false, function: i32 (i32)* @foo, variables: !2)<br>
+!5 = !DISubroutineType(types: !2)<br>
+!6 = !{i32 2, !"Dwarf Version", i32 4}<br>
+!7 = !{i32 2, !"Debug Info Version", i32 3}<br>
+!8 = !{!"clang version 3.8.0 (trunk 251524) (llvm/trunk 251531)"}<br>
+!9 = !DILocation(line: 2, column: 7, scope: !4)<br>
+!10 = !DILocation(line: 2, column: 9, scope: !4)<br>
+!11 = !DILocation(line: 3, column: 5, scope: !4)<br>
+!12 = !DILocation(line: 4, column: 3, scope: !4)<br>
+!13 = !DILocation(line: 5, column: 1, scope: !4)<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>