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

[Lei Wang via Phabricator via llvm-commits]

wlei added inline comments.


================
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();
----------------
wenlei wrote:
> 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.
Good point!


================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.h:145
+  // the preferred address
+  Optional<bool> isLoadedByMMap;
+
----------------
wenlei wrote:
> 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? 
That's to avoid redundant warnings if there're many sample without matching any leading mmap. see in line 663 `isLoadedByMMap.reset();` will mark it as "reported".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107097