[gold] Enable time trace profiler in LLVMgold #94293

mshockwave · 2024-06-03T23:47:38Z

To get the time trace of LTO in the gold plugin, now we can pass the time-trace=<time trace file> as well as
time-trace-granularity=<granularity> flags to LLVMgold.

Note that we still have to populate LTOConfig::TimeTraceEnabled and LTOConfig::TimeTraceGranularity because ThinLTO backend needs them.

To get the time trace of LTO in the gold plugin, we can pass the `time-trace=<time trace file>` as well as `time-trace-granularity=<granularity>` flags to LLVMgold.

llvmbot · 2024-06-03T23:48:13Z

@llvm/pr-subscribers-lto

Author: Min-Yih Hsu (mshockwave)

Changes

To get the time trace of LTO in the gold plugin, now we can pass the time-trace=<time trace file> as well as
time-trace-granularity=<granularity> flags to LLVMgold.

Note that we still have to populate LTOConfig::TimeTraceEnabled and LTOConfig::TimeTraceGranularity because ThinkLTO backend needs them.

Full diff: https://github.com/llvm/llvm-project/pull/94293.diff

2 Files Affected:

(added) llvm/test/tools/gold/X86/time-trace.ll (+32)
(modified) llvm/tools/gold/gold-plugin.cpp (+31)

diff --git a/llvm/test/tools/gold/X86/time-trace.ll b/llvm/test/tools/gold/X86/time-trace.ll
new file mode 100644
index 0000000000000..2707d1c1af6df
--- /dev/null
+++ b/llvm/test/tools/gold/X86/time-trace.ll
@@ -0,0 +1,32 @@
+; RUN: llvm-as %s -o %t.o
+
+; RUN: %gold -plugin %llvmshlibdir/LLVMgold%shlibext \
+; RUN:    -m elf_x86_64 --plugin-opt=time-trace=%t2.json \
+; RUN:    -shared %t.o -o %t3.s
+; RUN: FileCheck --input-file %t2.json %s
+
+; RUN: %gold -plugin %llvmshlibdir/LLVMgold%shlibext \
+; RUN:    -m elf_x86_64 --plugin-opt=time-trace=%t2.json \
+; RUN:    --plugin-opt=time-trace-granularity=250  \
+; RUN:    -shared %t.o -o %t3.s
+; RUN: FileCheck --input-file %t2.json %s
+
+; RUN: not %gold -plugin %llvmshlibdir/LLVMgold%shlibext \
+; RUN:    -m elf_x86_64 --plugin-opt=time-trace=%t2.json \
+; RUN:    --plugin-opt=time-trace-granularity=hello  \
+; RUN:    -shared %t.o -o %t3.s 2> %t4.txt
+; RUN: FileCheck --input-file %t4.txt %s --check-prefix=ERR
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @f1() {
+  ret void
+}
+
+define void @f2() {
+  ret void
+}
+
+; CHECK: "traceEvents":
+; ERR: Invalid time trace granularity: hello
diff --git a/llvm/tools/gold/gold-plugin.cpp b/llvm/tools/gold/gold-plugin.cpp
index 5503f7343cb67..265ebcbff5877 100644
--- a/llvm/tools/gold/gold-plugin.cpp
+++ b/llvm/tools/gold/gold-plugin.cpp
@@ -11,6 +11,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Bitcode/BitcodeReader.h"
 #include "llvm/Bitcode/BitcodeWriter.h"
@@ -31,6 +32,7 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/Threading.h"
+#include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/TargetParser/Host.h"
 #include <list>
