[PATCH] D138893: [llvm-profdata] Use flattening sample profile in profile supplementation

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:929
 
   /// Return the maximum of sample counts in a function body including functions
   /// inlined in it.
----------------
The comment needs to be updated to reflect the new behavior.


================
Comment at: llvm/test/tools/llvm-profdata/suppl-instr-with-sample-flatten.test:4
+
+Test all of goo's counters will be set to PseudoHot.
+RUN: llvm-profdata merge \
----------------
The test only sets bar.cc::bar's counter, not goo's?


================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:637
+  // profile does not have inlined callsite profiles.
+  // One caveat is the pre-inlined function -- their samples
+  // should be collapsed into the caller function.
----------------
pre-inlined, do you mean early-inlined?


================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:641
+  // info: the sum of entrycount and the max of maxcount.
+  // Here is the algorithm:
+  //   recursive (FS, root_name) {
----------------
Perhaps describe with a short example.


================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:663
+                                              const StringRef &RootName) {
+    auto BuildMaxSampleMapImpl = [&](const FunctionSamples &FS,
+                                     const StringRef &RootName,
----------------
There is no need for the nested lambda.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138893/new/

https://reviews.llvm.org/D138893