[PATCH] D139603: [llvm-profdata] Add option to cap profile output size

Snehasish Kumar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 28 17:24:26 PST 2022


snehasish added inline comments.


================
Comment at: llvm/include/llvm/ProfileData/SampleProfWriter.h:74
+  /// from llvm-profdata command line arguments. Ignore transient states (those
+  /// always being set by write() before use).
+  virtual void reset(std::unique_ptr<raw_ostream> &OS) {
----------------
huangjd wrote:
> snehasish wrote:
> > It looks like derived classes must always call this function. Can you add a comment here for the future? Maybe something like `// This function must always be called by the overridden implementation`? 
> It is not called by derived classes. It is called by llvm-profdata if the writer needs to be reused 
My comment was to add some documentation for future implementations which derive from SampleProfileWriter. For example, in this patch

`SampleProfileWriterText::reset` calls `SampleProfileWriter::reset(OS)` on L110. 
`SampleProfileWriterBinary::reset` calls `SampleProfileWriter::reset(OS)` on L142. 

I hope that clarifies the suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139603



More information about the llvm-commits mailing list