<div dir="ltr">Thanks, Sean. Patch coming up.</div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Nov 20, 2015 at 10:52 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.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 Fri, Nov 20, 2015 at 1:46 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: Fri Nov 20 15:46:38 2015<br>
New Revision: 253716<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=253716&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=253716&view=rev</a><br>
Log:<br>
SamplePGO - Do not count never-executed inlined functions when computing coverage.<br>
<br>
If a function was originally inlined but not actually hot at runtime,<br>
its samples will not be counted inside the parent function. This throws<br>
off the coverage calculation because it expects to find more used<br>
records than it should.<br>
<br>
Fixed by ignoring functions that will not be inlined into the parent.<br>
Currently, this is inlined functions with 0 samples. In subsequent<br>
patches, I'll change this to mean "cold" functions.<br>
<br>
Added:<br>
llvm/trunk/test/Transforms/SampleProfile/Inputs/cov-zero-samples.prof<br>
llvm/trunk/test/Transforms/SampleProfile/cov-zero-samples.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=253716&r1=253715&r2=253716&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/SampleProfile.cpp?rev=253716&r1=253715&r2=253716&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/IPO/SampleProfile.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/IPO/SampleProfile.cpp Fri Nov 20 15:46:38 2015<br>
@@ -225,20 +225,39 @@ bool SampleCoverageTracker::markSamplesU<br>
unsigned<br>
SampleCoverageTracker::countUsedSamples(const FunctionSamples *Samples) const {<br>
auto I = SampleCoverage.find(Samples);<br>
+<br>
+ // The size of the coverage map for Samples represents the number of records<br>
+ // that were marked used at least once.<br>
unsigned Count = (I != SampleCoverage.end()) ? I->second.size() : 0;<br>
- for (const auto &I : Samples->getCallsiteSamples())<br>
- Count += countUsedSamples(&I.second);<br>
+<br>
+ // If there are inlined callsites in this function, count the samples found<br>
+ // in the respective bodies. However, do not bother counting callees with 0<br>
+ // total samples, these are callees that were never invoked at runtime.<br>
+ for (const auto &I : Samples->getCallsiteSamples()) {<br>
+ const FunctionSamples *CalleeSamples = &I.second;<br>
+ if (CalleeSamples->getTotalSamples() > 0)<br>
+ Count += countUsedSamples(&I.second);<br></blockquote><div><br></div></div></div><div>I think you meant to replace the `&I.second` here with the variable you defined.</div><div><br></div><div>Same in countBodySamples.</div><span class="HOEnZb"><font color="#888888"><div><br></div><div>-- Sean Silva</div></font></span><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">
+ }<br>
+<br>
return Count;<br>
}<br>
<br>
/// Return the number of sample records in the body of this profile.<br>
///<br>
-/// The count includes all the samples in inlined callees.<br>
+/// The count includes all the samples in inlined callees. However, callsites<br>
+/// with 0 samples indicate inlined function calls that were never actually<br>
+/// invoked at runtime. Ignore these callsites for coverage purposes.<br>
unsigned<br>
SampleCoverageTracker::countBodySamples(const FunctionSamples *Samples) const {<br>
unsigned Count = Samples->getBodySamples().size();<br>
- for (const auto &I : Samples->getCallsiteSamples())<br>
- Count += countBodySamples(&I.second);<br>
+<br>
+ // Count all the callsites with non-zero samples.<br>
+ for (const auto &I : Samples->getCallsiteSamples()) {<br>
+ const FunctionSamples *CalleeSamples = &I.second;<br>
+ if (CalleeSamples->getTotalSamples() > 0)<br>
+ Count += countBodySamples(&I.second);<br>
+ }<br>
+<br>
return Count;<br>
}<br>
<br>
<br>
Added: llvm/trunk/test/Transforms/SampleProfile/Inputs/cov-zero-samples.prof<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SampleProfile/Inputs/cov-zero-samples.prof?rev=253716&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SampleProfile/Inputs/cov-zero-samples.prof?rev=253716&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/SampleProfile/Inputs/cov-zero-samples.prof (added)<br>
+++ llvm/trunk/test/Transforms/SampleProfile/Inputs/cov-zero-samples.prof Fri Nov 20 15:46:38 2015<br>
@@ -0,0 +1,10 @@<br>
+main:20111403:0<br>
+ 2.1: 404065<br>
+ 3: 443089<br>
+ 3.1: 0<br>
+ 4: 404066<br>
+ 6: 0<br>
+ 7: 0<br>
+ 3.1: _Z12never_calledi:0<br>
+ 0: 0<br>
+ 1: 0<br>
<br>
Added: llvm/trunk/test/Transforms/SampleProfile/cov-zero-samples.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SampleProfile/cov-zero-samples.ll?rev=253716&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SampleProfile/cov-zero-samples.ll?rev=253716&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/SampleProfile/cov-zero-samples.ll (added)<br>
+++ llvm/trunk/test/Transforms/SampleProfile/cov-zero-samples.ll Fri Nov 20 15:46:38 2015<br>
@@ -0,0 +1,142 @@<br>
+; 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<br>
+;<br>
+; CHECK: remark: cov-zero-samples.cc:9:25: Applied 404065 samples from profile (offset: 2.1)<br>
+; CHECK: remark: cov-zero-samples.cc:10:9: Applied 443089 samples from profile (offset: 3)<br>
+; CHECK: remark: cov-zero-samples.cc:10:36: Applied 0 samples from profile (offset: 3.1)<br>
+; CHECK: remark: cov-zero-samples.cc:11:12: Applied 404066 samples from profile (offset: 4)<br>
+; CHECK: remark: cov-zero-samples.cc:13:25: Applied 0 samples from profile (offset: 6)<br>
+; CHECK: remark: cov-zero-samples.cc:14:3: Applied 0 samples from profile (offset: 7)<br>
+; CHECK: remark: cov-zero-samples.cc:10:9: most popular destination for conditional branches at cov-zero-samples.cc:9:3<br>
+; CHECK: remark: cov-zero-samples.cc:11:12: most popular destination for conditional branches at cov-zero-samples.cc:10:9<br>
+;<br>
+; Coverage for this profile should be 100%<br>
+; CHECK-NOT: warning: cov-zero-samples.cc:1:<br>
+<br>
+@N = global i64 8000000000, align 8<br>
+@.str = private unnamed_addr constant [11 x i8] c"sum is %d\0A\00", align 1<br>
+<br>
+; Function Attrs: nounwind uwtable<br>
+define i32 @_Z12never_calledi(i32 %i) !dbg !4 {<br>
+entry:<br>
+ ret i32 0, !dbg !32<br>
+}<br>
+<br>
+; Function Attrs: nounwind readnone<br>
+declare void @llvm.dbg.declare(metadata, metadata, metadata)<br>
+<br>
+; Function Attrs: norecurse uwtable<br>
+define i32 @main() !dbg !8 {<br>
+entry:<br>
+ %retval = alloca i32, align 4<br>
+ %sum = alloca i32, align 4<br>
+ %i = alloca i64, align 8<br>
+ store i32 0, i32* %retval, align 4<br>
+ call void @llvm.dbg.declare(metadata i32* %sum, metadata !33, metadata !19), !dbg !34<br>
+ store i32 0, i32* %sum, align 4, !dbg !34<br>
+ call void @llvm.dbg.declare(metadata i64* %i, metadata !35, metadata !19), !dbg !37<br>
+ store i64 0, i64* %i, align 8, !dbg !37<br>
+ br label %for.cond, !dbg !38<br>
+<br>
+for.cond: ; preds = %for.inc, %entry<br>
+ %0 = load i64, i64* %i, align 8, !dbg !39<br>
+ %1 = load volatile i64, i64* @N, align 8, !dbg !42<br>
+ %cmp = icmp slt i64 %0, %1, !dbg !43<br>
+ br i1 %cmp, label %for.body, label %for.end, !dbg !44<br>
+<br>
+for.body: ; preds = %for.cond<br>
+ %2 = load i64, i64* %i, align 8, !dbg !45<br>
+ %3 = load volatile i64, i64* @N, align 8, !dbg !48<br>
+ %cmp1 = icmp sgt i64 %2, %3, !dbg !49<br>
+ br i1 %cmp1, label %if.then, label %if.end, !dbg !50<br>
+<br>
+if.then: ; preds = %for.body<br>
+ %4 = load i64, i64* %i, align 8, !dbg !51<br>
+ %conv = trunc i64 %4 to i32, !dbg !51<br>
+ %call = call i32 @_Z12never_calledi(i32 %conv), !dbg !53<br>
+ %5 = load i32, i32* %sum, align 4, !dbg !54<br>
+ %add = add nsw i32 %5, %call, !dbg !54<br>
+ store i32 %add, i32* %sum, align 4, !dbg !54<br>
+ br label %if.end, !dbg !55<br>
+<br>
+if.end: ; preds = %if.then, %for.body<br>
+ %6 = load i64, i64* %i, align 8, !dbg !56<br>
+ %div = sdiv i64 %6, 239, !dbg !57<br>
+ %7 = load i32, i32* %sum, align 4, !dbg !58<br>
+ %conv2 = sext i32 %7 to i64, !dbg !58<br>
+ %mul = mul nsw i64 %conv2, %div, !dbg !58<br>
+ %conv3 = trunc i64 %mul to i32, !dbg !58<br>
+ store i32 %conv3, i32* %sum, align 4, !dbg !58<br>
+ br label %for.inc, !dbg !59<br>
+<br>
+for.inc: ; preds = %if.end<br>
+ %8 = load i64, i64* %i, align 8, !dbg !60<br>
+ %inc = add nsw i64 %8, 1, !dbg !60<br>
+ store i64 %inc, i64* %i, align 8, !dbg !60<br>
+ br label %for.cond, !dbg !62<br>
+<br>
+for.end: ; preds = %for.cond<br>
+ %9 = load i32, i32* %sum, align 4, !dbg !63<br>
+ %call4 = call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([11 x i8], [11 x i8]* @.str, i32 0, i32 0), i32 %9), !dbg !64<br>
+ ret i32 0, !dbg !65<br>
+}<br>
+<br>
+declare i32 @printf(i8*, ...)<br>
+<br>
+!<a href="http://llvm.dbg.cu" rel="noreferrer" target="_blank">llvm.dbg.cu</a> = !{!0}<br>
+!llvm.module.flags = !{!15, !16}<br>
+!llvm.ident = !{!17}<br>
+<br>
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 3.8.0 (trunk 253667) (llvm/trunk 253670)", isOptimized: false, runtimeVersion: 0, emissionKind: 1, enums: !2, subprograms: !3, globals: !11)<br>
+!1 = !DIFile(filename: "cov-zero-samples.cc", directory: ".")<br>
+!2 = !{}<br>
+!3 = !{!4, !8}<br>
+!4 = distinct !DISubprogram(name: "never_called", linkageName: "_Z12never_calledi", scope: !1, file: !1, line: 5, type: !5, isLocal: false, isDefinition: true, scopeLine: 5, flags: DIFlagPrototyped, isOptimized: false, variables: !2)<br>
+!5 = !DISubroutineType(types: !6)<br>
+!6 = !{!7, !7}<br>
+!7 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)<br>
+!8 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 7, type: !9, isLocal: false, isDefinition: true, scopeLine: 7, flags: DIFlagPrototyped, isOptimized: false, variables: !2)<br>
+!9 = !DISubroutineType(types: !10)<br>
+!10 = !{!7}<br>
+!11 = !{!12}<br>
+!12 = !DIGlobalVariable(name: "N", scope: !0, file: !1, line: 3, type: !13, isLocal: false, isDefinition: true, variable: i64* @N)<br>
+!13 = !DIDerivedType(tag: DW_TAG_volatile_type, baseType: !14)<br>
+!14 = !DIBasicType(name: "long long int", size: 64, align: 64, encoding: DW_ATE_signed)<br>
+!15 = !{i32 2, !"Dwarf Version", i32 4}<br>
+!16 = !{i32 2, !"Debug Info Version", i32 3}<br>
+!17 = !{!"clang version 3.8.0 (trunk 253667) (llvm/trunk 253670)"}<br>
+!19 = !DIExpression()<br>
+!31 = !DILexicalBlockFile(scope: !4, file: !1, discriminator: 3)<br>
+!32 = !DILocation(line: 5, column: 27, scope: !31)<br>
+!33 = !DILocalVariable(name: "sum", scope: !8, file: !1, line: 8, type: !7)<br>
+!34 = !DILocation(line: 8, column: 7, scope: !8)<br>
+!35 = !DILocalVariable(name: "i", scope: !36, file: !1, line: 9, type: !14)<br>
+!36 = distinct !DILexicalBlock(scope: !8, file: !1, line: 9, column: 3)<br>
+!37 = !DILocation(line: 9, column: 18, scope: !36)<br>
+!38 = !DILocation(line: 9, column: 8, scope: !36)<br>
+!39 = !DILocation(line: 9, column: 25, scope: !40)<br>
+!40 = !DILexicalBlockFile(scope: !41, file: !1, discriminator: 1)<br>
+!41 = distinct !DILexicalBlock(scope: !36, file: !1, line: 9, column: 3)<br>
+!42 = !DILocation(line: 9, column: 29, scope: !40)<br>
+!43 = !DILocation(line: 9, column: 27, scope: !40)<br>
+!44 = !DILocation(line: 9, column: 3, scope: !40)<br>
+!45 = !DILocation(line: 10, column: 9, scope: !46)<br>
+!46 = distinct !DILexicalBlock(scope: !47, file: !1, line: 10, column: 9)<br>
+!47 = distinct !DILexicalBlock(scope: !41, file: !1, line: 9, column: 37)<br>
+!48 = !DILocation(line: 10, column: 13, scope: !46)<br>
+!49 = !DILocation(line: 10, column: 11, scope: !46)<br>
+!50 = !DILocation(line: 10, column: 9, scope: !47)<br>
+!51 = !DILocation(line: 10, column: 36, scope: !52)<br>
+!52 = !DILexicalBlockFile(scope: !46, file: !1, discriminator: 1)<br>
+!53 = !DILocation(line: 10, column: 23, scope: !52)<br>
+!54 = !DILocation(line: 10, column: 20, scope: !52)<br>
+!55 = !DILocation(line: 10, column: 16, scope: !52)<br>
+!56 = !DILocation(line: 11, column: 12, scope: !47)<br>
+!57 = !DILocation(line: 11, column: 14, scope: !47)<br>
+!58 = !DILocation(line: 11, column: 9, scope: !47)<br>
+!59 = !DILocation(line: 12, column: 3, scope: !47)<br>
+!60 = !DILocation(line: 9, column: 33, scope: !61)<br>
+!61 = !DILexicalBlockFile(scope: !41, file: !1, discriminator: 2)<br>
+!62 = !DILocation(line: 9, column: 3, scope: !61)<br>
+!63 = !DILocation(line: 13, column: 25, scope: !8)<br>
+!64 = !DILocation(line: 13, column: 3, scope: !8)<br>
+!65 = !DILocation(line: 14, column: 3, scope: !8)<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>