Add -fdiagnostics-show-hotness

[anemet]

Summary

I've recently added the ability for optimization remarks to include the
hotness of the corresponding code region. This uses PGO and allows
filtering of the optimization remarks by relevance. The idea was first
discussed here:
http://thread.gmane.org/gmane.comp.compilers.llvm.devel/98334

The general goal is to produce a YAML file with the remarks. Then, an
external tool could dynamically filter these by hotness and perhaps by
other things.

That said it makes sense to also expose this at the more basic level
where we just include the hotness info with each optimization remark.
For example, in D22694, the clang flag was pretty useful to measure the
overhead of the additional analyses required to include hotness.
(Without the flag we don't even run the analyses.)

For the record, Hal has already expressed support for the idea of this
patch on IRC.

[aaron.ballman]

include/clang/Basic/DiagnosticDriverKinds.td

193

Given that there's only one driver option that requires this diagnostic, I would replace the %0 and %1 directives with their hard-coded text for right now (unless you plan on needing this generality in the near future). If we need to generalize the diagnostic in the future, we can do so using a %select directive.

[anemet]

Address Aaron's comment

[anemet]

Add more reviewers suggested by Aaron. @dnovillo, @rsmith, is my thinking correct that this new flag does not belong to the R_Group.

[anemet]

Since -Rpass-with-hotness is not part of R_group, we need to manually forward
it to clang -cc1. I've also extended the test to cover this bug.

[aaron.ballman]

This LGTM, but you should wait for confirmation on the behavior of the R_Group behavior from @dnovillo or @rsmith before committing.

[anemet]

@dnovillo or @rsmith, can you please confirm if you agree that this new option -Rpass-with-hotness should not be part of R_group. R_group options enable/disable groups of remarks whereas this one is only modifying the text of the remarks. Thanks!

[dnovillo]

In D23284#526389, @anemet wrote:

@dnovillo or @rsmith, can you please confirm if you agree that this new option -Rpass-with-hotness should not be part of R_group. R_group options enable/disable groups of remarks whereas this one is only modifying the text of the remarks. Thanks!

I'm fine with it, but I don't have much of a say in how option groups are organized. If Richard agrees, then it LGTM.

[anemet]

In D23284#526413, @dnovillo wrote:

I'm fine with it, but I don't have much of a say in how option groups are organized. If Richard agrees, then it LGTM.

Ping. Would it be OK to commit this with the two LGTMs? I am around to fix any fall-outs or post-commit reviews from Richard.

[rsmith]

Sorry I missed this until now.

I think this should not be an -R option at all; like -W flags, the idea is for -R to only act as a filter for which diagnostics are shown. This option seems much more closely related to options like -fdiagnostics-show-option and -fdiagnostics-format=, and so should probably have a -fdiagnostics-something name.

[anemet]

In D23284#534879, @rsmith wrote:

I think this should not be an -R option at all; like -W flags, the idea is for -R to only act as a filter for which diagnostics are shown. This option seems much more closely related to options like -fdiagnostics-show-option and -fdiagnostics-format=, and so should probably have a -fdiagnostics-something name.

Sounds fine to me. Any preference from:

-fdiagnostics-include-hotness-in-remarks
-fpass-diagnostics-with-hotness
-fpass-diagnostics-include-hotness

or something else?

[rsmith]

In D23284#534919, @anemet wrote:

In D23284#534879, @rsmith wrote:

I think this should not be an -R option at all; like -W flags, the idea is for -R to only act as a filter for which diagnostics are shown. This option seems much more closely related to options like -fdiagnostics-show-option and -fdiagnostics-format=, and so should probably have a -fdiagnostics-something name.

Sounds fine to me. Any preference from:

-fdiagnostics-include-hotness-in-remarks
-fpass-diagnostics-with-hotness
-fpass-diagnostics-include-hotness

or something else?

I think this should start with -fdiagnostics to group it with other similar flags. -fdiagnostics-include-hotness-in-remarks seems too specific to me -- I could imagine generating warnings from the middle-end including hotness information, not just remarks. How about -fdiagnostics-show-hotness? (This would fit nicely with the existing -fdiagnostics-show-* flags.)

[anemet]

In D23284#535258, @rsmith wrote:

I think this should start with -fdiagnostics to group it with other similar flags. -fdiagnostics-include-hotness-in-remarks seems too specific to me -- I could imagine generating warnings from the middle-end including hotness information, not just remarks. How about -fdiagnostics-show-hotness? (This would fit nicely with the existing -fdiagnostics-show-* flags.)

SGTM. I'll update the patch.

[anemet]

This renames the flag to -fdiagnostics-show-hotness as requested by Richard.
Also added the missing documentation bits.

[rsmith]

lib/Frontend/CompilerInvocation.cpp

844

You should just use hasArg here, there is no -fno-diagnostics-show-hotness flag for -cc1.

[anemet]

Address Richard's comment

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_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.
+
+   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);
+                  ^
+
 .. _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:

 
 .. option:: -fno-crash-diagnostics
 
   Disable auto-generation of preprocessed source files during a clang crash.
 
 The -fno-crash-diagnostics flag can be helpful for speeding the process
 of generating a delta reduced test case.
 