@@ -220,6 +222,10 @@ namespace options {
   static std::string cs_profile_path;
   static bool cs_pgo_gen = false;
 
+  // Time trace options.
+  static std::string time_trace_file;
+  static unsigned time_trace_granularity = 500;
+
   static void process_plugin_option(const char *opt_)
   {
     if (opt_ == nullptr)
@@ -308,6 +314,14 @@ namespace options {
       RemarksFormat = std::string(opt);
     } else if (opt.consume_front("stats-file=")) {
       stats_file = std::string(opt);
+    } else if (opt.consume_front("time-trace=")) {
+      time_trace_file = std::string(opt);
+    } else if (opt.consume_front("time-trace-granularity=")) {
+      unsigned Granularity;
+      if (opt.getAsInteger(10, Granularity))
+        message(LDPL_FATAL, "Invalid time trace granularity: %s", opt);
+      else
+        time_trace_granularity = Granularity;
     } else {
       // Save this option to pass to the code generator.
       // ParseCommandLineOptions() expects argv[0] to be program name. Lazily
@@ -953,6 +967,10 @@ static std::unique_ptr<LTO> createLTO(IndexWriteCallback OnIndexWrite,
   Conf.HasWholeProgramVisibility = options::whole_program_visibility;
 
   Conf.StatsFile = options::stats_file;
+
+  Conf.TimeTraceEnabled = !options::time_trace_file.empty();
+  Conf.TimeTraceGranularity = options::time_trace_granularity;
+
   return std::make_unique<LTO>(std::move(Conf), Backend,
                                 options::ParallelCodeGenParallelismLevel);
 }
@@ -1113,6 +1131,19 @@ static ld_plugin_status allSymbolsReadHook() {
   if (unsigned NumOpts = options::extra.size())
     cl::ParseCommandLineOptions(NumOpts, &options::extra[0]);
 
+  // Initialize time trace profiler
+  if (!options::time_trace_file.empty())
+    llvm::timeTraceProfilerInitialize(options::time_trace_granularity,
+                                      options::extra.size() ? options::extra[0]
+                                                            : "LLVMgold");
+  auto FinalizeTimeTrace = llvm::make_scope_exit([&]() {
+    if (!llvm::timeTraceProfilerEnabled())
+      return;
+    assert(!options::time_trace_file.empty());
+    check(llvm::timeTraceProfilerWrite(options::time_trace_file, output_name));
+    llvm::timeTraceProfilerCleanup();
+  });
+
   std::vector<std::pair<SmallString<128>, bool>> Files = runLTO();
 
   if (options::TheOutputType == options::OT_DISABLE ||

teresajohnson

lgtm with a minor change below

teresajohnson · 2024-06-04T15:40:22Z

llvm/tools/gold/gold-plugin.cpp

+  if (!options::time_trace_file.empty())
+    llvm::timeTraceProfilerInitialize(options::time_trace_granularity,
+                                      options::extra.size() ? options::extra[0]
+                                                            : "LLVMgold");


At

llvm-project/llvm/tools/gold/gold-plugin.cpp

Line 316 in 6cd86d0

extra.push_back("LLVMgold");

we set extra[0] to "LLVMgold" if extra is empty, so this shouldn't be needed (extra should never be empty and extra[0] can always be used).

I think we reach this line (line 316) only if there are additional -plugin-opt that LLVMgold doesn't recognize. If there aren't any of those, I think options::extra will remain empty.

got it thanks

MaskRay · 2024-06-04T18:53:08Z

llvm/test/tools/gold/X86/time-trace.ll

+
+; RUN: %gold -plugin %llvmshlibdir/LLVMgold%shlibext \
+; RUN:    -m elf_x86_64 --plugin-opt=time-trace=%t2.json \
+; RUN:    -shared %t.o -o %t3.s


If the output is unused, prefer -o /dev/null; otherwise .so for shared objects.

It's fixed now.

tstellar · 2024-06-08T00:41:42Z

Hi @mshockwave we are seeing test failures on s390x with this change:
https://download.copr.fedorainfracloud.org/results/@fedora-llvm-team/llvm-snapshots-big-merge-20240607/fedora-39-s390x/07546351-llvm/builder-live.log.gz

nikic · 2024-06-11T08:28:38Z

llvm/tools/gold/gold-plugin.cpp

+    } else if (opt.consume_front("time-trace-granularity=")) {
+      unsigned Granularity;
+      if (opt.getAsInteger(10, Granularity))
+        message(LDPL_FATAL, "Invalid time trace granularity: %s", opt);


This is passing a StringRef to a varargs function looking for const char *. This is obviously wrong, but it looks like the ABI happens to work out fine on non-s390x architectures...

I've put up #95083 to fix this.

mshockwave · 2024-06-11T17:26:43Z

Hi @mshockwave we are seeing test failures on s390x with this change: https://download.copr.fedorainfracloud.org/results/@fedora-llvm-team/llvm-snapshots-big-merge-20240607/fedora-39-s390x/07546351-llvm/builder-live.log.gz

Sorry the notification must slipped thru my mailbox during the weekend. Thank you @nikic for the quick fix!

[gold] Enable time trace profiler in LLVMgold

289e7e0

To get the time trace of LTO in the gold plugin, we can pass the `time-trace=<time trace file>` as well as `time-trace-granularity=<granularity>` flags to LLVMgold.

mshockwave requested review from MaskRay and teresajohnson June 3, 2024 23:47

llvmbot added the LTO Link time optimization (regular/full LTO or ThinLTO) label Jun 3, 2024

teresajohnson approved these changes Jun 4, 2024

View reviewed changes

MaskRay reviewed Jun 4, 2024

View reviewed changes

MaskRay approved these changes Jun 4, 2024

View reviewed changes

fixup! [gold] Enable time trace profiler in LLVMgold

32dc5f3

mshockwave merged commit cd3255a into llvm:main Jun 4, 2024
4 of 6 checks passed

mshockwave deleted the patch/gold-time-trace branch June 4, 2024 21:01

nikic reviewed Jun 11, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[gold] Enable time trace profiler in LLVMgold #94293

[gold] Enable time trace profiler in LLVMgold #94293

mshockwave commented Jun 3, 2024 •

edited

Loading

llvmbot commented Jun 3, 2024

teresajohnson left a comment

teresajohnson Jun 4, 2024

mshockwave Jun 4, 2024

teresajohnson Jun 4, 2024

MaskRay Jun 4, 2024

mshockwave Jun 4, 2024

tstellar commented Jun 8, 2024

nikic Jun 11, 2024

nikic Jun 11, 2024

mshockwave commented Jun 11, 2024

[gold] Enable time trace profiler in LLVMgold #94293

[gold] Enable time trace profiler in LLVMgold #94293

Conversation

mshockwave commented Jun 3, 2024 • edited Loading

llvmbot commented Jun 3, 2024

teresajohnson left a comment

Choose a reason for hiding this comment

teresajohnson Jun 4, 2024

Choose a reason for hiding this comment

mshockwave Jun 4, 2024

Choose a reason for hiding this comment

teresajohnson Jun 4, 2024

Choose a reason for hiding this comment

MaskRay Jun 4, 2024

Choose a reason for hiding this comment

mshockwave Jun 4, 2024

Choose a reason for hiding this comment

tstellar commented Jun 8, 2024

nikic Jun 11, 2024

Choose a reason for hiding this comment

nikic Jun 11, 2024

Choose a reason for hiding this comment

mshockwave commented Jun 11, 2024

mshockwave commented Jun 3, 2024 •

edited

Loading