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

[Snehasish Kumar via Phabricator via llvm-commits]

snehasish added a comment.

In D116783#3228598 <https://reviews.llvm.org/D116783#3228598>, @tejohnson wrote:

> In D116783#3228553 <https://reviews.llvm.org/D116783#3228553>, @snehasish wrote:
>
>> PTAL, thanks!
>>
>> I would really like to have a test to ensure that the YAML we generate is valid but it seems hard to implement without defining a document type to parse into as a unittest (see https://llvm.org/docs/YamlIO.html#input). Alternatively, I could also extend the lit based test to check if YAML is available like this usage in MLIR (https://git.io/J9ORA). Any suggestions?
>
> I don't have a strong feeling. Looks like the tests that dump opt remarks to yaml format just manually check that the output lines look as expected but don't attempt to parse them via yaml itself.

Ok, let me punt on this for now.

I've changed the YAML naming from snake case to match the LLVM variable naming style to keep it consistent with the output from the macro based implementation in D117256 <https://reviews.llvm.org/D117256>.

PTAL, thanks!



================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:2484
+
+  OS << "memprof_profile:\n";
   Reader->printSummaries(OS);
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116783