SamplePGO - Count sample records in embedded profiles when computing coverage.
authorDiego Novillo <dnovillo@google.com>
Sat, 31 Oct 2015 21:53:58 +0000 (21:53 +0000)
committerDiego Novillo <dnovillo@google.com>
Sat, 31 Oct 2015 21:53:58 +0000 (21:53 +0000)
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

lib/Transforms/IPO/SampleProfile.cpp
test/Transforms/SampleProfile/Inputs/inline-coverage.prof [new file with mode: 0644]
test/Transforms/SampleProfile/inline-coverage.ll [new file with mode: 0644]

index 51e95a5887a27e292aad45fb246ab19c6ddb515f..7c01a8672feac6dd9cfa3781b576984e44769bb6 100644 (file)
@@ -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<LineLocation, unsigned> 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<uint64_t> 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 (file)
index 0000000..3d79273
--- /dev/null
@@ -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 (file)
index 0000000..7792785
--- /dev/null
@@ -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 <stdlib.h>
+;     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)