[PATCH] D116783: [memprof] Print out the summary in YAML format.

Teresa Johnson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jan 15 12:59:40 PST 2022


tejohnson accepted this revision.
tejohnson added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:2484
+
+  OS << "memprof_profile:\n";
   Reader->printSummaries(OS);
----------------
snehasish wrote:
> tejohnson wrote:
> > snehasish wrote:
> > > tejohnson wrote:
> > > > Can this be moved into printSummaries?
> > > Added a new printYAML method and moved the `OS << "memprof_profile:" statement there.
> > Is there a reason for not wanting to put it at the top of printSummaries?
> Yes, the MemprofProfile marks the outermost object which holds the rest of the information incl. the header information (from printSummaries) and the contents of the raw profile (functionality added in D116784). I decided to introduce the printYAML method in this patch rather than have the print inside printSummaries and then pull it out into a separate method in the next one where we print out the contents of the profile. LMK if you feel strongly about keeping it in the printSummaries method.
Ah, I missed the fact that D116784 adds more to the outer printYAML. This looks fine then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116783



More information about the llvm-commits mailing list