Non-debuginfo JITLink perf jitdump support

[pchintalapudi]

Summary

This patch ports PerfJITEventListener to a JITLink plugin, but adds unwind record support and drops debuginfo support temporarily. Debuginfo can be enabled in the future by providing a way to obtain a DWARFContext from a LinkGraph.

See D146060 for an experimental implementation that adds debuginfo parsing.

[pchintalapudi]

Add JITLink perf support without debuginfo record

This extracts the non-debuginfo changes from D146060 into its own revision, as debuginfo is more complicated currently.

[pchintalapudi]

Fix Windows compilation

[lhames]

This looks great -- thanks very much @pchintalapudi!

I love that you've gone for the full cross-process implementation -- that's amazing. Have you tested it in a cross-process setup with llvm-jitlink-executor? Is it really working out-of-process already?

I have a few requests, mostly cosmetic (though the licensing info should go in before this lands), but other than that LGTM.

llvm/include/llvm/ExecutionEngine/Orc/PerfSupportPlugin.h

1 ↗

(On Diff #506418)

Should be padded out to 80 cols with dashes either side of the name/description.

39–44 ↗

(On Diff #506418)

Do the perf JIT APIs support unloading code? If so these should have a // TODO to fill them in later. (I don't think they're needed for the initial commit).

135–140 ↗

(On Diff #506418)

In SimplePackedSerialization argument lists and tuples are guaranteed to have the same serialization, so there's a convenience method on the SPS tuple list that you can use here:

static size_t size(const PerfJITRecordPrefix &Val) {
  return SPSPerfJITRecordPrefix::AsArgList::size(
    static_cast<uint32_t>(Val.Id), Val.TotalSize, Val.Timestamp);
}

142–152 ↗

(On Diff #506418)

Likewise here:

static bool deserialize(SPSInputBuffer &IB, PerfJITRecordPrefix &Val) {
    uint32_t Id;
    if (SPSPerfJITRecordPrefix::AsArgList::deserialize(IB, Id, Val.TotalSize, Val.Timestamp)) {
      Val.Id = Id;
      return true;
    }
    return false;
}

153–157 ↗

(On Diff #506418)

And here:

static bool serialize(SPSOutputBuffer &OB, const PerfJITRecordPrefix &Val) {
  return SPSPerfJITRecordPrefix:AsArgList::
        serialize(OB, static_cast<uint32_t>(Val.Id), Val.TotalSize, Val.Timestamp);
}

165 ↗

(On Diff #506418)

Same advice as above here -- the methods below can probably be shortened by using SPSPerfJITCodeLoadRecord::AsArgList.

207 ↗

(On Diff #506418)

More potential AsArgList simplifications here.

242 ↗

(On Diff #506418)

More potential AsArgList simplifications here.

278 ↗

(On Diff #506418)

More potential AsArgList simplifications here.

324 ↗

(On Diff #506418)

More potential AsArgList simplifications here.

llvm/include/llvm/ExecutionEngine/Orc/TargetProcess/JITLoaderPerf.h

1 ↗

(On Diff #506418)

Looks like this is missing the file name / description line.

//===------- JITLoaderPerf.h - Register profiler objects ------*- C++ -*-===//

llvm/lib/ExecutionEngine/Orc/PerfSupportPlugin.cpp

1

Needs LLVM licensing header.

22

There's already a LinkGraph::dump(raw_ostream&) method. Could that be used instead of this?

Alternatively if the former is dumping too verbosely could we clean it up? Or add this to LinkGraph as dumpCompact?

47–86

This shoul

82–83

It looks like the header is a known size -- you might be better off using a BinaryStreamWriter here.

100–113

This is being used to get the timestamp in the host process. Should we be timestamping the deserialized records on the executor side instead?

llvm/lib/ExecutionEngine/Orc/TargetProcess/JITLoaderPerf.cpp

1 ↗

(On Diff #506418)

Needs LLVM licensing header.

We should also have a file-level TODO to move this into the ORC runtime once we can.

[pchintalapudi]

Address review comments

[pchintalapudi]

Do the perf JIT APIs support unloading code? If so these should have a // TODO to fill them in later. (I don't think they're needed for the initial commit).

I don't see any records for code unloading in https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/Documentation/jitdump-specification.txt , and PerfJITEventListener didn't have support for code unloading either, so I'm inclined to believe it doesn't exist right now.

[pchintalapudi]

Diff against origin, address review comments

[pchintalapudi]

Add license to JITLoaderPerf

[pchintalapudi]

Diff against origin

[pchintalapudi]

• Missed a license

[pchintalapudi]

Cascade writer errors upward

[lhames]

In D146169#4205438, @pchintalapudi wrote:

Do the perf JIT APIs support unloading code? If so these should have a // TODO to fill them in later. (I don't think they're needed for the initial commit).

I don't see any records for code unloading in https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/Documentation/jitdump-specification.txt , and PerfJITEventListener didn't have support for code unloading either, so I'm inclined to believe it doesn't exist right now.

Thanks for checking, and for addressing all the review feedback. LGTM!

I really appreciate your work on this -- it's very exciting to see profiling support making its way into ORC. Thanks Prem!

• Lang.

[pchintalapudi]

Fix bugs in unwind record creation

[pchintalapudi]

Rebase on main

[pchintalapudi]

Diff against main

[pchintalapudi]

• Switch to lookupAndRecordAddrs

[lhames]

Just testing this on Linux and Darwin now.

On Linux it looks good, but I've switch llvm-jitlink's -perf-support option to false by default since it's writing unconditionally to the filesystem, which would be surprising default behavior.

On Darwin I'm seeing unused variable warnings due to the #ifdef __linux__ conditionals. Would it be reasonable to just conditionalize inclusion of that whole file in the TargetProcess library? When it's not included the registration addr will just be unavailable and PerfSupportPlugin::Create will error out based on that. Alternatively we could make most of JITLoaderPerf.cpp conditional on #ifdef __linux__ and just have non-linux versions of registerJITLoaderPerfImpl, registerJITLoaderPerfStartImpl, and registerJITLoaderPerfEndImpl that return descriptive errors.

@pchintalapudi -- what do you think?

[lhames]

Also I've just noticed that JITLoaderPerf.cpp (in OrcTargetProcess) depends directly on llvm/ExecutionEngine/Orc/TargetProcess/JITLoaderPerf.h from Orc. That's a layering violation -- the shared types should be moved up into a header in llvm/ExecutionEngine/Orc/Shared.

Let me know if you want me to take care of that.

[pchintalapudi]

Address review comments

[pchintalapudi]

Rebase

[lhames]

LGTM. Thanks very much @pchintalapudi!

[void]

I think this change is causing build failures. Could you take a look?

[thakis]

This breaks the build on Mac and win (eg http://45.33.8.238/macm1/58953/step_4.txt) (Linux is fine)

Please take a look and revert for now if it takes a while to fix.

[thurston]

It also breaks Android: https://lab.llvm.org/buildbot/#/builders/77/builds/26352/steps/8/logs/stdio

FAILED: lib/ExecutionEngine/Orc/TargetProcess/CMakeFiles/LLVMOrcTargetProcess.dir/JITLoaderPerf.cpp.o 
CCACHE_CPP2=yes CCACHE_HASHDIR=yes /usr/bin/ccache /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm_build0/bin/clang++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D_LIBCPP_ENABLE_ASSERTIONS -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm_build64/lib/ExecutionEngine/Orc/TargetProcess -I/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/llvm/lib/ExecutionEngine/Orc/TargetProcess -I/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm_build64/include -I/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/llvm/include -gmlt -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG  -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -std=c++17 -MD -MT lib/ExecutionEngine/Orc/TargetProcess/CMakeFiles/LLVMOrcTargetProcess.dir/JITLoaderPerf.cpp.o -MF lib/ExecutionEngine/Orc/TargetProcess/CMakeFiles/LLVMOrcTargetProcess.dir/JITLoaderPerf.cpp.o.d -o lib/ExecutionEngine/Orc/TargetProcess/CMakeFiles/LLVMOrcTargetProcess.dir/JITLoaderPerf.cpp.o -c /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/llvm/lib/ExecutionEngine/Orc/TargetProcess/JITLoaderPerf.cpp
/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/llvm/lib/ExecutionEngine/Orc/TargetProcess/JITLoaderPerf.cpp:349:12: error: moving a local object in a return statement prevents copy elision [-Werror,-Wpessimizing-move]
    return std::move(err);
           ^
/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/llvm/lib/ExecutionEngine/Orc/TargetProcess/JITLoaderPerf.cpp:349:12: note: remove std::move call here
    return std::move(err);
           ^~~~~~~~~~   ~
/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/llvm/lib/ExecutionEngine/Orc/TargetProcess/JITLoaderPerf.cpp:378:12: error: moving a local object in a return statement prevents copy elision [-Werror,-Wpessimizing-move]
    return std::move(err);
           ^
/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/llvm/lib/ExecutionEngine/Orc/TargetProcess/JITLoaderPerf.cpp:378:12: note: remove std::move call here
    return std::move(err);
           ^~~~~~~~~~   ~
2 errors generated.

[vchuravy]

Reverted for now.

[lhames]

Looks like a few warnings triggering errors on -Werror bots. There are the moves above, plus:

C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\lib\ExecutionEngine\Orc\PerfSupportPlugin.cpp(285,44): warning: lambda capture 'MR' is not used [-Wunused-lambda-capture]
C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\lib\ExecutionEngine\Orc\PerfSupportPlugin.cpp(88,24): warning: unused function 'timespec_to_ns' [-Wunused-function]
C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\lib\ExecutionEngine\Orc\PerfSupportPlugin.cpp(93,24): warning: unused function 'perf_get_timestamp' [-Wunused-function]

[lhames]

Oh, and the execution test will need a # REQUIRES: native && target-x86_64 line to prevent it running on other architectures (e.g. https://lab.llvm.org/buildbot#builders/230/builds/11974).

Side note: I should probably add nice error message to llvm-jitlink for "you're trying to run code from a different architecture", rather than just jumping straight into it. :)

[vitalybuka]

Can you please fix or revert https://lab.llvm.org/buildbot/#/builders/5/builds/36753

I guess JIT does not play well with sanitizers, maybe just set UNSUPPORTED: for whatever is msan feature

[kda]

Disabled MSAN on ELF_perf.s.
https://github.com/llvm/llvm-project/commit/c7294486f3ee5f70a261ab78ab94e1490d87ebde

Please repair.

llvm/lib/ExecutionEngine/Orc/PerfSupportPlugin.cpp

 //===----- PerfSupportPlugin.cpp --- Utils for perf support -----*- 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
 //
 //===----------------------------------------------------------------------===//
 //
 // Handles support for registering code with perf
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ExecutionEngine/Orc/PerfSupportPlugin.h"
 
 #include "llvm/ExecutionEngine/Orc/TargetProcess/JITLoaderPerf.h"
 
 #include "llvm/DebugInfo/DWARF/DWARFContext.h"
 #include "llvm/ExecutionEngine/JITLink/x86_64.h"
 #include "llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h"
 #include "llvm/ExecutionEngine/Orc/Shared/SimplePackedSerialization.h"
 
 #define DEBUG_TYPE "orc"
 
 using namespace llvm;
 using namespace llvm::orc;
 using namespace llvm::jitlink;
 
 namespace {
 
 // Creates an EH frame header prepared for a 32-bit relative relocation
 // to the start of the .eh_frame section. Absolute injects a 64-bit absolute
 // address space offset 4 bytes from the start instead of 4 bytes
 Expected<std::string> createX64EHFrameHeader(Section &EHFrame,
                                    support::endianness endianness,
                                    bool absolute) {
   uint8_t Version = 1;
   uint8_t EhFramePtrEnc = 0;
   if (absolute) {
     EhFramePtrEnc |= dwarf::DW_EH_PE_sdata8 | dwarf::DW_EH_PE_absptr;
   } else {
     EhFramePtrEnc |= dwarf::DW_EH_PE_sdata4 | dwarf::DW_EH_PE_datarel;
   }
   uint8_t FDECountEnc = dwarf::DW_EH_PE_omit;
   uint8_t TableEnc = dwarf::DW_EH_PE_omit;
   // X86_64_64 relocation to the start of the .eh_frame section
   uint32_t EHFrameRelocation = 0;
   // uint32_t FDECount = 0;
   // Skip the FDE binary search table
   // We'd have to reprocess the CIEs to get this information,
   // which seems like more trouble than it's worth
   // TODO consider implementing this.
   // binary search table goes here
 
   size_t HeaderSize = (sizeof(Version) + sizeof(EhFramePtrEnc) +
-                               sizeof(FDECountEnc) + sizeof(TableEnc) + absolute
+                               sizeof(FDECountEnc) + sizeof(TableEnc) + (absolute
                            ? sizeof(uint64_t)
-                           : sizeof(EHFrameRelocation));
+                           : sizeof(EHFrameRelocation)));
   std::string HeaderContent(HeaderSize, '\0');
   BinaryStreamWriter Writer(
       MutableArrayRef<uint8_t>(
           reinterpret_cast<uint8_t *>(HeaderContent.data()), HeaderSize),
       endianness);
   if (auto Err = Writer.writeInteger(Version))
     return std::move(Err);
   if (auto Err = Writer.writeInteger(EhFramePtrEnc))
     return std::move(Err);
   if (auto Err = Writer.writeInteger(FDECountEnc))
     return std::move(Err);
   if (auto Err = Writer.writeInteger(TableEnc))
     return std::move(Err);
   if (absolute) {
-    uint64_t EHFrameAddr = support::endian::byte_swap(
-        SectionRange(EHFrame).getStart().getValue(), endianness);
+    uint64_t EHFrameAddr = SectionRange(EHFrame).getStart().getValue();
     if (auto Err = Writer.writeInteger(EHFrameAddr))
       return std::move(Err);
   } else {
     if (auto Err = Writer.writeInteger(EHFrameRelocation))
       return std::move(Err);
   }
   return HeaderContent;
 }
 
 constexpr StringRef RegisterPerfStartSymbolName =
     "llvm_orc_registerJITLoaderPerfStart";
 constexpr StringRef RegisterPerfEndSymbolName =
     "llvm_orc_registerJITLoaderPerfEnd";
 constexpr StringRef RegisterPerfImplSymbolName =
     "llvm_orc_registerJITLoaderPerfImpl";
 
 static inline uint64_t timespec_to_ns(const struct timespec *ts) {
   const uint64_t NanoSecPerSec = 1000000000;
   return ((uint64_t)ts->tv_sec * NanoSecPerSec) + ts->tv_nsec;
 }
 
 static inline uint64_t perf_get_timestamp() {
 #ifdef __linux__
   struct timespec ts;
   int ret;
 
   ret = clock_gettime(CLOCK_MONOTONIC, &ts);
   if (ret)
     return 0;
 
   return timespec_to_ns(&ts);
 #else
   return 0;
 #endif
 }
 
 static PerfJITCodeLoadRecord
 getCodeLoadRecord(const Symbol &Sym, std::atomic<uint64_t> &CodeIndex) {
   PerfJITCodeLoadRecord Record;
   auto Name = Sym.getName();
   auto Addr = Sym.getAddress();
   auto Size = Sym.getSize();
   Record.Prefix.Id = PerfJITRecordType::JIT_CODE_LOAD;
   // Runtime sets PID
   Record.Pid = 0;
   // Runtime sets TID
   Record.Tid = 0;
   Record.Vma = Addr.getValue();
   Record.CodeAddr = Addr.getValue();