[ORE] Add diagnostics hotness threshold

[modocache]

Summary

Add an option to prevent 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!

include/llvm/IR/LLVMContext.h

196–199

What's the difference between None and 0?

lib/Analysis/OptimizationDiagnosticInfo.cpp

166–172

You need to add the same code to MachineOptimizationRemarkEmitter::emit() too.

[modocache]

include/llvm/IR/LLVMContext.h

196–199

I had originally thought that there should be a distinction between "the threshold has been set to zero" and "the threshold has not been set", but taking another look at this patch and the one to Clang, I realize there isn't a need for it. I'll remove the Optional<> here, good call!

lib/Analysis/OptimizationDiagnosticInfo.cpp

166–172

Oh, good catch, thank you! Do you know if there's an existing test for MachineOptimizationRemarkEmitter?

[anemet]

lib/Analysis/OptimizationDiagnosticInfo.cpp

166–172

There are a few, look for pass-remarks under test/CodeGen. The one I originally added is test/CodeGen/AArch64/arm64-spill-remarks.ll.

[modocache]

Add hotness threshold check to MachineOptimizationRemarkEmitter, and change the threshold attribute type for LLVMContext from an optional unsigned to just an unsigned.

[modocache]

Add tests for the MachineOptimizationRemarkEmitter codepath, to test/CodeGen/AArch64/arm64-spill-remarks.ll.

[anemet]

LGTM now, thanks!

[modocache]

Great, thank you!

include/llvm/IR/LLVMContext.h

public:
 
   /// \brief Return if a code hotness metric should be included in optimization
   /// diagnostics.
   bool getDiagnosticsHotnessRequested() const;
   /// \brief Set if a code hotness metric should be included in optimization
   /// diagnostics.
   void setDiagnosticsHotnessRequested(bool Requested);
 
+  /// \brief 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.
+  uint64_t getDiagnosticsHotnessThreshold() const;
+
+  /// \brief Set the minimum hotness value a diagnostic needs in order to be
+  /// included in optimization diagnostics.
+  void setDiagnosticsHotnessThreshold(uint64_t Threshold);
+
   /// \brief Return the YAML file used by the backend to save optimization
   /// diagnostics.  If null, diagnostics are not saved in a file but only
   /// emitted via the diagnostic handler.
   yaml::Output *getDiagnosticsOutputFile();
   /// Set the diagnostics output file used for optimization diagnostics.
   ///
   /// By default or if invoked with null, diagnostics are not saved in a file
   /// but only emitted via the diagnostic handler.  Even if an output file is

lib/Analysis/OptimizationDiagnosticInfo.cpp