+.. _rpass:
+
 Options to Emit Optimization Reports
 ------------------------------------
 
 Optimization reports trace, at a high-level, all the major decisions
 done by compiler transformations. For instance, when the inliner
 decides to inline function ``foo()`` into ``bar()``, or the loop unroller
 decides to unroll a loop N times, or the vectorizer decides to
 vectorize a loop body.

 To request a report from every optimization pass, you should use
 `-Rpass=.*` (in fact, you can use any valid POSIX regular
 expression). However, do not expect a report from every transformation
 made by the compiler. Optimization remarks do not really make sense
 outside of the major transformations (e.g., inlining, vectorization,
 loop optimizations) and not every optimization pass supports this
 feature.
 
+Note that when using profile-guided optimization information, profile hotness
+information can be included in the remarks (see
+:ref:`-fdiagnostics-show-hotness <opt_fdiagnostics-show-hotness>`).
+
 Current limitations
 ^^^^^^^^^^^^^^^^^^^
 
 1. Optimization remarks that refer to function names will display the
    mangled name of the function. Since these remarks are emitted by the
    back end of the compiler, it does not know anything about the input
    language, nor its mangling rules.
 

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">,
+  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">;

include/clang/Driver/Options.td

 def fdepfile_entry : Joined<["-"], "fdepfile-entry=">,
     Group<f_clang_Group>, Flags<[CC1Option]>;
 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_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]>,

 def fno_constant_cfstrings : Flag<["-"], "fno-constant-cfstrings">, Group<f_Group>,
   Flags<[CC1Option]>,
   HelpText<"Disable creation of CodeFoundation-type constant strings">;
 def fno_cxx_exceptions: Flag<["-"], "fno-cxx-exceptions">, Group<f_Group>;
 def fno_cxx_modules : Flag <["-"], "fno-cxx-modules">, Group<f_Group>,
   Flags<[DriverOption]>;
 def fno_diagnostics_fixit_info : Flag<["-"], "fno-diagnostics-fixit-info">, Group<f_Group>,
   Flags<[CC1Option]>, HelpText<"Do not include fixit information in diagnostics">;
+def fno_diagnostics_show_hotness : Flag<["-"], "fno-diagnostics-show-hotness">, Group<f_Group>;
 def fno_diagnostics_show_option : Flag<["-"], "fno-diagnostics-show-option">, Group<f_Group>;
 def fno_diagnostics_show_note_include_stack : Flag<["-"], "fno-diagnostics-show-note-include-stack">,
     Flags<[CC1Option]>, Group<f_Group>;
 def fno_declspec : Flag<["-"], "fno-declspec">, Group<f_clang_Group>,
   HelpText<"Disallow __declspec as a keyword">, Flags<[CC1Option]>;
 def fno_dollars_in_identifiers : Flag<["-"], "fno-dollars-in-identifiers">, Group<f_Group>,
   HelpText<"Disallow '$' in identifiers">, Flags<[CC1Option]>;
 def fno_elide_constructors : Flag<["-"], "fno-elide-constructors">, Group<f_Group>,

include/clang/Frontend/CodeGenOptions.def

 
 /// The default TLS model to use.
 ENUM_CODEGENOPT(DefaultTLSModel, TLSModel, 2, GeneralDynamicTLSModel)
 
 /// 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)
+
 #undef CODEGENOPT
 #undef ENUM_CODEGENOPT
 #undef VALUE_CODEGENOPT
 

lib/CodeGen/CodeGenAction.cpp

