[Driver] Add -fdiagnostics-hotness-threshold

[modocache]

Summary

Depends on https://reviews.llvm.org/D34867.

Add a Clang frontend option to enable optimization remark hotness
thresholds, which were added to LLVM in https://reviews.llvm.org/D34867.
This prevents diagnostics that do not meet a minimum hotness
threshold from being output. When generating optimization remarks for large
codebases with a ton of cold code paths, this option can be used
to limit the optimization remark output at a reasonable size.
Discussion of this change can be read here:
http://lists.llvm.org/pipermail/llvm-dev/2017-June/114377.html

[anemet]

Great!

docs/UsersManual.rst

339–340

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.

include/clang/Basic/DiagnosticDriverKinds.td

197–201

Can you merge these two by taking the name of the option as the parameter %0?

include/clang/Driver/Options.td

728

Switch "hotness value" to "profile count"?

lib/Frontend/CompilerInvocation.cpp

908–920

There is no {} for single statement blocks.

test/Frontend/optimization-remark-with-hotness.c

9–20

I think that one of these (or additional tests) should also cover the default value (i.e. omitted -fdiagnostics-hotness-threshold=).

[modocache]

Address comments, for the most part. As for the documentation: I think this is OK for now, but there are definitely a few more options that I'd like to document, and those will require updating some of the existing docs as well (one example that comes to mind is '-foptimization-record-file='). If it's alright with you, I'd like to do this in a future diff.

[anemet]

Looks great, thank you!

[modocache]

Awesome, thank you for all the reviews!

docs/UsersManual.rst

::
          t.c:3:11: warning: conversion specifies type 'char *' but the argument has type 'int' [-Wformat]
          t.c:3:11: warning: conversion specifies type 'char *' but the argument has type 'int' [-Wformat,1]
          t.c:3:11: warning: conversion specifies type 'char *' but the argument has type 'int' [-Wformat,Format String]
 
    This category can be used by clients that want to group diagnostics
    by category, so it should be a high level category. We want dozens
    of these, not hundreds or thousands of them.
 
+.. _opt_fsave-optimization-record:
+
+**-fsave-optimization-record**
+   Write optimization remarks to a YAML file.
+
+   This option, which defaults to off, controls whether Clang writes
+   optimization reports to a YAML file. By recording diagnostics in a file,
+   using a structured YAML format, users can parse or sort the remarks in a
+   convenient way.
+
 .. _opt_fdiagnostics-show-hotness:
 
 **-f[no-]diagnostics-show-hotness**
    Enable profile hotness information in diagnostic line.
 
-   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
-   optimization remarks (see :ref:`Options to Emit Optimization Reports
-   <rpass>`).  The hotness information allows users to focus on the hot
-   optimization remarks that are likely to be more relevant for run-time
-   performance.
+   This option controls whether Clang prints the profile hotness associated
+   with diagnostics in the presence of profile-guided optimization information.
+   This is currently supported with optimization remarks (see
+   :ref:`Options to Emit Optimization Reports <rpass>`). The hotness information
+   allows users to focus on the hot optimization remarks that are likely to be
+   more relevant for run-time performance.
 
    For example, in this output, the block containing the callsite of `foo` was
    executed 3000 times according to the profile data:
 
    ::
 
          s.c:7:10: remark: foo inlined into bar (hotness: 3000) [-Rpass-analysis=inline]
            sum += foo(x, x - 2);
                   ^
 
+   This option is implied when
+   :ref:`-fsave-optimization-record <opt_fsave-optimization-record>` is used.
+   Otherwise, it defaults to off.
+
+.. _opt_fdiagnostics-hotness-threshold
+
+**-fdiagnostics-hotness-threshold**
+   Prevent optimization remarks from being output if they do not have at least
+   this hotness value.
+
+   This option, which defaults to zero, controls the minimum hotness an
+   optimization remark would need in order to be output by Clang. This is
+   currently supported with optimization remarks (see :ref:`Options to Emit
+   Optimization Reports <rpass>`) when profile hotness information in
+   diagnostics is enabled (see
+   :ref:`-fdiagnostics-show-hotness <opt_fdiagnostics-show-hotness>`).
+
 .. _opt_fdiagnostics-fixit-info:
 
 **-f[no-]diagnostics-fixit-info**
    Enable "FixIt" information in the diagnostics output.
 
    This option, which defaults to on, controls whether or not Clang
    prints the information on how to fix a specific diagnostic
    underneath it when it knows. For example, in this output:

