From: Diego Novillo Date: Sat, 31 Oct 2015 21:53:58 +0000 (+0000) Subject: SamplePGO - Count sample records in embedded profiles when computing coverage. X-Git-Url: http://plrg.eecs.uci.edu/git/?a=commitdiff_plain;h=012b206fd3503dbc1b2ff0bab99ef626665b16ff;p=oota-llvm.git SamplePGO - Count sample records in embedded profiles when computing coverage. The initial coverage checking code for sample records failed to count records inside inlined profiles. This change fixes the oversight. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@251752 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Transforms/IPO/SampleProfile.cpp b/lib/Transforms/IPO/SampleProfile.cpp index 51e95a5887a..7c01a8672fe 100644 --- a/lib/Transforms/IPO/SampleProfile.cpp +++ b/lib/Transforms/IPO/SampleProfile.cpp @@ -183,10 +183,11 @@ class SampleCoverageTracker { public: SampleCoverageTracker() : SampleCoverage() {} - void markSamplesUsed(const FunctionSamples *Samples, uint32_t LineOffset, + bool markSamplesUsed(const FunctionSamples *Samples, uint32_t LineOffset, uint32_t Discriminator); - unsigned computeCoverage(const FunctionSamples *Samples) const; - unsigned getNumUsedSamples(const FunctionSamples *Samples) const; + unsigned computeCoverage(unsigned Used, unsigned Total) const; + unsigned countUsedSamples(const FunctionSamples *Samples) const; + unsigned countBodySamples(const FunctionSamples *Samples) const; private: typedef DenseMap BodySampleCoverageMap; @@ -210,18 +211,35 @@ SampleCoverageTracker CoverageTracker; /// Mark as used the sample record for the given function samples at /// (LineOffset, Discriminator). -void SampleCoverageTracker::markSamplesUsed(const FunctionSamples *Samples, +/// +/// \returns true if this is the first time we mark the given record. +bool SampleCoverageTracker::markSamplesUsed(const FunctionSamples *Samples, uint32_t LineOffset, uint32_t Discriminator) { - BodySampleCoverageMap &Coverage = SampleCoverage[Samples]; - Coverage[LineLocation(LineOffset, Discriminator)]++; + LineLocation Loc(LineOffset, Discriminator); + unsigned &Count = SampleCoverage[Samples][Loc]; + return ++Count == 1; } /// Return the number of sample records that were applied from this profile. unsigned -SampleCoverageTracker::getNumUsedSamples(const FunctionSamples *Samples) const { +SampleCoverageTracker::countUsedSamples(const FunctionSamples *Samples) const { auto I = SampleCoverage.find(Samples); - return (I != SampleCoverage.end()) ? I->second.size() : 0; + unsigned Count = (I != SampleCoverage.end()) ? I->second.size() : 0; + for (const auto &I : Samples->getCallsiteSamples()) + Count += countUsedSamples(&I.second); + return Count; +} + +/// Return the number of sample records in the body of this profile. +/// +/// The count includes all the samples in inlined callees. +unsigned +SampleCoverageTracker::countBodySamples(const FunctionSamples *Samples) const { + unsigned Count = Samples->getBodySamples().size(); + for (const auto &I : Samples->getCallsiteSamples()) + Count += countBodySamples(&I.second); + return Count; } /// Return the fraction of sample records used in this profile. @@ -229,13 +247,11 @@ SampleCoverageTracker::getNumUsedSamples(const FunctionSamples *Samples) const { /// The returned value is an unsigned integer in the range 0-100 indicating /// the percentage of sample records that were used while applying this /// profile to the associated function. -unsigned -SampleCoverageTracker::computeCoverage(const FunctionSamples *Samples) const { - uint32_t NumTotalRecords = Samples->getBodySamples().size(); - uint32_t NumUsedRecords = getNumUsedSamples(Samples); - assert(NumUsedRecords <= NumTotalRecords && +unsigned SampleCoverageTracker::computeCoverage(unsigned Used, + unsigned Total) const { + assert(Used <= Total && "number of used records cannot exceed the total number of records"); - return NumTotalRecords > 0 ? NumUsedRecords * 100 / NumTotalRecords : 100; + return Total > 0 ? Used * 100 / Total : 100; } /// Clear all the per-function data used to load samples and propagate weights. @@ -323,8 +339,15 @@ SampleProfileLoader::getInstWeight(const Instruction &Inst) const { uint32_t Discriminator = DIL->getDiscriminator(); ErrorOr R = FS->findSamplesAt(LineOffset, Discriminator); if (R) { - if (SampleProfileCoverage) - CoverageTracker.markSamplesUsed(FS, LineOffset, Discriminator); + bool FirstMark = + CoverageTracker.markSamplesUsed(FS, LineOffset, Discriminator); + 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"); + } DEBUG(dbgs() << " " << Lineno << "." << DIL->getDiscriminator() << ":" << Inst << " (line offset: " << Lineno - HeaderLineno << "." << DIL->getDiscriminator() << " - weight: " << R.get() @@ -377,20 +400,6 @@ bool SampleProfileLoader::computeBlockWeights(Function &F) { DEBUG(printBlockWeight(dbgs(), &BB)); } - if (SampleProfileCoverage) { - unsigned Coverage = CoverageTracker.computeCoverage(Samples); - if (Coverage < SampleProfileCoverage) { - StringRef Filename = getDISubprogram(&F)->getFilename(); - F.getContext().diagnose(DiagnosticInfoSampleProfile( - Filename.str().c_str(), getFunctionLoc(F), - Twine(CoverageTracker.getNumUsedSamples(Samples)) + " of " + - Twine(Samples->getBodySamples().size()) + - " available profile records (" + Twine(Coverage) + - "%) were applied", - DS_Warning)); - } - } - return Changed; } @@ -994,6 +1003,21 @@ bool SampleProfileLoader::emitAnnotations(Function &F) { propagateWeights(F); } + // If coverage checking was requested, compute it now. + if (SampleProfileCoverage) { + unsigned Used = CoverageTracker.countUsedSamples(Samples); + unsigned Total = CoverageTracker.countBodySamples(Samples); + unsigned Coverage = CoverageTracker.computeCoverage(Used, Total); + if (Coverage < SampleProfileCoverage) { + StringRef Filename = getDISubprogram(&F)->getFilename(); + F.getContext().diagnose(DiagnosticInfoSampleProfile( + Filename.str().c_str(), getFunctionLoc(F), + Twine(Used) + " of " + Twine(Total) + " available profile records (" + + Twine(Coverage) + "%) were applied", + DS_Warning)); + } + } + return Changed; } diff --git a/test/Transforms/SampleProfile/Inputs/inline-coverage.prof b/test/Transforms/SampleProfile/Inputs/inline-coverage.prof new file mode 100644 index 00000000000..3d792733149 --- /dev/null +++ b/test/Transforms/SampleProfile/Inputs/inline-coverage.prof @@ -0,0 +1,7 @@ +main:501438:0 + 2.1: 23478 + 3: 23478 + 4: 0 + 0: 0 + 3: _Z3fool:172746 + 1: 31878 diff --git a/test/Transforms/SampleProfile/inline-coverage.ll b/test/Transforms/SampleProfile/inline-coverage.ll new file mode 100644 index 00000000000..77927858983 --- /dev/null +++ b/test/Transforms/SampleProfile/inline-coverage.ll @@ -0,0 +1,130 @@ +; 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 +; +; Original code: +; +; 1 #include +; 2 +; 3 long long int foo(long i) { +; 4 return rand() * i; +; 5 } +; 6 +; 7 int main() { +; 8 long long int sum = 0; +; 9 for (int i = 0; i < 200000 * 3000; i++) +; 10 sum += foo(i); +; 11 return sum > 0 ? 0 : 1; +; 12 } +; +; CHECK: remark: coverage.cc:10:12: inlined hot callee '_Z3fool' with 172746 samples into 'main' +; CHECK: remark: coverage.cc:9:19: Applied 23478 samples from profile +; CHECK: remark: coverage.cc:10:16: Applied 23478 samples from profile +; CHECK: remark: coverage.cc:4:10: Applied 31878 samples from profile +; CHECK: remark: coverage.cc:11:10: Applied 0 samples from profile +; CHECK: remark: coverage.cc:10:16: most popular destination for conditional branches at coverage.cc:9:3 +; +; 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 + +define i64 @_Z3fool(i64 %i) { +entry: + %i.addr = alloca i64, align 8 + store i64 %i, i64* %i.addr, align 8 + call void @llvm.dbg.declare(metadata i64* %i.addr, metadata !16, metadata !17), !dbg !18 + %call = call i32 @rand(), !dbg !19 + %conv = sext i32 %call to i64, !dbg !19 + %0 = load i64, i64* %i.addr, align 8, !dbg !20 + %mul = mul nsw i64 %conv, %0, !dbg !21 + ret i64 %mul, !dbg !22 +} + +declare void @llvm.dbg.declare(metadata, metadata, metadata) + +declare i32 @rand() + +define i32 @main() { +entry: + %retval = alloca i32, align 4 + %sum = alloca i64, align 8 + %i = alloca i32, align 4 + store i32 0, i32* %retval, align 4 + call void @llvm.dbg.declare(metadata i64* %sum, metadata !23, metadata !17), !dbg !24 + store i64 0, i64* %sum, align 8, !dbg !24 + call void @llvm.dbg.declare(metadata i32* %i, metadata !25, metadata !17), !dbg !27 + store i32 0, i32* %i, align 4, !dbg !27 + br label %for.cond, !dbg !28 + +for.cond: ; preds = %for.inc, %entry + %0 = load i32, i32* %i, align 4, !dbg !29 + %cmp = icmp slt i32 %0, 600000000, !dbg !32 + br i1 %cmp, label %for.body, label %for.end, !dbg !33 + +for.body: ; preds = %for.cond + %1 = load i32, i32* %i, align 4, !dbg !34 + %conv = sext i32 %1 to i64, !dbg !34 + %call = call i64 @_Z3fool(i64 %conv), !dbg !35 + %2 = load i64, i64* %sum, align 8, !dbg !36 + %add = add nsw i64 %2, %call, !dbg !36 + store i64 %add, i64* %sum, align 8, !dbg !36 + br label %for.inc, !dbg !37 + +for.inc: ; preds = %for.body + %3 = load i32, i32* %i, align 4, !dbg !38 + %inc = add nsw i32 %3, 1, !dbg !38 + store i32 %inc, i32* %i, align 4, !dbg !38 + br label %for.cond, !dbg !39 + +for.end: ; preds = %for.cond + %4 = load i64, i64* %sum, align 8, !dbg !40 + %cmp1 = icmp sgt i64 %4, 0, !dbg !41 + %cond = select i1 %cmp1, i32 0, i32 1, !dbg !40 + ret i32 %cond, !dbg !42 +} + +!llvm.dbg.cu = !{!0} +!llvm.module.flags = !{!13, !14} +!llvm.ident = !{!15} + +!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 3.8.0 (trunk 251738) (llvm/trunk 251737)", isOptimized: false, runtimeVersion: 0, emissionKind: 1, enums: !2, subprograms: !3) +!1 = !DIFile(filename: "coverage.cc", directory: ".") +!2 = !{} +!3 = !{!4, !9} +!4 = distinct !DISubprogram(name: "foo", linkageName: "_Z3fool", scope: !1, file: !1, line: 3, type: !5, isLocal: false, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: false, function: i64 (i64)* @_Z3fool, variables: !2) +!5 = !DISubroutineType(types: !6) +!6 = !{!7, !8} +!7 = !DIBasicType(name: "long long int", size: 64, align: 64, encoding: DW_ATE_signed) +!8 = !DIBasicType(name: "long int", size: 64, align: 64, encoding: DW_ATE_signed) +!9 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 7, type: !10, isLocal: false, isDefinition: true, scopeLine: 7, flags: DIFlagPrototyped, isOptimized: false, function: i32 ()* @main, variables: !2) +!10 = !DISubroutineType(types: !11) +!11 = !{!12} +!12 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed) +!13 = !{i32 2, !"Dwarf Version", i32 4} +!14 = !{i32 2, !"Debug Info Version", i32 3} +!15 = !{!"clang version 3.8.0 (trunk 251738) (llvm/trunk 251737)"} +!16 = !DILocalVariable(name: "i", arg: 1, scope: !4, file: !1, line: 3, type: !8) +!17 = !DIExpression() +!18 = !DILocation(line: 3, column: 24, scope: !4) +!19 = !DILocation(line: 4, column: 10, scope: !4) +!20 = !DILocation(line: 4, column: 19, scope: !4) +!21 = !DILocation(line: 4, column: 17, scope: !4) +!22 = !DILocation(line: 4, column: 3, scope: !4) +!23 = !DILocalVariable(name: "sum", scope: !9, file: !1, line: 8, type: !7) +!24 = !DILocation(line: 8, column: 17, scope: !9) +!25 = !DILocalVariable(name: "i", scope: !26, file: !1, line: 9, type: !12) +!26 = distinct !DILexicalBlock(scope: !9, file: !1, line: 9, column: 3) +!27 = !DILocation(line: 9, column: 12, scope: !26) +!28 = !DILocation(line: 9, column: 8, scope: !26) +!29 = !DILocation(line: 9, column: 19, scope: !30) +!30 = !DILexicalBlockFile(scope: !31, file: !1, discriminator: 1) +!31 = distinct !DILexicalBlock(scope: !26, file: !1, line: 9, column: 3) +!32 = !DILocation(line: 9, column: 21, scope: !30) +!33 = !DILocation(line: 9, column: 3, scope: !30) +!34 = !DILocation(line: 10, column: 16, scope: !31) +!35 = !DILocation(line: 10, column: 12, scope: !31) +!36 = !DILocation(line: 10, column: 9, scope: !31) +!37 = !DILocation(line: 10, column: 5, scope: !31) +!38 = !DILocation(line: 9, column: 39, scope: !31) +!39 = !DILocation(line: 9, column: 3, scope: !31) +!40 = !DILocation(line: 11, column: 10, scope: !9) +!41 = !DILocation(line: 11, column: 14, scope: !9) +!42 = !DILocation(line: 11, column: 3, scope: !9)