[PATCH] D89707: [CSSPGO][llvm-profgen] Parse mmap events from perf script

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/docs/CommandGuide/llvm-profgen.rst:14
+
+The :program:`llvm-profgen` utility generates a SPGO profile data file from 
+given perf script data files.
----------------
wlei wrote:
> wmi wrote:
> > Please explain what SPGO represents here.
> @wmi Thanks for the helpful list of feedbacks. More description is added here
> 
small nit: searching through the code base, seems like the canonical name is "sample-based profile guided optimizations", most notably from the help message of `fprofile-sample-use`, so let's be consistent. 


================
Comment at: llvm/tools/llvm-profgen/llvm-profgen.cpp:149
+  ///
+  void parseMMap2Event(const line_iterator Line) {
+    // Parse a line like:
----------------
`const line_iterator&` ?


================
Comment at: llvm/tools/llvm-profgen/llvm-profgen.cpp:210
+    // Load the binaries.
+    for (auto Filename : BinaryFilenames)
+      loadBinary(Filename, /*AllowNameConflict*/ false);
----------------
What about we pass in `BinaryFilenames` and `PerfTraceFilenames` as parameters to this function (or to ctor), instead of letting `PerfReader` coupled with command-line options directly. Then perhaps name it `readFromInput(BinaryFilenames, PerfTraceFilenames)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89707