void HandleTranslationUnit(ASTContext &C) override {
         Ctx.getInlineAsmDiagnosticHandler();
       void *OldContext = Ctx.getInlineAsmDiagnosticContext();
       Ctx.setInlineAsmDiagnosticHandler(InlineAsmDiagHandler, this);
 
       LLVMContext::DiagnosticHandlerTy OldDiagnosticHandler =
           Ctx.getDiagnosticHandler();
       void *OldDiagnosticContext = Ctx.getDiagnosticContext();
       Ctx.setDiagnosticHandler(DiagnosticHandler, this);
+      Ctx.setDiagnosticHotnessRequested(CodeGenOpts.DiagnosticsWithHotness);
 
       // Link LinkModule into this module if present, preserving its validity.
       for (auto &I : LinkModules) {
         unsigned LinkFlags = I.first;
         CurLinkModule = I.second.get();
         if (Linker::linkModules(*getModule(), std::move(I.second), LinkFlags))
           return;
       }
assert(D.getSeverity() == llvm::DS_Remark ||
          D.getSeverity() == llvm::DS_Warning);
 
   StringRef Filename;
   unsigned Line, Column;
   bool BadDebugInfo = false;
   FullSourceLoc Loc = getBestLocationFromDebugLoc(D, BadDebugInfo, Filename,
       Line, Column);
 
+  std::string Msg;
+  raw_string_ostream MsgStream(Msg);
+  MsgStream << D.getMsg().str();
+
+  if (D.getHotness())
+    MsgStream << " (hotness: " << *D.getHotness() << ")";
+
   Diags.Report(Loc, DiagID)
       << AddFlagValue(D.getPassName() ? D.getPassName() : "")
-      << D.getMsg().str();
+      << MsgStream.str();
 
   if (BadDebugInfo)
     // If we were not able to translate the file:line:col information
     // back to a SourceLocation, at least emit a note stating that
     // we could not translate this location. This can happen in the
     // case of #line directives.
     Diags.Report(Loc, diag::note_fe_backend_invalid_loc)
         << Filename << Line << Column;

lib/Driver/Tools.cpp

for (const Arg *A :
        Args.filtered(options::OPT_clang_ignored_gcc_optimization_f_Group)) {
     D.Diag(diag::warn_ignored_gcc_optimization) << A->getAsString(Args);
     A->claim();
   }
 
   claimNoWarnArgs(Args);
 
   Args.AddAllArgs(CmdArgs, options::OPT_R_Group);
+
   Args.AddAllArgs(CmdArgs, options::OPT_W_Group);
   if (Args.hasFlag(options::OPT_pedantic, options::OPT_no_pedantic, false))
     CmdArgs.push_back("-pedantic");
   Args.AddLastArg(CmdArgs, options::OPT_pedantic_errors);
   Args.AddLastArg(CmdArgs, options::OPT_w);
 
   // Handle -{std, ansi, trigraphs} -- take the last of -{std, ansi}
   // (-ansi is equivalent to -std=c89 or -std=c++98).
if (Args.hasFlag(options::OPT_fdiagnostics_show_option,
     CmdArgs.push_back("-fdiagnostics-show-option");
 
   if (const Arg *A =
           Args.getLastArg(options::OPT_fdiagnostics_show_category_EQ)) {
     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_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);
+  if (Opts.DiagnosticsWithHotness &&
+      Opts.getProfileUse() == CodeGenOptions::ProfileNone)
+    Diags.Report(diag::warn_drv_fdiagnostics_show_hotness_requires_pgo);
+
   // 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 (!Opts.SampleProfileFile.empty())
     NeedLocTracking = true;
 
   // If the user requested a flag that requires source locations available in
   // the backend, make sure that the backend tracks source location information.

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

+foo
+# Func Hash:
+0
+# Num Counters:
+1
+# Counter Values:
+30
+
+bar
+# Func Hash:
+0
+# Num Counters:
+1
+# Counter Values:
+30
+
+main
+# Func Hash:
+4
+# Num Counters:
+2
+# Counter Values:
+1
+30
+

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

+// RUN: llvm-profdata merge \
+// RUN:     %S/Inputs/optimization-remark-with-hotness.proftext   \
+// RUN:     -o %t.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 -fdiagnostics-show-hotness -verify
+// The clang version of the previous test.
+// RUN: %clang -target x86_64-apple-macosx10.9 %s -o /dev/null \
+// RUN:     -fprofile-instr-use=%t.profdata -Rpass=inline \
+// RUN:     -Rpass-analysis=inline -fdiagnostics-show-hotness -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:     -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:     | 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:
+  // NO_PGO: '-fdiagnostics-show-hotness' 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 should never be inlined}}
+    bar(argc);
+  return sum;
+}