include/clang/Basic/DiagnosticDriverKinds.td

 def warn_drv_preprocessed_input_file_unused : Warning<
   "%0: previously preprocessed input%select{ unused when '%2' is present|}1">,
   InGroup<UnusedCommandLineArgument>;
 def warn_drv_unused_argument : Warning<
   "argument unused during compilation: '%0'">,
   InGroup<UnusedCommandLineArgument>;
 def warn_drv_empty_joined_argument : Warning<
   "joined argument expects additional value: '%0'">,
   InGroup<UnusedCommandLineArgument>;
-def warn_drv_fdiagnostics_show_hotness_requires_pgo : Warning<
-  "argument '-fdiagnostics-show-hotness' requires profile-guided optimization information">,
+def warn_drv_diagnostics_hotness_requires_pgo : Warning<
+  "argument '%0' requires profile-guided optimization information">,
   InGroup<UnusedCommandLineArgument>;
 def warn_drv_clang_unsupported : Warning<
   "the clang compiler does not support '%0'">;
 def warn_drv_deprecated_arg : Warning<
   "argument '%0' is deprecated, use '%1' instead">, InGroup<Deprecated>;
 def warn_drv_assuming_mfloat_abi_is : Warning<
   "unknown platform, assuming -mfloat-abi=%0">;
 def warn_ignoring_ftabstop_value : Warning<
   "ignoring invalid -ftabstop value '%0', using default value %1">;
 def warn_drv_overriding_flag_option : Warning<

include/clang/Driver/Options.td

 def fdiagnostics_fixit_info : Flag<["-"], "fdiagnostics-fixit-info">, Group<f_clang_Group>;
 def fdiagnostics_parseable_fixits : Flag<["-"], "fdiagnostics-parseable-fixits">, Group<f_clang_Group>,
     Flags<[CoreOption, CC1Option]>, HelpText<"Print fix-its in machine parseable form">;
 def fdiagnostics_print_source_range_info : Flag<["-"], "fdiagnostics-print-source-range-info">,
     Group<f_clang_Group>,  Flags<[CC1Option]>,
     HelpText<"Print source range spans in numeric form">;
 def fdiagnostics_show_hotness : Flag<["-"], "fdiagnostics-show-hotness">, Group<f_Group>,
     Flags<[CC1Option]>, HelpText<"Enable profile hotness information in diagnostic line">;
+def fdiagnostics_hotness_threshold_EQ : Joined<["-"], "fdiagnostics-hotness-threshold=">,
+    Group<f_Group>, Flags<[CC1Option]>, MetaVarName<"<number>">,
+    HelpText<"Prevent optimization remarks from being output if they do not have at least this profile count">;
 def fdiagnostics_show_option : Flag<["-"], "fdiagnostics-show-option">, Group<f_Group>,
     Flags<[CC1Option]>, HelpText<"Print option name with mappable diagnostics">;
 def fdiagnostics_show_note_include_stack : Flag<["-"], "fdiagnostics-show-note-include-stack">,
     Group<f_Group>,  Flags<[CC1Option]>, HelpText<"Display include stacks for diagnostic notes">;
 def fdiagnostics_format_EQ : Joined<["-"], "fdiagnostics-format=">, Group<f_clang_Group>;
 def fdiagnostics_show_category_EQ : Joined<["-"], "fdiagnostics-show-category=">, Group<f_clang_Group>;
 def fdiagnostics_show_template_tree : Flag<["-"], "fdiagnostics-show-template-tree">,
     Group<f_Group>, Flags<[CC1Option]>,

include/clang/Frontend/CodeGenOptions.def

 
 /// Number of path components to strip when emitting checks. (0 == full
 /// filename)
 VALUE_CODEGENOPT(EmitCheckPathComponentsToStrip, 32, 0)
 
 /// Whether to report the hotness of the code region for optimization remarks.
 CODEGENOPT(DiagnosticsWithHotness, 1, 0)
 
