[LTO] Support filtering by hotness threshold

[anemet]

Summary

This wires up -pass-remarks-hotness-threshold to LTO and ThinLTO.

Next is to change the clang driver to pass this
with -fdiagnostics-hotness-threshold.

[anemet]

Ping

[mehdi_amini]

LGTM

test/ThinLTO/X86/diagnostic-handler-remarks-with-hotness.ll

21 ↗

(On Diff #127784)

The pipe and the line break on the last line don't make much sense to me?

[inglorion]

I reverted this commit in r326975 because it failed ThinLTO/X86/diagnostic-handler-remarks-with-hotness.ll.

FAIL: LLVM :: ThinLTO/X86/diagnostic-handler-remarks-with-hotness.ll (1 of 1)
******************** TEST 'LLVM :: ThinLTO/X86/diagnostic-handler-remarks-with-hotness.ll' FAILED ********************
Script:
--
/usr/local/google/home/inglorion/clang/bin/opt -module-summary /usr/local/google/home/inglorion/llvm/llvm/test/ThinLTO/X86/diagnostic-handle
r-remarks-with-hotness.ll -o /usr/local/google/home/inglorion/clang/test/ThinLTO/X86/Output/diagnostic-handler-remarks-with-hotness.ll.tmp1.bc
/usr/local/google/home/inglorion/clang/bin/opt -module-summary /usr/local/google/home/inglorion/llvm/llvm/test/ThinLTO/X86/Inputs/diagnostic
-handler-remarks.ll -o /usr/local/google/home/inglorion/clang/test/ThinLTO/X86/Output/diagnostic-handler-remarks-with-hotness.ll.tmp2.bc
/usr/local/google/home/inglorion/clang/bin/llvm-lto -thinlto-action=run           -lto-pass-remarks-output=/usr/local/google/home/inglorion/clang/test/ThinLTO/X86/Output/diagnostic-handler-remarks-with-hotness.ll.tmp-no-th.yaml           -lto-pass-remarks-with-hotness           -
exported-symbol _func2           -exported-symbol _main /usr/local/google/home/inglorion/clang/test/ThinLTO/X86/Output/diagnostic-handler-re
marks-with-hotness.ll.tmp1.bc /usr/local/google/home/inglorion/clang/test/ThinLTO/X86/Output/diagnostic-handler-remarks-with-hotness.ll.tmp2.bc 2>&1 |      /usr/local/google/home/inglorion/clang/bin/FileCheck /usr/local/google/home/inglorion/llvm/llvm/test/ThinLTO/X86/diagnostic-
handler-remarks-with-hotness.ll -allow-empty
/usr/local/google/home/inglorion/clang/bin/llvm-lto -thinlto-action=run           -lto-pass-remarks-output=/usr/local/google/home/inglorion/clang/test/ThinLTO/X86/Output/diagnostic-handler-remarks-with-hotness.ll.tmp-low-th.yaml           -lto-pass-remarks-with-hotness
-lto-pass-remarks-hotness-threshold=20           -exported-symbol _func2           -exported-symbol _main /usr/local/google/home/inglorion/c
lang/test/ThinLTO/X86/Output/diagnostic-handler-remarks-with-hotness.ll.tmp1.bc /usr/local/google/home/inglorion/clang/test/ThinLTO/X86/Outp
ut/diagnostic-handler-remarks-with-hotness.ll.tmp2.bc 2>&1 |  /usr/local/google/home/inglorion/clang/bin/llvm-lto -thinlto-action=run
    -lto-pass-remarks-output=/usr/local/google/home/inglorion/clang/test/ThinLTO/X86/Output/diagnostic-handler-remarks-with-hotness.ll.tmp-h
igh-th.yaml           -lto-pass-remarks-with-hotness           -lto-pass-remarks-hotness-threshold=100           -exported-symbol _func2
       -exported-symbol _main /usr/local/google/home/inglorion/clang/test/ThinLTO/X86/Output/diagnostic-handler-remarks-with-hotness.ll.tmp1
.bc /usr/local/google/home/inglorion/clang/test/ThinLTO/X86/Output/diagnostic-handler-remarks-with-hotness.ll.tmp2.bc 2>&1 |  cat /usr/local
/google/home/inglorion/clang/test/ThinLTO/X86/Output/diagnostic-handler-remarks-with-hotness.ll.tmp-no-th.yaml.thin.0.yaml | /usr/local/goog
le/home/inglorion/clang/bin/FileCheck /usr/local/google/home/inglorion/llvm/llvm/test/ThinLTO/X86/diagnostic-handler-remarks-with-hotness.ll
 -check-prefix=YAML1
cat /usr/local/google/home/inglorion/clang/test/ThinLTO/X86/Output/diagnostic-handler-remarks-with-hotness.ll.tmp-low-th.yaml.thin.0.yaml |
/usr/local/google/home/inglorion/clang/bin/FileCheck /usr/local/google/home/inglorion/llvm/llvm/test/ThinLTO/X86/diagnostic-handler-remarks-
with-hotness.ll -check-prefix=YAML1
cat /usr/local/google/home/inglorion/clang/test/ThinLTO/X86/Output/diagnostic-handler-remarks-with-hotness.ll.tmp-high-th.yaml.thin.0.yaml |
 /usr/local/google/home/inglorion/clang/bin/FileCheck /usr/local/google/home/inglorion/llvm/llvm/test/ThinLTO/X86/diagnostic-handler-remarks
-with-hotness.ll -allow-empty -check-prefix=YAML1_TH_HIGH
cat /usr/local/google/home/inglorion/clang/test/ThinLTO/X86/Output/diagnostic-handler-remarks-with-hotness.ll.tmp-no-th.yaml.thin.1.yaml | /
usr/local/google/home/inglorion/clang/bin/FileCheck /usr/local/google/home/inglorion/llvm/llvm/test/ThinLTO/X86/diagnostic-handler-remarks-w
ith-hotness.ll -check-prefix=YAML2
cat /usr/local/google/home/inglorion/clang/test/ThinLTO/X86/Output/diagnostic-handler-remarks-with-hotness.ll.tmp-low-th.yaml.thin.1.yaml |
/usr/local/google/home/inglorion/clang/bin/FileCheck /usr/local/google/home/inglorion/llvm/llvm/test/ThinLTO/X86/diagnostic-handler-remarks-
with-hotness.ll -allow-empty -check-prefix=YAML2_TH_HIGH
cat /usr/local/google/home/inglorion/clang/test/ThinLTO/X86/Output/diagnostic-handler-remarks-with-hotness.ll.tmp-high-th.yaml.thin.1.yaml |
 /usr/local/google/home/inglorion/clang/bin/FileCheck /usr/local/google/home/inglorion/llvm/llvm/test/ThinLTO/X86/diagnostic-handler-remarks
-with-hotness.ll -allow-empty -check-prefix=YAML2_TH_HIGH
--
Exit Code: 1

[anemet]

@inglorion Is this from a bot, I didn't see any failures? A bit more info would be helpful.

[llvm-commits]

Is this from a bot? A bit more info would be helpful.

[anemet]

@inglorion, I am inclined to recommit this unless I hear from you in a few days:

1. you reverted without the usual prior notice to provide time to fix
2. looks like this does not happen on a public bot
3. you failed to provide sufficient information to reproduce.

Hope to hear from you.

Adam

llvm/trunk/include/llvm/LTO/Config.h

struct Config {
   std::string SampleProfile;
 
   /// Optimization remarks file path.
   std::string RemarksFilename = "";
 
   /// 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.
+  unsigned RemarksHotnessThreshold = 0;
+
   /// Whether to emit the pass manager debuggging informations.
   bool DebugPassManager = false;
 
   bool ShouldDiscardValueNames = true;
   DiagnosticHandlerFunction DiagHandler;
 
   /// If this field is set, LTO will write input file paths and symbol
   /// resolutions here in llvm-lto2 command line flag format. This can be

llvm/trunk/include/llvm/LTO/LTO.h

 /// resulting directory if it does not yet exist.
 std::string getThinLTOOutputFile(const std::string &Path,
                                  const std::string &OldPrefix,
                                  const std::string &NewPrefix);
 
 /// Setup optimization remarks.
 Expected<std::unique_ptr<ToolOutputFile>>
 setupOptimizationRemarks(LLVMContext &Context, StringRef LTORemarksFilename,
-                         bool LTOPassRemarksWithHotness, int Count = -1);
+                         bool LTOPassRemarksWithHotness,
+                         unsigned LTOPassRemarksHotnessThreshold,
+                         int Count = -1);
 
 class LTO;
 struct SymbolResolution;
 class ThinBackendProc;
 
 /// An input file. This is a symbol table wrapper that only exposes the
 /// information that an LTO client should need in order to do symbol resolution.
 class InputFile {

llvm/trunk/lib/LTO/LTO.cpp

if (Error E = BackendProc->start(Task, Mod.second, ImportLists[Mod.first],
                                      ResolvedODR[Mod.first], ThinLTO.ModuleMap))
       return E;
     ++Task;
   }
 
   return BackendProc->wait();
 }
 
