[PATCH] D82355: Add --hot-func-list to llvm-profdata show for sample profiles

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:1059
+                                raw_fd_ostream &OS) {
+  assert(ColumnOffset.size() >= ColNum && ColumnTitle.size() >= ColNum);
+  assert(TotalFuncCount > 0);
----------------
Same, remove `ColNum`, then change to `assert(ColumnOffset.size() == ColumnTitle.size());`


================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:1088-1099
+    if (ColNum > 1) {
+      FOS.PadToColumn(ColumnOffset[1]);
+      FOS << R.MaxCount;
+      if (ColNum > 2) {
+        FOS.PadToColumn(ColumnOffset[2]);
+        FOS << R.EntryCount;
+        if (ColNum > 3) {
----------------
I think we can just assume `ColNum` is `ColumnOffset .size()`, then remove the nested ifs.


================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:1164
+  }
+  dumpHotFunctionList(ColNum, ColumnTitle, ColumnOffset, PrintValues,
+                      HotFuncCount, Profiles.size(), HotFuncSample,
----------------
Why do we need `ColNum` as a parameter? We are always going to print `ColumnTitle.size()` columns, right? Can we simplify the code to remove `ColNum` parameter?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82355