[PATCH] D82355: Add --hot-func-list to llvm-profdata show for sample profiles
Wenlei He via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 23 23:25:39 PDT 2020
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
More information about the llvm-commits
mailing list