-Expected<std::unique_ptr<ToolOutputFile>>
-lto::setupOptimizationRemarks(LLVMContext &Context,
-                              StringRef LTORemarksFilename,
-                              bool LTOPassRemarksWithHotness, int Count) {
+Expected<std::unique_ptr<ToolOutputFile>> lto::setupOptimizationRemarks(
+    LLVMContext &Context, StringRef LTORemarksFilename,
+    bool LTOPassRemarksWithHotness, unsigned LTOPassRemarksHotnessThreshold,
+    int Count) {
   if (LTORemarksFilename.empty())
     return nullptr;
 
   std::string Filename = LTORemarksFilename;
   if (Count != -1)
     Filename += ".thin." + llvm::utostr(Count) + ".yaml";
 
   std::error_code EC;
   auto DiagnosticFile =
       llvm::make_unique<ToolOutputFile>(Filename, EC, sys::fs::F_None);
   if (EC)
     return errorCodeToError(EC);
   Context.setDiagnosticsOutputFile(
       llvm::make_unique<yaml::Output>(DiagnosticFile->os()));
   if (LTOPassRemarksWithHotness)
     Context.setDiagnosticsHotnessRequested(true);
+  if (LTOPassRemarksHotnessThreshold)
+    Context.setDiagnosticsHotnessThreshold(LTOPassRemarksHotnessThreshold);
   DiagnosticFile->keep();
   return std::move(DiagnosticFile);
 }

llvm/trunk/lib/LTO/LTOBackend.cpp

Error lto::backend(Config &C, AddStreamFn AddStream,
   Expected<const Target *> TOrErr = initAndLookupTarget(C, *Mod);
   if (!TOrErr)
     return TOrErr.takeError();
 
   std::unique_ptr<TargetMachine> TM = createTargetMachine(C, *TOrErr, *Mod);
 
   // Setup optimization remarks.
   auto DiagFileOrErr = lto::setupOptimizationRemarks(
-      Mod->getContext(), C.RemarksFilename, C.RemarksWithHotness);
+      Mod->getContext(), C.RemarksFilename, C.RemarksWithHotness,
+      C.RemarksHotnessThreshold);
   if (!DiagFileOrErr)
     return DiagFileOrErr.takeError();
   auto DiagnosticOutputFile = std::move(*DiagFileOrErr);
 
   if (!C.CodeGenOnly) {
     if (!opt(C, TM.get(), 0, *Mod, /*IsThinLTO=*/false,
              /*ExportSummary=*/&CombinedIndex, /*ImportSummary=*/nullptr)) {
       finalizeOptimizationRemarks(std::move(DiagnosticOutputFile));

llvm/trunk/lib/LTO/LTOCodeGenerator.cpp

cl::opt<std::string>
     LTORemarksFilename("lto-pass-remarks-output",
                        cl::desc("Output filename for pass remarks"),
                        cl::value_desc("filename"));
 
 cl::opt<bool> LTOPassRemarksWithHotness(
     "lto-pass-remarks-with-hotness",
     cl::desc("With PGO, include profile count in optimization remarks"),
     cl::Hidden);
+
+cl::opt<unsigned> LTOPassRemarksHotnessThreshold(
+    "lto-pass-remarks-hotness-threshold",
+    cl::desc("Minimum profile count required for an optimization remark to be "
+             "output"),
+    cl::Hidden);
 }
 
 LTOCodeGenerator::LTOCodeGenerator(LLVMContext &Context)
     : Context(Context), MergedModule(new Module("ld-temp.o", Context)),
       TheLinker(new Linker(*MergedModule)) {
   Context.setDiscardValueNames(LTODiscardValueNames);
   Context.enableDebugTypeODRUniquing();
   initializeLTOPasses();

 /// Optimize merged modules using various IPO passes
 bool LTOCodeGenerator::optimize(bool DisableVerify, bool DisableInline,
                                 bool DisableGVNLoadPRE,
                                 bool DisableVectorization) {
   if (!this->determineTarget())
     return false;
 
   auto DiagFileOrErr = lto::setupOptimizationRemarks(
-      Context, LTORemarksFilename, LTOPassRemarksWithHotness);
+      Context, LTORemarksFilename, LTOPassRemarksWithHotness,
+      LTOPassRemarksHotnessThreshold);
   if (!DiagFileOrErr) {
     errs() << "Error: " << toString(DiagFileOrErr.takeError()) << "\n";
     report_fatal_error("Can't get an output file for the remarks");
   }
   DiagnosticOutputFile = std::move(*DiagFileOrErr);
 
   // We always run the verifier once on the merged module, the `DisableVerify`
   // parameter only applies to subsequent verify.

llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp

 
 #define DEBUG_TYPE "thinlto"
 
 namespace llvm {
 // Flags -discard-value-names, defined in LTOCodeGenerator.cpp
 extern cl::opt<bool> LTODiscardValueNames;
 extern cl::opt<std::string> LTORemarksFilename;
 extern cl::opt<bool> LTOPassRemarksWithHotness;
+extern cl::opt<unsigned> LTOPassRemarksHotnessThreshold;
 }
 
 namespace {
 
 static cl::opt<int>
     ThreadCount("threads", cl::init(llvm::heavyweight_hardware_concurrency()));
 
 // Simple helper to save temporary files for debug.
for (auto IndexCount : ModulesOrdering) {
             return;
           }
         }
 
         LLVMContext Context;
         Context.setDiscardValueNames(LTODiscardValueNames);
         Context.enableDebugTypeODRUniquing();
         auto DiagFileOrErr = lto::setupOptimizationRemarks(
-            Context, LTORemarksFilename, LTOPassRemarksWithHotness, count);
+            Context, LTORemarksFilename, LTOPassRemarksWithHotness,
+            LTOPassRemarksHotnessThreshold, count);
         if (!DiagFileOrErr) {
           errs() << "Error: " << toString(DiagFileOrErr.takeError()) << "\n";
           report_fatal_error("ThinLTO: Can't get an output file for the "
                              "remarks");
         }
 
         // Parse module now
         auto TheModule =

llvm/trunk/test/LTO/X86/diagnostic-handler-remarks-with-hotness.ll

 ; Check that the hotness attribute is included in the optimization record file
 ; with -lto-pass-remarks-with-hotness.
 
 ; RUN: llvm-as < %s >%t.bc
 ; RUN: rm -f %t.yaml
 ; RUN: llvm-lto -lto-pass-remarks-output=%t.yaml \
 ; RUN:          -lto-pass-remarks-with-hotness \
 ; RUN:          -exported-symbol _main -o %t.o %t.bc
 ; RUN: cat %t.yaml | FileCheck -check-prefix=YAML %s
 
+; RUN: llvm-lto -lto-pass-remarks-output=%t.yaml \
+; RUN:          -lto-pass-remarks-with-hotness \
+; RUN:          -lto-pass-remarks-hotness-threshold=400 \
+; RUN:          -exported-symbol _main -o %t.o %t.bc
+; RUN: cat %t.yaml | FileCheck -allow-empty -check-prefix=YAML_TH_HIGH %s
+
+; RUN: llvm-lto -lto-pass-remarks-output=%t.yaml \
+; RUN:          -lto-pass-remarks-with-hotness \
+; RUN:          -lto-pass-remarks-hotness-threshold=200 \
+; RUN:          -exported-symbol _main -o %t.o %t.bc
+; RUN: cat %t.yaml | FileCheck -allow-empty -check-prefix=YAML %s
+
 ; YAML:      --- !Passed
 ; YAML-NEXT: Pass:            inline
 ; YAML-NEXT: Name:            Inlined
 ; YAML-NEXT: Function:        main
 ; YAML-NEXT: Hotness:         300
 ; YAML-NEXT: Args:
 ; YAML-NEXT:   - Callee:          foo
 ; YAML-NEXT:   - String:          ' inlined into '
 ; YAML-NEXT:   - Caller:          main
 ; YAML-NEXT:   - String:          ' with cost='
 ; YAML-NEXT:   - Cost:            '-15000'
 ; YAML-NEXT:   - String:          ' (threshold='
 ; YAML-NEXT:   - Threshold:       '337'
 ; YAML-NEXT:   - String:          ')'
 ; YAML-NEXT: ...
 
+; YAML_TH_HIGH-NOT: Name:            Inlined
+
+
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-darwin"
 
 declare i32 @bar()
 
 define i32 @foo() {
   %a = call i32 @bar()
   ret i32 %a
 }
 
 define i32 @main() !prof !0 {
   %i = call i32 @foo()
   ret i32 %i
 }
 
 !0 = !{!"function_entry_count", i64 300}

llvm/trunk/test/ThinLTO/X86/diagnostic-handler-remarks-with-hotness.ll

 ; RUN: opt -module-summary %s -o %t1.bc
 ; RUN: opt -module-summary %p/Inputs/diagnostic-handler-remarks.ll -o %t2.bc
 
 ; Check that the hotness attribute is included in the optimization record file
 ; with -lto-pass-remarks-with-hotness.
 
 ; RUN: llvm-lto -thinlto-action=run \
-; RUN:          -lto-pass-remarks-output=%t.yaml \
+; RUN:          -lto-pass-remarks-output=%t-no-th.yaml \
 ; RUN:          -lto-pass-remarks-with-hotness \
 ; RUN:          -exported-symbol _func2 \
 ; RUN:          -exported-symbol _main %t1.bc %t2.bc 2>&1 | \
 ; RUN:     FileCheck %s -allow-empty
 ; CHECK-NOT: remark:
 ; CHECK-NOT: llvm-lto:
 
+; RUN: llvm-lto -thinlto-action=run \
+; RUN:          -lto-pass-remarks-output=%t-low-th.yaml \
+; RUN:          -lto-pass-remarks-with-hotness \
+; RUN:          -lto-pass-remarks-hotness-threshold=20 \
+; RUN:          -exported-symbol _func2 \
+; RUN:          -exported-symbol _main %t1.bc %t2.bc 2>&1 | \
+
+; RUN: llvm-lto -thinlto-action=run \
+; RUN:          -lto-pass-remarks-output=%t-high-th.yaml \
+; RUN:          -lto-pass-remarks-with-hotness \
+; RUN:          -lto-pass-remarks-hotness-threshold=100 \
+; RUN:          -exported-symbol _func2 \
+; RUN:          -exported-symbol _main %t1.bc %t2.bc 2>&1 | \
+
 
 ; Verify that bar is imported and inlined into foo
-; RUN: cat %t.yaml.thin.0.yaml | FileCheck %s -check-prefix=YAML1
+; RUN: cat %t-no-th.yaml.thin.0.yaml | FileCheck %s -check-prefix=YAML1
+; RUN: cat %t-low-th.yaml.thin.0.yaml | FileCheck %s -check-prefix=YAML1
+; RUN: cat %t-high-th.yaml.thin.0.yaml | FileCheck %s -allow-empty -check-prefix=YAML1_TH_HIGH
 ; YAML1:      --- !Passed
 ; YAML1-NEXT: Pass:            inline
 ; YAML1-NEXT: Name:            Inlined
 ; YAML1-NEXT: Function:        main
 ; YAML1-NEXT: Hotness:         50
 ; YAML1-NEXT: Args:
 ; YAML1-NEXT:   - Callee:          foo
 ; YAML1-NEXT:   - String:          ' inlined into '
 ; YAML1-NEXT:   - Caller:          main
 ; YAML1-NEXT:   - String:          ' with cost='
 ; YAML1-NEXT:   - Cost:            '-30'
 ; YAML1-NEXT:   - String:          ' (threshold='
 ; YAML1-NEXT:   - Threshold:       '337'
 ; YAML1-NEXT:   - String:          ')'
 ; YAML1-NEXT: ...
 
+; YAML1_TH_HIGH-NOT: Name:            Inlined
+
 
 ; Verify that bar is imported and inlined into foo
-; RUN: cat %t.yaml.thin.1.yaml | FileCheck %s -check-prefix=YAML2
+; RUN: cat %t-no-th.yaml.thin.1.yaml | FileCheck %s -check-prefix=YAML2
+; RUN: cat %t-low-th.yaml.thin.1.yaml | FileCheck %s -allow-empty -check-prefix=YAML2_TH_HIGH
+; RUN: cat %t-high-th.yaml.thin.1.yaml | FileCheck %s -allow-empty -check-prefix=YAML2_TH_HIGH
 ; YAML2:      --- !Passed
 ; YAML2-NEXT: Pass:            inline
 ; YAML2-NEXT: Name:            Inlined
 ; YAML2-NEXT: Function:        foo
 ; YAML2-NEXT: Args:
 ; YAML2-NEXT:   - Callee:          bar
 ; YAML2-NEXT:   - String:          ' inlined into '
 ; YAML2-NEXT:   - Caller:          foo
 ; YAML2-NEXT:   - String:          ' with cost='
 ; YAML2-NEXT:   - Cost:            '-30'
 ; YAML2-NEXT:   - String:          ' (threshold='
 ; YAML2-NEXT:   - Threshold:       '337'
 ; YAML2-NEXT:   - String:          ')'
 ; YAML2-NEXT: ...
 
+; YAML2_TH_HIGH-NOT: Name:            Inlined
 
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.11.0"
 
 define i32 @bar() {
   ret i32 42
 }
 declare i32 @foo()
 define i32 @main() !prof !0 {
   %i = call i32 @foo()
   ret i32 %i
 }
 
 !0 = !{!"function_entry_count", i64 50}