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

[Wenlei He via Phabricator via llvm-commits]

wenlei accepted this revision.
wenlei added a comment.

lgtm with nit on flags. thanks!



================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.h:145
+  // the preferred address
+  Optional<bool> isLoadedByMMap;
+
----------------
wlei wrote:
> 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".
Ok, I see, thanks for clarification. In that case, after resetting, the meaning of the flag no longer abides to its definition described in the comment above? Mmap is still not seen, but we reset it to unknown just to avoid duplicated warning.. May consider a separate flag (like MissingMMapWarned) is cleaner.

Another nit: we use upper case initial for member field - `IsLoadedByMMap`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107097