[PATCH] D125255: [llvm-profgen] Support a threshold to control hiding warning summaries

Mon May 9 16:50:43 PDT 2022

wlei added a comment.

In D125255#3502157 <https://reviews.llvm.org/D125255#3502157>, @wenlei wrote:

> I'm not sure if a universal threshold for filtering warning is a good way to deal with this. Maybe look at this from a different angle -- for some warnings, any instance is bad, for others, we can tolerate them. Hence, an alternative: for critical ones, we should always print out any non-zero value; for others, print them only under a "verbose" mode?

Sounds good.

1. add verbose mode to enable to print all of them.
2. add a parameter to `emitWarningSummary` to customize value for critical warning, default value increase to 20%.
3. IMP, warning of "inconsistent name from symbol table and dwarf info" should be critical. changed thres to 0.

================
Comment at: llvm/tools/llvm-profgen/ErrorHandling.h:57
+  double P = static_cast<double>(Num) * 100 / Total;
+  if (P < WarningSummaryThres)
+    return;
----------------
hoy wrote:
> Use <= ?
fixed!

================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:46

+cl::opt<int> WarningSummaryThres(
+    "warning-summary-threshold", llvm::cl::init(5),
----------------
hoy wrote:
> nit: use unsigned int
> 
> Wondering what's a good place to do the validation. It should be within [1,100].
Added validation in `validateCommandLine`, it can be 0 to print all the number between [0,1].

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125255