[PATCH] D34868: [Driver] Add -fdiagnostics-hotness-threshold

[Adam Nemet via Phabricator via cfe-commits]

anemet added a comment.

Great!



================
Comment at: docs/UsersManual.rst:330-332
    This option, which defaults to off, controls whether Clang prints the
    profile hotness associated with a diagnostics in the presence of
    profile-guided optimization information.  This is currently supported with
----------------
While you're here, could you please update this information.  This option is implied by -fsave-optimization-record so it's not generally true that it's off by default.


================
Comment at: include/clang/Basic/DiagnosticDriverKinds.td:198-203
 def warn_drv_fdiagnostics_show_hotness_requires_pgo : Warning<
   "argument '-fdiagnostics-show-hotness' requires profile-guided optimization information">,
   InGroup<UnusedCommandLineArgument>;
+def warn_drv_fdiagnostics_hotness_threshold_requires_pgo : Warning<
+  "argument '-fdiagnostics-hotness-threshold=' requires profile-guided optimization information">,
+  InGroup<UnusedCommandLineArgument>;
----------------
Can you merge these two by taking the name of the option as the parameter %0?


================
Comment at: include/clang/Driver/Options.td:728
+    Group<f_Group>, Flags<[CC1Option]>, MetaVarName<"<number>">,
+    HelpText<"Prevent optimization remarks from being output if they do not have at least this hotness value">;
 def fdiagnostics_show_option : Flag<["-"], "fdiagnostics-show-option">, Group<f_Group>,
----------------
Switch "hotness value" to "profile count"?


================
Comment at: lib/Frontend/CompilerInvocation.cpp:912-914
+  if (Opts.DiagnosticsWithHotness && !UsingProfile) {
     Diags.Report(diag::warn_drv_fdiagnostics_show_hotness_requires_pgo);
   }
----------------
There is no {} for single statement blocks.


================
Comment at: test/Frontend/optimization-remark-with-hotness.c:9-20
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \
 // RUN:     optimization-remark-with-hotness.c %s -emit-llvm-only \
 // RUN:     -fprofile-instrument-use-path=%t.profdata -Rpass=inline \
 // RUN:     -Rpass-analysis=inline -Rpass-missed=inline \
-// RUN:     -fdiagnostics-show-hotness -verify
+// RUN:     -fdiagnostics-show-hotness -fdiagnostics-hotness-threshold=10 \
+// RUN:     -verify
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \
----------------
I think that one of these (or additional tests) should also cover the default value (i.e. omitted -fdiagnostics-hotness-threshold=).


https://reviews.llvm.org/D34868