[PATCH] D79291: [llvm-profdata] Support -detailed-summary for Sample Profile

[Wei Mi via Phabricator via llvm-commits]

wmi added inline comments.


================
Comment at: llvm/lib/IR/ProfileSummary.cpp:209-215
+void ProfileSummary::printSummary(raw_ostream &OS) {
+  OS << "Total functions: " << NumFunctions << "\n";
+  OS << "Maximum function count: " << MaxFunctionCount << "\n";
+  OS << "Maximum sample count: " << MaxCount << "\n";
+}
+
+void ProfileSummary::printDetailedSummary(raw_ostream &OS) {
----------------
wenlei wrote:
> wmi wrote:
> > wenlei wrote:
> > > wmi wrote:
> > > > Why we need printSummary and printDetailedSummary instead of one print function? printSummary seems only print very limited information in ProfileSummary.
> > > That was mostly for sharing printDetailedSummary between InstrProf and SampleProf for llvm-profdata. The printSummary part is always printed for InstrProf, so separated out in order to share printDetailedSummary.
> > > 
> > > 
> > I mean can we have one print function, printSummary for example, and dump everything including the detail inside of it? Is it a problem for InstrProf test?
> Yeah, it causes problem for InstrProf tests. Stuff in `printSummary` is always printed for InstrProfile even without detailed-summary flag, and it's interleaved with other outputs. Tests actually leverage the interleaved structure for checking, so if we combine `printSummary` with `printDetailedSummary`, many tests need to be updated, and detail-summary need to be added to many tests too. We could do that, I was trying to avoid that churn though... 
Ok, then can we let printDetailedSummary just print stuff in DetailedSummary and let printSummary print more information like NumCounts, TotalCounts, ....?  It makes what those functions should print a little clearer.

InstrProf can print NumCounts and TotalCounts directly since it has already done that in this way. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79291