[PATCH] D59835: [ProfileSummary] Count callsite samples when computing total samples.

[Taewook Oh via Phabricator via llvm-commits]

twoh created this revision.
twoh added reviewers: eraman, danielcdh, wmi.
Herald added subscribers: jdoerfert, hiraditya.
Herald added a project: LLVM.

Currently ProfileSummaryBuilder doesn't count into callsite samples when computing total samples. Considering that ProfileSummaryInfo is used to checked the hotness of not only body samples but also callsite samples (from SampleProfileLoader), I think the callsite sample counts should be considered when computing total samples.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59835

Files:
  llvm/include/llvm/ProfileData/ProfileCommon.h
  llvm/lib/ProfileData/ProfileSummaryBuilder.cpp
  llvm/test/Transforms/SampleProfile/Inputs/inline.compactbinary.afdo
  llvm/test/Transforms/SampleProfile/Inputs/summary.prof
  llvm/test/Transforms/SampleProfile/compact-binary-profile.ll
  llvm/test/Transforms/SampleProfile/summary.ll


Index: llvm/test/Transforms/SampleProfile/summary.ll
===================================================================
--- llvm/test/Transforms/SampleProfile/summary.ll
+++ llvm/test/Transforms/SampleProfile/summary.ll
@@ -7,9 +7,20 @@
   ret i32 1, !dbg !2
 }
 
+define i32 @baz() !dbg !3 {
+entry:
+    %call = call i32 @bar(), !dbg !4
+    ret i32 %call, !dbg !5
+}
+
 ; CHECK-DAG: {{![0-9]+}} = !{i32 1, !"ProfileSummary", {{![0-9]+}}}
-; CHECK-DAG: {{![0-9]+}} = !{!"NumFunctions", i64 2}
+; CHECK-DAG: {{![0-9]+}} = !{!"TotalCount", i64 900}
+; CHECK-DAG: {{![0-9]+}} = !{!"NumCounts", i64 5}
+; CHECK-DAG: {{![0-9]+}} = !{!"NumFunctions", i64 3}
 ; CHECK-DAG: {{![0-9]+}} = !{!"MaxFunctionCount", i64 3}
 
 !1 = distinct !DISubprogram(name: "bar")
 !2 = !DILocation(line: 2, scope: !2)
+!3 = distinct !DISubprogram(name: "baz")
+!4 = !DILocation(line: 1, scope: !4)
+!5 = !DILocation(line: 2, scope: !5)
Index: llvm/test/Transforms/SampleProfile/compact-binary-profile.ll
===================================================================
--- llvm/test/Transforms/SampleProfile/compact-binary-profile.ll
+++ llvm/test/Transforms/SampleProfile/compact-binary-profile.ll
@@ -26,7 +26,7 @@
 ; CHECK: br i1 %cmp, label %while.body, label %while.end{{.*}} !prof ![[IDX1:[0-9]*]]
 ; CHECK: br i1 %cmp1, label %if.then, label %if.else{{.*}} !prof ![[IDX2:[0-9]*]]
 ; CHECK: call i32 (i8*, ...) @printf{{.*}} !prof ![[IDX3:[0-9]*]]
-; CHECK: = !{!"TotalCount", i64 10944}
+; CHECK: = !{!"TotalCount", i64 26781}
 ; CHECK: = !{!"MaxCount", i64 5553}
 ; CHECK: ![[IDX1]] = !{!"branch_weights", i32 5392, i32 163}
 ; CHECK: ![[IDX2]] = !{!"branch_weights", i32 5280, i32 113}
Index: llvm/test/Transforms/SampleProfile/Inputs/summary.prof
===================================================================
--- llvm/test/Transforms/SampleProfile/Inputs/summary.prof
+++ llvm/test/Transforms/SampleProfile/Inputs/summary.prof
@@ -2,3 +2,8 @@
  1: 100
 foo:200:1
  1: 200
+baz:600:1
+ 1: 0
+ 2: 300
+ 1: bar:300
+  1: 300
Index: llvm/lib/ProfileData/ProfileSummaryBuilder.cpp
===================================================================
--- llvm/lib/ProfileData/ProfileSummaryBuilder.cpp
+++ llvm/lib/ProfileData/ProfileSummaryBuilder.cpp
@@ -44,12 +44,17 @@
 // To compute the detailed summary, we consider each line containing samples as
 // equivalent to a block with a count in the instrumented profile.
 void SampleProfileSummaryBuilder::addRecord(
-    const sampleprof::FunctionSamples &FS) {
-  NumFunctions++;
-  if (FS.getHeadSamples() > MaxFunctionCount)
-    MaxFunctionCount = FS.getHeadSamples();
+    const sampleprof::FunctionSamples &FS, bool isCallsiteSample) {
+  if (!isCallsiteSample) {
+    NumFunctions++;
+    if (FS.getHeadSamples() > MaxFunctionCount)
+      MaxFunctionCount = FS.getHeadSamples();
+  }
   for (const auto &I : FS.getBodySamples())
     addCount(I.second.getSamples());
+  for (const auto &I : FS.getCallsiteSamples())
+    for (const auto &CS : I.second)
+      addRecord(CS.second, true);
 }
 
 // The argument to this method is a vector of cutoff percentages and the return
Index: llvm/include/llvm/ProfileData/ProfileCommon.h
===================================================================
--- llvm/include/llvm/ProfileData/ProfileCommon.h
+++ llvm/include/llvm/ProfileData/ProfileCommon.h
@@ -83,7 +83,8 @@
   SampleProfileSummaryBuilder(std::vector<uint32_t> Cutoffs)
       : ProfileSummaryBuilder(std::move(Cutoffs)) {}
 
-  void addRecord(const sampleprof::FunctionSamples &FS);
+  void addRecord(const sampleprof::FunctionSamples &FS,
+                 bool isCallsiteSample = false);
   std::unique_ptr<ProfileSummary> getSummary();
 };
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D59835.192319.patch
Type: text/x-patch
Size: 3700 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190326/be2817d6/attachment.bin>