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

Fri Jan 7 14:34:49 PST 2022

snehasish added a comment.

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?

================
Comment at: llvm/lib/ProfileData/RawMemProfReader.cpp:101

 void RawMemProfReader::printSummaries(raw_ostream &OS) const {
   const char *Next = DataBuffer->getBufferStart();
----------------
tejohnson wrote:
> Add a comment here and/or at the declaration that this is printing in YAML format.
Documented the format in the header.

================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:2484
+
+  OS << "memprof_profile:\n";
   Reader->printSummaries(OS);
----------------
tejohnson wrote:
> Can this be moved into printSummaries?
Added a new printYAML method and moved the `OS << "memprof_profile:" statement there.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116783