[PATCH] D107097: [llvm-profgen] Support perf script without parsing MMap events

[Wenlei He via Phabricator via llvm-commits]

wenlei added a comment.

Thanks for making the changes. It's almost ready, except two more tweaks.



================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:657-664
+      auto &isLoadedByMMap = Sample->Binary->getIsLoadedByMMap();
+      if (isLoadedByMMap.hasValue() && !isLoadedByMMap) {
+        WithColor::warning()
+            << "No relevant mmap event is matched, will use preferred "
+               "address as the base loading address!\n";
+        // Avoid redundant warning, only warn at the first unmatched sample.
+        isLoadedByMMap.reset();
----------------
Would it be cleaner if we move this to be after line 646 - at that point, we've located the binary using top addresses from stack sample, and we're ready to warn even if we don't get any lbr extracted due to leading outgoing branches.


================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.h:145
+  // the preferred address
+  Optional<bool> isLoadedByMMap;
+
----------------
Do we need a three-way Optional value here? Would it work if we just always initialize it to false, and set it to true when mmap is seen for the binary? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107097