void OptimizationRemarkEmitter::computeHotness(
   if (V)
     OptDiag.setHotness(computeHotness(V));
 }
 
 void OptimizationRemarkEmitter::emit(
     DiagnosticInfoOptimizationBase &OptDiagBase) {
   auto &OptDiag = cast<DiagnosticInfoIROptimization>(OptDiagBase);
   computeHotness(OptDiag);
+  // If a diagnostic has a hotness value, then only emit it if its hotness
+  // meets the threshold.
+  if (OptDiag.getHotness() &&
+      *OptDiag.getHotness() <
+          F->getContext().getDiagnosticsHotnessThreshold()) {
+    return;
+  }
 
   yaml::Output *Out = F->getContext().getDiagnosticsOutputFile();
   if (Out) {
     auto *P = const_cast<DiagnosticInfoOptimizationBase *>(&OptDiagBase);
     *Out << P;
   }
   // FIXME: now that IsVerbose is part of DI, filtering for this will be moved
   // from here to clang.
   if (!OptDiag.isVerbose() || shouldEmitVerbose())
     F->getContext().diagnose(OptDiag);
 }
 
 OptimizationRemarkEmitterWrapperPass::OptimizationRemarkEmitterWrapperPass()
     : FunctionPass(ID) {
   initializeOptimizationRemarkEmitterWrapperPassPass(
       *PassRegistry::getPassRegistry());

lib/CodeGen/MachineOptimizationRemarkEmitter.cpp

 }
 
 void MachineOptimizationRemarkEmitter::emit(
     DiagnosticInfoOptimizationBase &OptDiagCommon) {
   auto &OptDiag = cast<DiagnosticInfoMIROptimization>(OptDiagCommon);
   computeHotness(OptDiag);
 
   LLVMContext &Ctx = MF.getFunction()->getContext();
+
+  // If a diagnostic has a hotness value, then only emit it if its hotness
+  // meets the threshold.
+  if (OptDiag.getHotness() &&
+      *OptDiag.getHotness() < Ctx.getDiagnosticsHotnessThreshold()) {
+    return;
+  }
+
   yaml::Output *Out = Ctx.getDiagnosticsOutputFile();
   if (Out) {
     auto *P = &const_cast<DiagnosticInfoOptimizationBase &>(OptDiagCommon);
     *Out << P;
   }
   // FIXME: now that IsVerbose is part of DI, filtering for this will be moved
   // from here to clang.
   if (!OptDiag.isVerbose() || shouldEmitVerbose())

lib/IR/LLVMContext.cpp

 
 void LLVMContext::setDiagnosticsHotnessRequested(bool Requested) {
   pImpl->DiagnosticsHotnessRequested = Requested;
 }
 bool LLVMContext::getDiagnosticsHotnessRequested() const {
   return pImpl->DiagnosticsHotnessRequested;
 }
 
+void LLVMContext::setDiagnosticsHotnessThreshold(uint64_t Threshold) {
+  pImpl->DiagnosticsHotnessThreshold = Threshold;
+}
+uint64_t LLVMContext::getDiagnosticsHotnessThreshold() const {
+  return pImpl->DiagnosticsHotnessThreshold;
+}
+
 yaml::Output *LLVMContext::getDiagnosticsOutputFile() {
   return pImpl->DiagnosticsOutputFile.get();
 }
 
 void LLVMContext::setDiagnosticsOutputFile(std::unique_ptr<yaml::Output> F) {
   pImpl->DiagnosticsOutputFile = std::move(F);
 }
 

lib/IR/LLVMContextImpl.h

public:
   
   LLVMContext::InlineAsmDiagHandlerTy InlineAsmDiagHandler = nullptr;
   void *InlineAsmDiagContext = nullptr;
 
   LLVMContext::DiagnosticHandlerTy DiagnosticHandler = nullptr;
   void *DiagnosticContext = nullptr;
   bool RespectDiagnosticFilters = false;
   bool DiagnosticsHotnessRequested = false;
+  uint64_t DiagnosticsHotnessThreshold = 0;
   std::unique_ptr<yaml::Output> DiagnosticsOutputFile;
 
   LLVMContext::YieldCallbackTy YieldCallback = nullptr;
   void *YieldOpaqueHandle = nullptr;
 
   using IntMapTy =
       DenseMap<APInt, std::unique_ptr<ConstantInt>, DenseMapAPIntKeyInfo>;
   IntMapTy IntConstants;

test/CodeGen/AArch64/arm64-spill-remarks.ll

 ; RUN: llc < %s -mtriple=arm64-apple-ios7.0 -aarch64-neon-syntax=apple -pass-remarks-missed=regalloc 2>&1 | FileCheck -check-prefix=REMARK %s
 ; RUN: llc < %s -mtriple=arm64-apple-ios7.0 -aarch64-neon-syntax=apple -pass-remarks-missed=regalloc -pass-remarks-with-hotness 2>&1 | FileCheck -check-prefix=HOTNESS %s
 ; RUN: llc < %s -mtriple=arm64-apple-ios7.0 -aarch64-neon-syntax=apple 2>&1 | FileCheck -check-prefix=NO_REMARK %s
 ; RUN: llc < %s -mtriple=arm64-apple-ios7.0 -aarch64-neon-syntax=apple -pass-remarks-output=%t.yaml -pass-remarks-with-hotness 2>&1 | FileCheck -check-prefix=NO_REMARK %s
 ; RUN: cat %t.yaml | FileCheck -check-prefix=YAML %s
+;
+; Verify that remarks below the hotness threshold are not output.
+; RUN: llc < %s -mtriple=arm64-apple-ios7.0 -aarch64-neon-syntax=apple -pass-remarks-missed=regalloc \
+; RUN:       -pass-remarks-with-hotness -pass-remarks-hotness-threshold=500 \
+; RUN:       2>&1 | FileCheck -check-prefix=THRESHOLD %s
+; RUN: llc < %s -mtriple=arm64-apple-ios7.0 -aarch64-neon-syntax=apple -pass-remarks-output=%t.threshold.yaml \
+; RUN:       -pass-remarks-with-hotness -pass-remarks-hotness-threshold=500 \
+; RUN:       2>&1 | FileCheck -check-prefix=NO_REMARK %s
+; RUN: cat %t.threshold.yaml | FileCheck -check-prefix=THRESHOLD_YAML %s
 
 ; This has two nested loops, each with one value that has to be spilled and
 ; then reloaded.
 
 ; (loop3:)
 ; REMARK: remark: /tmp/kk.c:3:20: 1 spills 1 reloads generated in loop{{$}}
 ; (loop2:)
 ; REMARK: remark: /tmp/kk.c:2:20: 1 spills 1 reloads generated in loop{{$}}
 ; (loop:)
 ; REMARK: remark: /tmp/kk.c:1:20: 2 spills 2 reloads generated in loop{{$}}
 
 ; (loop3:)
 ; HOTNESS: remark: /tmp/kk.c:3:20: 1 spills 1 reloads generated in loop (hotness: 300)
 ; (loop2:)
 ; HOTNESS: remark: /tmp/kk.c:2:20: 1 spills 1 reloads generated in loop (hotness: 30000)
 ; (loop:)
 ; HOTNESS: remark: /tmp/kk.c:1:20: 2 spills 2 reloads generated in loop (hotness: 300)
 
 ; NO_REMARK-NOT: remark
 
+; THRESHOLD-NOT: (hotness: 300)
+; THRESHOLD: remark: /tmp/kk.c:2:20: 1 spills 1 reloads generated in loop (hotness: 30000)
+
 ; YAML: --- !Missed
 ; YAML: Pass:            regalloc
 ; YAML: Name:            LoopSpillReload
 ; YAML: DebugLoc:        { File: /tmp/kk.c, Line: 3, Column: 20 }
 ; YAML: Function:        fpr128
 ; YAML: Hotness:         300
 ; YAML: Args:
 ; YAML:   - NumSpills:       '1'

 ; YAML: Args:
 ; YAML:   - NumSpills:       '2'
 ; YAML:   - String:          ' spills '
 ; YAML:   - NumReloads:      '2'
 ; YAML:   - String:          ' reloads '
 ; YAML:   - String:          generated in loop
 ; YAML: ...
 
+; THRESHOLD_YAML-NOT: Hotness:         300{{$}}
+; THRESHOLD_YAML: --- !Missed
+; THRESHOLD_YAML: Pass:            regalloc
+; THRESHOLD_YAML: Name:            LoopSpillReload
+; THRESHOLD_YAML: DebugLoc:        { File: /tmp/kk.c, Line: 2, Column: 20 }
+; THRESHOLD_YAML: Function:        fpr128
+; THRESHOLD_YAML: Hotness:         30000
+; THRESHOLD_YAML: Args:
+; THRESHOLD_YAML:   - NumSpills:       '1'
+; THRESHOLD_YAML:   - String:          ' spills '
+; THRESHOLD_YAML:   - NumReloads:      '1'
+; THRESHOLD_YAML:   - String:          ' reloads '
+; THRESHOLD_YAML:   - String:          generated in loop
+; THRESHOLD_YAML: ...
+
 define void @fpr128(<4 x float>* %p) nounwind ssp !prof !11 {
 entry:
   br label %loop, !dbg !8
 
 loop:
   %i = phi i32 [ 0, %entry], [ %i.2, %end2 ]
   br label %loop2, !dbg !9
 

test/Transforms/Inline/optimization-remarks-yaml.ll

-; RUN: opt < %s -S -inline -pass-remarks-missed=inline -pass-remarks-with-hotness \
+; RUN: opt < %s -S -inline -pass-remarks-missed=inline \
+; RUN:     -pass-remarks-with-hotness -pass-remarks-hotness-threshold 15 \
 ; RUN:     -pass-remarks-output=%t 2>&1 | FileCheck %s
 ; RUN: cat %t | FileCheck -check-prefix=YAML %s
 ; RUN: opt < %s -S -inline -pass-remarks-with-hotness -pass-remarks-output=%t
 ; RUN: cat %t | FileCheck -check-prefix=YAML %s
+;
+; Verify that remarks that don't meet the hotness threshold are not output.
+; RUN: opt < %s -S -inline -pass-remarks-missed=inline \
+; RUN:     -pass-remarks-with-hotness -pass-remarks-hotness-threshold 100 \
+; RUN:     -pass-remarks-output=%t.threshold 2>&1 | \
+; RUN:     FileCheck -check-prefix=THRESHOLD %s
+; RUN: test ! -s %t.threshold
+; RUN: opt < %s -S -inline \
+; RUN:     -pass-remarks-with-hotness -pass-remarks-hotness-threshold 100 \
+; RUN:     -pass-remarks-output=%t.threshold
+; The remarks output file should be empty.
+; RUN: test ! -s %t.threshold
 
 ; Check the YAML file generated for inliner remarks for this program:
 ;
 ;   1  int foo();
 ;   2  int bar();
 ;   3
 ;   4  int baz() {
 ;   5    return foo() + bar();

 ; YAML-NEXT: Args:
 ; YAML-NEXT:   - Callee: bar
 ; YAML-NEXT:   - String: ' will not be inlined into '
 ; YAML-NEXT:   - Caller: baz
 ; YAML-NEXT:     DebugLoc:        { File: /tmp/s.c, Line: 4, Column: 0 }
 ; YAML-NEXT:   - String: ' because its definition is unavailable'
 ; YAML-NEXT: ...
 
+; No remarks should be output, since none meet the threshold.
+; THRESHOLD-NOT: remark
+
 ; ModuleID = '/tmp/s.c'
 source_filename = "/tmp/s.c"
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.11.0"
 
 ; Function Attrs: nounwind ssp uwtable
 define i32 @"\01baz"() !dbg !7 !prof !14 {
 entry:

tools/llc/llc.cpp

 
 static cl::list<std::string> IncludeDirs("I", cl::desc("include search path"));
 
 static cl::opt<bool> PassRemarksWithHotness(
     "pass-remarks-with-hotness",
     cl::desc("With PGO, include profile count in optimization remarks"),
     cl::Hidden);
 
+static cl::opt<unsigned> PassRemarksHotnessThreshold(
+    "pass-remarks-hotness-threshold",
+    cl::desc("Minimum profile count required for an optimization remark to be output"),
+    cl::Hidden);
+
 static cl::opt<std::string>
     RemarksFilename("pass-remarks-output",
                     cl::desc("YAML output filename for pass remarks"),
                     cl::value_desc("filename"));
 
 namespace {
 static ManagedStatic<std::vector<std::string>> RunPassNames;
 
int main(int argc, char **argv) {
   // Set a diagnostic handler that doesn't exit on the first error
   bool HasError = false;
   Context.setDiagnosticHandler(DiagnosticHandler, &HasError);
   Context.setInlineAsmDiagnosticHandler(InlineAsmDiagHandler, &HasError);
 
   if (PassRemarksWithHotness)
     Context.setDiagnosticsHotnessRequested(true);
 
+  if (PassRemarksHotnessThreshold)
+    Context.setDiagnosticsHotnessThreshold(PassRemarksHotnessThreshold);
+
   std::unique_ptr<tool_output_file> YamlFile;
   if (RemarksFilename != "") {
     std::error_code EC;
     YamlFile = llvm::make_unique<tool_output_file>(RemarksFilename, EC,
                                                    sys::fs::F_None);
     if (EC) {
       errs() << EC.message() << '\n';
       return 1;

tools/opt/opt.cpp

static cl::opt<bool> Coroutines(
   cl::desc("Enable coroutine passes."),
   cl::init(false), cl::Hidden);
 
 static cl::opt<bool> PassRemarksWithHotness(
     "pass-remarks-with-hotness",
     cl::desc("With PGO, include profile count in optimization remarks"),
     cl::Hidden);
 
+static cl::opt<unsigned> PassRemarksHotnessThreshold(
+    "pass-remarks-hotness-threshold",
+    cl::desc("Minimum profile count required for an optimization remark to be output"),
+    cl::Hidden);
+
 static cl::opt<std::string>
     RemarksFilename("pass-remarks-output",
                     cl::desc("YAML output filename for pass remarks"),
                     cl::value_desc("filename"));
 
 static inline void addPass(legacy::PassManagerBase &PM, Pass *P) {
   // Add the pass to the pass manager...
   PM.add(P);
#endif
 
   Context.setDiscardValueNames(DiscardValueNames);
   if (!DisableDITypeMap)
     Context.enableDebugTypeODRUniquing();
 
   if (PassRemarksWithHotness)
     Context.setDiagnosticsHotnessRequested(true);
 
+  if (PassRemarksHotnessThreshold)
+    Context.setDiagnosticsHotnessThreshold(PassRemarksHotnessThreshold);
+
   std::unique_ptr<tool_output_file> YamlFile;
   if (RemarksFilename != "") {
     std::error_code EC;
     YamlFile = llvm::make_unique<tool_output_file>(RemarksFilename, EC,
                                                    sys::fs::F_None);
     if (EC) {
       errs() << EC.message() << '\n';
       return 1;