[Remarks][1/2] Expand remarks hotness threshold option support in more tools

This is the #1 of 2 changes that make remarks hotness threshold option
available in more tools. The changes also allow the threshold to sync with
hotness threshold from profile summary with special value 'auto'.

This change modifies the interface of lto::setupLLVMOptimizationRemarks() to
accept remarks hotness threshold. Update all the tools that use it with remarks
hotness threshold options:

* lld: '--opt-remarks-hotness-threshold='
* llvm-lto2: '--pass-remarks-hotness-threshold='
* llvm-lto: '--lto-pass-remarks-hotness-threshold='
* gold plugin: '-plugin-opt=opt-remarks-hotness-threshold='

Differential Revision: https://reviews.llvm.org/D85809

Author: apolloww

lld/ELF/Config.h

@@ -111,6 +111,7 @@ struct Configuration {
   llvm::StringRef mapFile;
   llvm::StringRef outputFile;
   llvm::StringRef optRemarksFilename;
+  llvm::Optional<uint64_t> optRemarksHotnessThreshold = 0;
   llvm::StringRef optRemarksPasses;
   llvm::StringRef optRemarksFormat;
   llvm::StringRef progName;

lld/ELF/Driver.cpp

@@ -48,6 +48,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/LTO/LTO.h"
+#include "llvm/Remarks/HotnessThresholdParser.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Compression.h"
 #include "llvm/Support/GlobPattern.h"
@@ -1013,6 +1014,17 @@ static void readConfigs(opt::InputArgList &args) {
   config->oFormatBinary = isOutputFormatBinary(args);
   config->omagic = args.hasFlag(OPT_omagic, OPT_no_omagic, false);
   config->optRemarksFilename = args.getLastArgValue(OPT_opt_remarks_filename);
+
+  // Parse remarks hotness threshold. Valid value is either integer or 'auto'.
+  if (auto *arg = args.getLastArg(OPT_opt_remarks_hotness_threshold)) {
+    auto resultOrErr = remarks::parseHotnessThresholdOption(arg->getValue());
+    if (!resultOrErr)
+      error(arg->getSpelling() + ": invalid argument '" + arg->getValue() +
+            "', only integer or 'auto' is supported");
+    else
+      config->optRemarksHotnessThreshold = *resultOrErr;
+  }
+
   config->optRemarksPasses = args.getLastArgValue(OPT_opt_remarks_passes);
   config->optRemarksWithHotness = args.hasArg(OPT_opt_remarks_with_hotness);
   config->optRemarksFormat = args.getLastArgValue(OPT_opt_remarks_format);

lld/ELF/LTO.cpp

@@ -143,6 +143,7 @@ static lto::Config createConfig() {
   c.RemarksFilename = std::string(config->optRemarksFilename);
   c.RemarksPasses = std::string(config->optRemarksPasses);
   c.RemarksWithHotness = config->optRemarksWithHotness;
+  c.RemarksHotnessThreshold = config->optRemarksHotnessThreshold;
   c.RemarksFormat = std::string(config->optRemarksFormat);
 
   c.SampleProfile = std::string(config->ltoSampleProfile);

lld/ELF/Options.td

@@ -548,6 +548,10 @@ def disable_verify: F<"disable-verify">;
 defm mllvm: Eq<"mllvm", "Additional arguments to forward to LLVM's option processing">;
 def opt_remarks_filename: Separate<["--"], "opt-remarks-filename">,
   HelpText<"YAML output file for optimization remarks">;
+defm opt_remarks_hotness_threshold: EEq<"opt-remarks-hotness-threshold",
+  "Minimum profile count required for an optimization remark to be output."
+  " Use 'auto' to apply the threshold from profile summary.">,
+  MetaVarName<"<value>">;
 def opt_remarks_passes: Separate<["--"], "opt-remarks-passes">,
   HelpText<"Regex for the passes that need to be serialized to the output file">;
 def opt_remarks_with_hotness: FF<"opt-remarks-with-hotness">,
@@ -596,6 +600,21 @@ def: J<"plugin-opt=cs-profile-path=">,
 def: J<"plugin-opt=obj-path=">,
   Alias<lto_obj_path_eq>,
   HelpText<"Alias for --lto-obj-path=">;
+def: J<"plugin-opt=opt-remarks-filename=">,
+  Alias<opt_remarks_filename>,
+  HelpText<"Alias for --opt-remarks-filename">;
+def: J<"plugin-opt=opt-remarks-passes=">,
+  Alias<opt_remarks_passes>,
+  HelpText<"Alias for --opt-remarks-passes">;
+def: J<"plugin-opt=opt-remarks-format=">,
+  Alias<opt_remarks_format>,
+  HelpText<"Alias for --opt-remarks-format">;
+def: F<"plugin-opt=opt-remarks-with-hotness">,
+  Alias<opt_remarks_with_hotness>,
+  HelpText<"Alias for --opt-remarks-with_hotness">;
+def: J<"plugin-opt=opt-remarks-hotness-threshold=">,
+  Alias<opt_remarks_hotness_threshold>,
+  HelpText<"Alias for --opt-remarks-hotness-threshold">;
 def: J<"plugin-opt=sample-profile=">,
   Alias<lto_sample_profile>, HelpText<"Alias for --lto-sample-profile">;
 def: F<"plugin-opt=save-temps">, Alias<save_temps>, HelpText<"Alias for --save-temps">;

lld/test/ELF/lto/opt-remarks.ll

@@ -1,19 +1,25 @@
 ; REQUIRES: x86
 ; RUN: llvm-as %s -o %t.o
 
-; RUN: rm -f %t.yaml
+; RUN: rm -f %t.yaml %t1.yaml %t.hot.yaml %t.t300.yaml %t.t301.yaml
 ; RUN: ld.lld --opt-remarks-filename %t.yaml %t.o -o %t -shared -save-temps
 ; RUN: llvm-dis %t.0.4.opt.bc -o - | FileCheck %s
 ; RUN: ld.lld --opt-remarks-with-hotness --opt-remarks-filename %t.hot.yaml \
 ; RUN:  %t.o -o %t -shared
+; RUN: ld.lld --opt-remarks-with-hotness --opt-remarks-hotness-threshold=300 \
+; RUN:   --opt-remarks-filename %t.t300.yaml %t.o -o %t -shared
+; RUN: ld.lld --opt-remarks-with-hotness --opt-remarks-hotness-threshold=301 \
+; RUN:   --opt-remarks-filename %t.t301.yaml %t.o -o %t -shared
 ; RUN: cat %t.yaml | FileCheck %s -check-prefix=YAML
 ; RUN: cat %t.hot.yaml | FileCheck %s -check-prefix=YAML-HOT
+; RUN: FileCheck %s -check-prefix=YAML-HOT < %t.t300.yaml
+; RUN: count 0 < %t.t301.yaml
 ; RUN: ld.lld --opt-remarks-filename %t1.yaml --opt-remarks-passes inline %t.o \
 ; RUN: -o /dev/null -shared
 ; RUN: cat %t1.yaml | FileCheck %s -check-prefix=YAML-PASSES
 ; RUN: ld.lld --opt-remarks-filename %t1.yaml --opt-remarks-format yaml %t.o \
 ; RUN: -o /dev/null -shared
-; RUN: cat %t.yaml | FileCheck %s -check-prefix=YAML
+; RUN: FileCheck %s -check-prefix=YAML < %t1.yaml
 
 ; Check that @tinkywinky is inlined after optimizations.
 ; CHECK-LABEL: define i32 @main

llvm/include/llvm/IR/LLVMContext.h

@@ -222,13 +222,20 @@ class LLVMContext {
   void setDiagnosticsHotnessRequested(bool Requested);
 
   /// Return the minimum hotness value a diagnostic would need in order
-  /// to be included in optimization diagnostics. If there is no minimum, this
-  /// returns None.
+  /// to be included in optimization diagnostics.
+  ///
+  /// Three possible return values:
+  /// 0            - threshold is disabled. Everything will be printed out.
+  /// positive int - threshold is set.
+  /// UINT64_MAX   - threshold is not yet set, and needs to be synced from
+  ///                profile summary. Note that in case of missing profile
+  ///                summary, threshold will be kept at "MAX", effectively
+  ///                suppresses all remarks output.
   uint64_t getDiagnosticsHotnessThreshold() const;
 
   /// Set the minimum hotness value a diagnostic needs in order to be
   /// included in optimization diagnostics.
-  void setDiagnosticsHotnessThreshold(uint64_t Threshold);
+  void setDiagnosticsHotnessThreshold(Optional<uint64_t> Threshold);
 
   /// The "main remark streamer" used by all the specialized remark streamers.
   /// This streamer keeps generic remark metadata in memory throughout the life

llvm/include/llvm/IR/LLVMRemarkStreamer.h

@@ -79,16 +79,15 @@ Expected<std::unique_ptr<ToolOutputFile>>
 setupLLVMOptimizationRemarks(LLVMContext &Context, StringRef RemarksFilename,
                              StringRef RemarksPasses, StringRef RemarksFormat,
                              bool RemarksWithHotness,
-                             unsigned RemarksHotnessThreshold = 0);
+                             Optional<uint64_t> RemarksHotnessThreshold = 0);
 
 /// Setup optimization remarks that output directly to a raw_ostream.
 /// \p OS is managed by the caller and should be open for writing as long as \p
 /// Context is streaming remarks to it.
-Error setupLLVMOptimizationRemarks(LLVMContext &Context, raw_ostream &OS,
-                                   StringRef RemarksPasses,
-                                   StringRef RemarksFormat,
-                                   bool RemarksWithHotness,
-                                   unsigned RemarksHotnessThreshold = 0);
+Error setupLLVMOptimizationRemarks(
+    LLVMContext &Context, raw_ostream &OS, StringRef RemarksPasses,
+    StringRef RemarksFormat, bool RemarksWithHotness,
+    Optional<uint64_t> RemarksHotnessThreshold = 0);
 
 } // end namespace llvm
 

llvm/include/llvm/LTO/Config.h

@@ -121,6 +121,21 @@ struct Config {
   /// Whether to emit optimization remarks with hotness informations.
   bool RemarksWithHotness = false;
 
+  /// The minimum hotness value a diagnostic needs in order to be included in
+  /// optimization diagnostics.
+  ///
+  /// The threshold is an Optional value, which maps to one of the 3 states:
+  /// 1. 0            => threshold disabled. All emarks will be printed.
+  /// 2. positive int => manual threshold by user. Remarks with hotness exceed
+  ///                    threshold will be printed.
+  /// 3. None         => 'auto' threshold by user. The actual value is not
+  ///                    available at command line, but will be synced with
+  ///                    hotness threhold from profile summary during
+  ///                    compilation.
+  ///
+  /// If threshold option is not specified, it is disabled by default.
+  llvm::Optional<uint64_t> RemarksHotnessThreshold = 0;
+
   /// The format used for serializing remarks (default: YAML).
   std::string RemarksFormat = "";
 

llvm/include/llvm/LTO/LTO.h

@@ -82,10 +82,10 @@ std::string getThinLTOOutputFile(const std::string &Path,
                                  const std::string &NewPrefix);
 
 /// Setup optimization remarks.
-Expected<std::unique_ptr<ToolOutputFile>>
-setupLLVMOptimizationRemarks(LLVMContext &Context, StringRef RemarksFilename,
-                             StringRef RemarksPasses, StringRef RemarksFormat,
-                             bool RemarksWithHotness, int Count = -1);
+Expected<std::unique_ptr<ToolOutputFile>> setupLLVMOptimizationRemarks(
+    LLVMContext &Context, StringRef RemarksFilename, StringRef RemarksPasses,
+    StringRef RemarksFormat, bool RemarksWithHotness,
+    Optional<uint64_t> RemarksHotnessThreshold = 0, int Count = -1);
 
 /// Setups the output file for saving statistics.
 Expected<std::unique_ptr<ToolOutputFile>>

llvm/include/llvm/Remarks/HotnessThresholdParser.h

@@ -0,0 +1,63 @@
+//===- HotnessThresholdParser.h - Parser for hotness threshold --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file implements a simple parser to decode commandline option for
+/// remarks hotness threshold that supports both int and a special 'auto' value.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_REMARKS_HOTNESSTHRESHOLDPARSER_H
+#define LLVM_REMARKS_HOTNESSTHRESHOLDPARSER_H
+
+#include "llvm/ADT/Optional.h"
+#include "llvm/Support/CommandLine.h"
+
+namespace llvm {
+namespace remarks {
+
+// Parse remarks hotness threshold argument value.
+// Valid option values are
+// 1. integer: manually specified threshold; or
+// 2. string 'auto': automatically get threshold from profile summary.
+//
+// Return None Optional if 'auto' is specified, indicating the value will
+// be filled later during PSI.
+inline Expected<Optional<uint64_t>> parseHotnessThresholdOption(StringRef Arg) {
+  if (Arg == "auto")
+    return None;
+
+  int64_t Val;
+  if (Arg.getAsInteger(10, Val))
+    return createStringError(llvm::inconvertibleErrorCode(),
+                             "Not an integer: %s", Arg.data());
+
+  // Negative integer effectively means no threshold
+  return Val < 0 ? 0 : Val;
+}
+
+// A simple CL parser for '*-remarks-hotness-threshold='
+class HotnessThresholdParser : public cl::parser<Optional<uint64_t>> {
+public:
+  HotnessThresholdParser(cl::Option &O) : cl::parser<Optional<uint64_t>>(O) {}
+
+  bool parse(cl::Option &O, StringRef ArgName, StringRef Arg,
+             Optional<uint64_t> &V) {
+    auto ResultOrErr = parseHotnessThresholdOption(Arg);
+    if (!ResultOrErr)
+      return O.error("Invalid argument '" + Arg +
+                     "', only integer or 'auto' is supported.");
+
+    V = *ResultOrErr;
+    return false;
+  }
+};
+
+} // namespace remarks
+} // namespace llvm
+#endif // LLVM_REMARKS_HOTNESSTHRESHOLDPARSER_H

llvm/include/llvm/Support/CommandLine.h

@@ -1481,7 +1481,7 @@ class opt : public Option,
 
   template <class... Mods>
   explicit opt(const Mods &... Ms)
-      : Option(Optional, NotHidden), Parser(*this) {
+      : Option(llvm::cl::Optional, NotHidden), Parser(*this) {
     apply(this, Ms...);
     done();
   }

llvm/lib/IR/LLVMContext.cpp

@@ -146,11 +146,11 @@ bool LLVMContext::getDiagnosticsHotnessRequested() const {
   return pImpl->DiagnosticsHotnessRequested;
 }
 
-void LLVMContext::setDiagnosticsHotnessThreshold(uint64_t Threshold) {
+void LLVMContext::setDiagnosticsHotnessThreshold(Optional<uint64_t> Threshold) {
   pImpl->DiagnosticsHotnessThreshold = Threshold;
 }
 uint64_t LLVMContext::getDiagnosticsHotnessThreshold() const {
-  return pImpl->DiagnosticsHotnessThreshold;
+  return pImpl->DiagnosticsHotnessThreshold.getValueOr(UINT64_MAX);
 }
 
 remarks::RemarkStreamer *LLVMContext::getMainRemarkStreamer() {

llvm/lib/IR/LLVMContextImpl.h

@@ -1323,7 +1323,26 @@ class LLVMContextImpl {
   std::unique_ptr<DiagnosticHandler> DiagHandler;
   bool RespectDiagnosticFilters = false;
   bool DiagnosticsHotnessRequested = false;
-  uint64_t DiagnosticsHotnessThreshold = 0;
+  /// The minimum hotness value a diagnostic needs in order to be included in
+  /// optimization diagnostics.
+  ///
+  /// The threshold is an Optional value, which maps to one of the 3 states:
+  /// 1). 0            => threshold disabled. All emarks will be printed.
+  /// 2). positive int => manual threshold by user. Remarks with hotness exceed
+  ///                     threshold will be printed.
+  /// 3). None         => 'auto' threshold by user. The actual value is not
+  ///                     available at command line, but will be synced with
+  ///                     hotness threhold from profile summary during
+  ///                     compilation.
+  ///
+  /// State 1 and 2 are considered as terminal states. State transition is
+  /// only allowed from 3 to 2, when the threshold is first synced with profile
+  /// summary. This ensures that the threshold is set only once and stays
+  /// constant.
+  ///
+  /// If threshold option is not specified, it is disabled (0) by default.
+  Optional<uint64_t> DiagnosticsHotnessThreshold = 0;
+
   /// The specialized remark streamer used by LLVM's OptimizationRemarkEmitter.
   std::unique_ptr<LLVMRemarkStreamer> LLVMRS;
 

llvm/lib/IR/LLVMRemarkStreamer.cpp

@@ -92,7 +92,7 @@ char LLVMRemarkSetupFormatError::ID = 0;
 Expected<std::unique_ptr<ToolOutputFile>> llvm::setupLLVMOptimizationRemarks(
     LLVMContext &Context, StringRef RemarksFilename, StringRef RemarksPasses,
     StringRef RemarksFormat, bool RemarksWithHotness,
-    unsigned RemarksHotnessThreshold) {
+    Optional<uint64_t> RemarksHotnessThreshold) {
   if (RemarksWithHotness)
     Context.setDiagnosticsHotnessRequested(true);
 
@@ -137,11 +137,10 @@ Expected<std::unique_ptr<ToolOutputFile>> llvm::setupLLVMOptimizationRemarks(
   return std::move(RemarksFile);
 }
 
-Error llvm::setupLLVMOptimizationRemarks(LLVMContext &Context, raw_ostream &OS,
-                                         StringRef RemarksPasses,
-                                         StringRef RemarksFormat,
-                                         bool RemarksWithHotness,
-                                         unsigned RemarksHotnessThreshold) {
+Error llvm::setupLLVMOptimizationRemarks(
+    LLVMContext &Context, raw_ostream &OS, StringRef RemarksPasses,
+    StringRef RemarksFormat, bool RemarksWithHotness,
+    Optional<uint64_t> RemarksHotnessThreshold) {
   if (RemarksWithHotness)
     Context.setDiagnosticsHotnessRequested(true);
 

llvm/lib/LTO/LTO.cpp

@@ -983,7 +983,8 @@ Error LTO::runRegularLTO(AddStreamFn AddStream) {
   // Setup optimization remarks.
   auto DiagFileOrErr = lto::setupLLVMOptimizationRemarks(
       RegularLTO.CombinedModule->getContext(), Conf.RemarksFilename,
-      Conf.RemarksPasses, Conf.RemarksFormat, Conf.RemarksWithHotness);
+      Conf.RemarksPasses, Conf.RemarksFormat, Conf.RemarksWithHotness,
+      Conf.RemarksHotnessThreshold);
   if (!DiagFileOrErr)
     return DiagFileOrErr.takeError();
 
@@ -1488,7 +1489,8 @@ Error LTO::runThinLTO(AddStreamFn AddStream, NativeObjectCache Cache,
 
 Expected<std::unique_ptr<ToolOutputFile>> lto::setupLLVMOptimizationRemarks(
     LLVMContext &Context, StringRef RemarksFilename, StringRef RemarksPasses,
-    StringRef RemarksFormat, bool RemarksWithHotness, int Count) {
+    StringRef RemarksFormat, bool RemarksWithHotness,
+    Optional<uint64_t> RemarksHotnessThreshold, int Count) {
   std::string Filename = std::string(RemarksFilename);
   // For ThinLTO, file.opt.<format> becomes
   // file.opt.<format>.thin.<num>.<format>.
@@ -1498,7 +1500,8 @@ Expected<std::unique_ptr<ToolOutputFile>> lto::setupLLVMOptimizationRemarks(
             .str();
 
   auto ResultOrErr = llvm::setupLLVMOptimizationRemarks(
-      Context, Filename, RemarksPasses, RemarksFormat, RemarksWithHotness);
+      Context, Filename, RemarksPasses, RemarksFormat, RemarksWithHotness,
+      RemarksHotnessThreshold);
   if (Error E = ResultOrErr.takeError())
     return std::move(E);
 

llvm/lib/LTO/LTOBackend.cpp

@@ -574,7 +574,8 @@ Error lto::thinBackend(const Config &Conf, unsigned Task, AddStreamFn AddStream,
   // Setup optimization remarks.
   auto DiagFileOrErr = lto::setupLLVMOptimizationRemarks(
       Mod.getContext(), Conf.RemarksFilename, Conf.RemarksPasses,
-      Conf.RemarksFormat, Conf.RemarksWithHotness, Task);
+      Conf.RemarksFormat, Conf.RemarksWithHotness, Conf.RemarksHotnessThreshold,
+      Task);
   if (!DiagFileOrErr)
     return DiagFileOrErr.takeError();
   auto DiagnosticOutputFile = std::move(*DiagFileOrErr);