[PATCH] D60977: [llvm-profdata] Add overlap command to compute similarity b/w two profile files

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: llvm/docs/CommandGuide/llvm-profdata.rst:268
+The result overlap distribution is a percentage number, ranging from 0.0% to
+100.0%, where 0.0% means there is not overlap and 100.0% means a perfect
+overlap.
----------------
not --> no


================
Comment at: llvm/docs/CommandGuide/llvm-profdata.rst:286
+
+.. option:: -output=output, -o=output
+
----------------
-o=output or -o output


================
Comment at: llvm/lib/ProfileData/InstrProf.cpp:1210
+void OverlapStats::dump(raw_fd_ostream &OS) const {
+  OS << "EdgeCount: (# of functions)\n";
+  OS << "  Overlap:\t" << format("%.3f%%", Overlap.EdgeCount * 100) << " ("
----------------
For function level dump, the # of function is redundant -- perhaps change it to # of counters

The overall dump format looks better with this:
1) program level:

      Edge Profile:   (overlap, # of functions )
                                80%,      10
       Incall Profile:  (overlap, # of functions)
                                70%,       10
       ...
2) function level
       Edge Profile: (overlap, # of counters)
                               75%,     10
       ...



================
Comment at: llvm/lib/ProfileData/InstrProf.cpp:1211
+  OS << "EdgeCount: (# of functions)\n";
+  OS << "  Overlap:\t" << format("%.3f%%", Overlap.EdgeCount * 100) << " ("
+     << Overlap.NumFuncs << ")\n";
----------------
The header and content does not match in fields.


================
Comment at: llvm/lib/ProfileData/InstrProfWriter.cpp:190
 
+void InstrProfWriter::overlapRecord(NamedInstrProfRecord &&Other,
+                                    OverlapStats &Overlap,
----------------
Why is this a writer method? 


================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:223
+    if (FuncOverlap.Valid) {
+      OS << "Function level overlap info of " << I.Name << " (hash=" << I.Hash
+         << ")\n";
----------------
The title print including the summary can be pushed into FuncOverlap::dump() method


================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:228
+  }
+  OS << "== Program level overlap info ==\n"
+     << "  BaseFilename: " << BaseFilename
----------------
The title can be pushed into FunctionOverlap::dump() method.


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

https://reviews.llvm.org/D60977