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

[Snehasish Kumar via Phabricator via llvm-commits]

snehasish added inline comments.


================
Comment at: llvm/lib/ProfileData/RawMemProfReader.cpp:33
+
+  return Summary{
+      Header->version,
----------------
davidxl wrote:
> snehasish wrote:
> > davidxl wrote:
> > > This assumes one Header in the profile, but it is possible to have multiple raw profiles concatenated in the same file?
> > This is just a helper which reads some data from 1 raw profile. Start only needs to point to the beginning of a header (see usage in RawMemProfReader::printSummary). Is there anything you recommend to make this usage clearer?
> I misunderstood - I thought this interface is used to compute the 'combined' summary.
> 
> Do we have a need to have the later?  For instance, for printSummary, is it helpful to just print out the combined data to the user instead of printing out multiple 'summaries'? Intuitively, summary means the later.
> 
> 
I renamed the function to `printSummaries` and added comments to clarify what it does. For now this is just used to test the raw format reader. I don't think there is much value in the combined summary apart from sanity checking - the indexed format conversion step will have to merge the raw profiles prior to processing. At that point I can enhance the printed out summary if it helps us in testing. Wdyt?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114286