+/// The minimum hotness value a diagnostic needs in order to be included in
+/// optimization diagnostics.
+VALUE_CODEGENOPT(DiagnosticsHotnessThreshold, 32, 0)
+
 /// Whether copy relocations support is available when building as PIE.
 CODEGENOPT(PIECopyRelocations, 1, 0)
 
 /// Whether we should use the undefined behaviour optimization for control flow
 /// paths that reach the end of a function without executing a required return.
 CODEGENOPT(StrictReturn, 1, 1)
 
 /// Whether emit extra debug info for sample pgo profile collection.

lib/CodeGen/CodeGenAction.cpp

IRGenFinished = true;
       void *OldContext = Ctx.getInlineAsmDiagnosticContext();
       Ctx.setInlineAsmDiagnosticHandler(InlineAsmDiagHandler, this);
 
       LLVMContext::DiagnosticHandlerTy OldDiagnosticHandler =
           Ctx.getDiagnosticHandler();
       void *OldDiagnosticContext = Ctx.getDiagnosticContext();
       Ctx.setDiagnosticHandler(DiagnosticHandler, this);
       Ctx.setDiagnosticsHotnessRequested(CodeGenOpts.DiagnosticsWithHotness);
+      if (CodeGenOpts.DiagnosticsHotnessThreshold != 0)
+        Ctx.setDiagnosticsHotnessThreshold(
+            CodeGenOpts.DiagnosticsHotnessThreshold);
 
       std::unique_ptr<llvm::tool_output_file> OptRecordFile;
       if (!CodeGenOpts.OptRecordFile.empty()) {
         std::error_code EC;
         OptRecordFile =
           llvm::make_unique<llvm::tool_output_file>(CodeGenOpts.OptRecordFile,
                                                     EC, sys::fs::F_None);
         if (EC) {

lib/Driver/ToolChains/Clang.cpp

if (const Arg *A =
     CmdArgs.push_back("-fdiagnostics-show-category");
     CmdArgs.push_back(A->getValue());
   }
 
   if (Args.hasFlag(options::OPT_fdiagnostics_show_hotness,
                    options::OPT_fno_diagnostics_show_hotness, false))
     CmdArgs.push_back("-fdiagnostics-show-hotness");
 
+  if (const Arg *A =
+          Args.getLastArg(options::OPT_fdiagnostics_hotness_threshold_EQ)) {
+    std::string Opt = std::string("-fdiagnostics-hotness-threshold=") + A->getValue();
+    CmdArgs.push_back(Args.MakeArgString(Opt));
+  }
+
   if (const Arg *A = Args.getLastArg(options::OPT_fdiagnostics_format_EQ)) {
     CmdArgs.push_back("-fdiagnostics-format");
     CmdArgs.push_back(A->getValue());
   }
 
   if (Arg *A = Args.getLastArg(
           options::OPT_fdiagnostics_show_note_include_stack,
           options::OPT_fno_diagnostics_show_note_include_stack)) {

lib/Frontend/CompilerInvocation.cpp

static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
   if (Arg *A = Args.getLastArg(OPT_Rpass_analysis_EQ)) {
     Opts.OptimizationRemarkAnalysisPattern =
         GenerateOptimizationRemarkRegex(Diags, Args, A);
     NeedLocTracking = true;
   }
 
   Opts.DiagnosticsWithHotness =
       Args.hasArg(options::OPT_fdiagnostics_show_hotness);
   bool UsingSampleProfile = !Opts.SampleProfileFile.empty();
+  bool UsingProfile = UsingSampleProfile ||
+      (Opts.getProfileUse() != CodeGenOptions::ProfileNone);
 
-  if (Opts.DiagnosticsWithHotness &&
-      Opts.getProfileUse() == CodeGenOptions::ProfileNone &&
-      !UsingSampleProfile) {
-    Diags.Report(diag::warn_drv_fdiagnostics_show_hotness_requires_pgo);
-  }
+  if (Opts.DiagnosticsWithHotness && !UsingProfile)
+    Diags.Report(diag::warn_drv_diagnostics_hotness_requires_pgo)
+        << "-fdiagnostics-show-hotness";
+
+  Opts.DiagnosticsHotnessThreshold = getLastArgUInt64Value(
+      Args, options::OPT_fdiagnostics_hotness_threshold_EQ, 0);
+  if (Opts.DiagnosticsHotnessThreshold > 0 && !UsingProfile)
+    Diags.Report(diag::warn_drv_diagnostics_hotness_requires_pgo)
+        << "-fdiagnostics-hotness-threshold=";
 
   // If the user requested to use a sample profile for PGO, then the
   // backend will need to track source location information so the profile
   // can be incorporated into the IR.
   if (UsingSampleProfile)
     NeedLocTracking = true;
 
   // If the user requested a flag that requires source locations available in

test/Frontend/Inputs/optimization-remark-with-hotness-sample.proftext

 foo:0:0
  0: 0
 bar:29:29
- 6: foo:0
+ 9: foo:0
 main:0:0
  0: 0 bar:0
 

test/Frontend/optimization-remark-with-hotness.c

 // Generate instrumentation and sampling profile data.
 // RUN: llvm-profdata merge \
 // RUN:     %S/Inputs/optimization-remark-with-hotness.proftext \
 // RUN:     -o %t.profdata
 // RUN: llvm-profdata merge -sample \
 // RUN:     %S/Inputs/optimization-remark-with-hotness-sample.proftext \
 // RUN:     -o %t-sample.profdata
 //
 // 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: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \
 // RUN:     optimization-remark-with-hotness.c %s -emit-llvm-only \
 // RUN:     -fprofile-sample-use=%t-sample.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
 // The clang version of the previous test.
 // RUN: %clang -target x86_64-apple-macosx10.9 %s -c -emit-llvm -o /dev/null \
 // RUN:     -fprofile-instr-use=%t.profdata -Rpass=inline \
 // RUN:     -Rpass-analysis=inline -Rpass-missed=inline \
-// RUN:     -fdiagnostics-show-hotness -Xclang -verify
+// RUN:     -fdiagnostics-show-hotness -fdiagnostics-hotness-threshold=10 \
+// RUN:     -Xclang -verify
 // 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 2>&1 | FileCheck -check-prefix=HOTNESS_OFF %s
 // 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 -Rno-pass-with-hotness  2>&1 | FileCheck \
+// RUN:     -Rpass-analysis=inline -Rno-pass-with-hotness 2>&1 | FileCheck \
 // RUN:     -check-prefix=HOTNESS_OFF %s
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \
 // RUN:     optimization-remark-with-hotness.c %s -emit-llvm-only \
-// RUN:     -Rpass=inline -Rpass-analysis=inline -fdiagnostics-show-hotness  2>&1 \
+// RUN:     -fprofile-instrument-use-path=%t.profdata -Rpass=inline \
+// RUN:     -Rpass-analysis=inline -fdiagnostics-show-hotness \
+// RUN:     -fdiagnostics-hotness-threshold=100 2>&1 \
+// RUN:     | FileCheck -allow-empty -check-prefix=THRESHOLD %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \
+// RUN:     optimization-remark-with-hotness.c %s -emit-llvm-only \
+// RUN:     -Rpass=inline -Rpass-analysis=inline \
+// RUN:     -fdiagnostics-show-hotness -fdiagnostics-hotness-threshold=10 2>&1 \
 // RUN:     | FileCheck -check-prefix=NO_PGO %s
 
 int foo(int x, int y) __attribute__((always_inline));
 int foo(int x, int y) { return x + y; }
 
 int sum = 0;
 
 void bar(int x) {
   // HOTNESS_OFF: foo inlined into bar
   // HOTNESS_OFF-NOT: hotness:
+  // THRESHOLD-NOT: inlined
+  // THRESHOLD-NOT: hotness
   // NO_PGO: '-fdiagnostics-show-hotness' requires profile-guided optimization information
+  // NO_PGO: '-fdiagnostics-hotness-threshold=' requires profile-guided optimization information
   // expected-remark@+2 {{foo should always be inlined (cost=always) (hotness: 30)}}
   // expected-remark@+1 {{foo inlined into bar (hotness: 30)}}
   sum += foo(x, x - 2);
 }
 
 int main(int argc, const char *argv[]) {
   for (int i = 0; i < 30; i++)
     // expected-remark@+1 {{bar not inlined into main because it should never be inlined}}
     bar(argc);
   return sum;
 }