[PATCH] D114286: [memprof] Extend llvm-profdata to display MemProf profile summaries.

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson accepted this revision.
tejohnson added a comment.

lgtm with a couple of minor comments below (please ignore until after the holiday weekend, forgot to finalize these comments yesterday).



================
Comment at: llvm/include/llvm/ProfileData/MemProfData.inc:47
+  uint64_t mib_offset;
+  uint64_t stack_offset;
+});
----------------
Nit: we're using a combo of coding styles in this file. I.e. underscore separated names (a la sanitizer runtime) in this struct and lower camel case in the SegmentEntry struct (buildId). Looks like InstrProfData.inc in this directory uses the LLVM coding style of upper camel case - perhaps that should be done here too for consistency?


================
Comment at: llvm/test/tools/llvm-profdata/memprof-multi.test:35
+We expect 2 MIB entries, 1 each for the malloc calls in the program. Unlike the
+memprof-basic.test we do not see any allocation from glibc.
+
----------------
Is this just dumb luck? Doesn't really matter since you have saved the profile, but might if we ever need to regenerate the profile as described above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114286