[Orc] Add AutoRegisterCode option for DebugObjectManagerPlugin

[sgraenitz]

Summary

Configure the plugin to automatically call the debugger rendezvous breakpoint __jit_debug_register_code() for every translation unit (enabled) or never at all (disabled). Default API and behavior remain unchanged.

If AutoRegisterCode is turned off, it's the client's own responsibility to call the rendezvous breakpoint function at an appropriate time.
Depending on the complexity of the debugger's rendezvous breakpoint implementation, this can provide significant performance improvements in cases where many debug objects are added in sequence.

[sgraenitz]

We don't have versioning of the RPC interface, so adding the AutoRegisterCode parameter will be a breaking change. It should show up as a compiler error for clients with customized target process implementations. That makes it obvious and easy to fix (I hope).
Despite of the RPC interface change, I didn't opt for the alternative implementation of configuring the AutoRegisterCode setting only once on the executor side. I think it's better to have it as a parameter for the actual RPC call, as this allows the controller to modify the behavior dynamically.

What do you think?

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

113

@lhames I've never used these alloc actions in practice and just did copy/paste here. Do you think it's ok like this?

[lhames]

LGTM. Thanks Stefan!

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

113

This looks good, but you'll need to update llvm/lib/ExecutionEngine/Orc/DebuggerSupportPlugin.cpp's use to pass the bool arg in.

DebugObjectManagerPlugin.h should probably be updated to use allocation actions in the future, but that can be done independently of this patch.

Side note, since I'm not sure it's well documented: An AllocAction is just an SPS wrapper function that returns an SPSError. It could be called via the regular RPC path, but by restricting the return type to SPSError it becomes possible to batch up calls on the LinkGraph instead: the memory manager will call the actions on our behalf (using the serialized argument buffer, so the memory manager doesn't care about the serialized argument types), and the memory manager knows how to interpret the return value, since it's always SPSError.

Since llvm_orc_registerJITLoaderGDBAllocAction and llvm_orc_registerJITLoaderGDBWrapper are identical we could just have one call the other, but they're also not a lot of code to duplicate either.

[sgraenitz]

Add documentation for AutoRegisterCode parameter

[sgraenitz]

Switch to the explicit ctor in lli, llvm-jitlink and LLJITWithRemoteDebugging (not adding user-facing options for now)

[sgraenitz]

Update DebuggerSupportPlugin to use the new RPC interface

[sgraenitz]

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

113

Alright, thanks! Then this can stay as is for the moment.

I included the update for DebuggerSupportPlugin, which I always thought had re-used the EPCDebugObjectRegistrar. It was meant to encapsulate the RPC interface. However, the DebuggerSupportPlugin does everything on its own.

Is it supposed to stay like that? Because then I will drop EPCDebugObjectRegistrar at some point..

llvm/include/llvm/ExecutionEngine/Orc/DebugObjectManagerPlugin.h

public:
   /// RequireDebugSections:
   ///   Submit debug objects to the executor only if they contain actual debug
   ///   info. Turning this off may allow minimal debugging based on raw symbol
   ///   names. Note that this may cause significant memory and transport
   ///   overhead for objects built with a release configuration.
   ///
   DebugObjectManagerPlugin(ExecutionSession &ES,
                            std::unique_ptr<DebugObjectRegistrar> Target,
-                           bool RequireDebugSections);
+                           bool RequireDebugSections, bool AutoRegisterCode);
   ~DebugObjectManagerPlugin();
 
   void notifyMaterializing(MaterializationResponsibility &MR,
                            jitlink::LinkGraph &G, jitlink::JITLinkContext &Ctx,
                            MemoryBufferRef InputObject) override;
 
   Error notifyEmitted(MaterializationResponsibility &MR) override;
   Error notifyFailed(MaterializationResponsibility &MR) override;
private:
   std::map<MaterializationResponsibility *, OwnedDebugObject> PendingObjs;
   std::map<ResourceKey, std::vector<OwnedDebugObject>> RegisteredObjs;
 
   std::mutex PendingObjsLock;
   std::mutex RegisteredObjsLock;
 
   std::unique_ptr<DebugObjectRegistrar> Target;
   bool RequireDebugSections;
+  bool AutoRegisterCode;
 };
 
 } // namespace orc
 } // namespace llvm
 
 #endif // LLVM_EXECUTIONENGINE_ORC_DEBUGOBJECTMANAGERPLUGIN_H

llvm/include/llvm/ExecutionEngine/Orc/EPCDebugObjectRegistrar.h

 namespace llvm {
 namespace orc {
 
 class ExecutionSession;
 
 /// Abstract interface for registering debug objects in the executor process.
 class DebugObjectRegistrar {
 public:
-  virtual Error registerDebugObject(ExecutorAddrRange TargetMem) = 0;
+  virtual Error registerDebugObject(ExecutorAddrRange TargetMem,
+                                    bool AutoRegisterCode) = 0;
   virtual ~DebugObjectRegistrar() = default;
 };
 
 /// Use ExecutorProcessControl to register debug objects locally or in a remote
 /// executor process.
 class EPCDebugObjectRegistrar : public DebugObjectRegistrar {
 public:
   EPCDebugObjectRegistrar(ExecutionSession &ES, ExecutorAddr RegisterFn)
       : ES(ES), RegisterFn(RegisterFn) {}
 
-  Error registerDebugObject(ExecutorAddrRange TargetMem) override;
+  Error registerDebugObject(ExecutorAddrRange TargetMem,
+                            bool AutoRegisterCode) override;
 
 private:
   ExecutionSession &ES;
   ExecutorAddr RegisterFn;
 };
 
 /// Create a ExecutorProcessControl-based DebugObjectRegistrar that emits debug
 /// objects to the GDB JIT interface. This will use the EPC's lookupSymbols

llvm/lib/ExecutionEngine/Orc/DebugObjectManagerPlugin.cpp

default:
     // TODO: Once we add support for other formats, we might want to split this
     // into multiple files.
     return nullptr;
   }
 }
 
 DebugObjectManagerPlugin::DebugObjectManagerPlugin(
     ExecutionSession &ES, std::unique_ptr<DebugObjectRegistrar> Target,
-    bool RequireDebugSections)
+    bool RequireDebugSections, bool AutoRegisterCode)
     : ES(ES), Target(std::move(Target)),
-      RequireDebugSections(RequireDebugSections) {}
+      RequireDebugSections(RequireDebugSections),
+      AutoRegisterCode(AutoRegisterCode) {}
 
 DebugObjectManagerPlugin::DebugObjectManagerPlugin(
     ExecutionSession &ES, std::unique_ptr<DebugObjectRegistrar> Target)
-    : DebugObjectManagerPlugin(ES, std::move(Target), true) {}
+    : DebugObjectManagerPlugin(ES, std::move(Target), true, true) {}
 
 DebugObjectManagerPlugin::~DebugObjectManagerPlugin() = default;
 
 void DebugObjectManagerPlugin::notifyMaterializing(
     MaterializationResponsibility &MR, LinkGraph &G, JITLinkContext &Ctx,
     MemoryBufferRef ObjBuffer) {
   std::lock_guard<std::mutex> Lock(PendingObjsLock);
   assert(PendingObjs.count(&MR) == 0 &&
Error DebugObjectManagerPlugin::notifyEmitted(
 
   It->second->finalizeAsync(
       [this, &FinalizePromise, &MR](Expected<ExecutorAddrRange> TargetMem) {
         // Any failure here will fail materialization.
         if (!TargetMem) {
           FinalizePromise.set_value(TargetMem.takeError());
           return;
         }
-        if (Error Err = Target->registerDebugObject(*TargetMem)) {
+        if (Error Err =
+                Target->registerDebugObject(*TargetMem, AutoRegisterCode)) {
           FinalizePromise.set_value(std::move(Err));
           return;
         }
 
         // Once our tracking info is updated, notifyEmitted() can return and
         // finish materialization.
         FinalizePromise.set_value(MR.withResourceKeyDo([&](ResourceKey K) {
           assert(PendingObjs.count(&MR) && "We still hold PendingObjsLock");

llvm/lib/ExecutionEngine/Orc/EPCDebugObjectRegistrar.cpp

Expected<std::unique_ptr<EPCDebugObjectRegistrar>> createJITLoaderGDBRegistrar(
   assert(Result->size() == 1 && "Unexpected number of dylibs in result");
   assert((*Result)[0].size() == 1 &&
          "Unexpected number of addresses in result");
 
   return std::make_unique<EPCDebugObjectRegistrar>(
       ES, ExecutorAddr((*Result)[0][0]));
 }
 
-Error EPCDebugObjectRegistrar::registerDebugObject(
-    ExecutorAddrRange TargetMem) {
-  return ES.callSPSWrapper<void(shared::SPSExecutorAddrRange)>(RegisterFn,
-                                                               TargetMem);
+Error EPCDebugObjectRegistrar::registerDebugObject(ExecutorAddrRange TargetMem,
+                                                   bool AutoRegisterCode) {
+  return ES.callSPSWrapper<void(shared::SPSExecutorAddrRange, bool)>(
+      RegisterFn, TargetMem, AutoRegisterCode);
 }
 
 } // namespace orc
 } // namespace llvm

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

 #endif
 }
 }
 
 using namespace llvm;
 using namespace llvm::orc;
 
 // Register debug object, return error message or null for success.
-static void registerJITLoaderGDBImpl(const char *ObjAddr, size_t Size) {
+static void appendJITDebugDescriptor(const char *ObjAddr, size_t Size) {
   LLVM_DEBUG({
-    dbgs() << "Registering debug object with GDB JIT interface "
+    dbgs() << "Adding debug object to GDB JIT interface "
            << formatv("([{0:x16} -- {1:x16}])",
                       reinterpret_cast<uintptr_t>(ObjAddr),
                       reinterpret_cast<uintptr_t>(ObjAddr + Size))
            << "\n";
   });
 
   jit_code_entry *E = new jit_code_entry;
   E->symfile_addr = ObjAddr;
   E->symfile_size = Size;
   E->prev_entry = nullptr;
 
   // Serialize rendezvous with the debugger as well as access to shared data.
   static std::mutex JITDebugLock;
   std::lock_guard<std::mutex> Lock(JITDebugLock);
 
   // Insert this entry at the head of the list.
   jit_code_entry *NextEntry = __jit_debug_descriptor.first_entry;
   E->next_entry = NextEntry;
   if (NextEntry) {
     NextEntry->prev_entry = E;
   }
 
   __jit_debug_descriptor.first_entry = E;
   __jit_debug_descriptor.relevant_entry = E;
-
-  // Run into the rendezvous breakpoint.
   __jit_debug_descriptor.action_flag = JIT_REGISTER_FN;
-  __jit_debug_register_code();
 }
 
 extern "C" orc::shared::CWrapperFunctionResult
 llvm_orc_registerJITLoaderGDBAllocAction(const char *Data, size_t Size) {
   using namespace orc::shared;
-  return WrapperFunction<SPSError(SPSExecutorAddrRange)>::handle(
+  return WrapperFunction<SPSError(SPSExecutorAddrRange, bool)>::handle(
              Data, Size,
-             [](ExecutorAddrRange R) {
-               registerJITLoaderGDBImpl(R.Start.toPtr<const char *>(),
+             [](ExecutorAddrRange R, bool AutoRegisterCode) {
+               appendJITDebugDescriptor(R.Start.toPtr<const char *>(),
                                         R.size());
+               // Run into the rendezvous breakpoint.
+               if (AutoRegisterCode)
+                 __jit_debug_register_code();
                return Error::success();
              })
       .release();
 }
 
 extern "C" orc::shared::CWrapperFunctionResult
 llvm_orc_registerJITLoaderGDBWrapper(const char *Data, uint64_t Size) {
   using namespace orc::shared;
-  return WrapperFunction<SPSError(SPSExecutorAddrRange)>::handle(
+  return WrapperFunction<SPSError(SPSExecutorAddrRange, bool)>::handle(
              Data, Size,
-             [](ExecutorAddrRange R) {
-               registerJITLoaderGDBImpl(R.Start.toPtr<const char *>(),
+             [](ExecutorAddrRange R, bool AutoRegisterCode) {
+               appendJITDebugDescriptor(R.Start.toPtr<const char *>(),
                                         R.size());
+               // Run into the rendezvous breakpoint.
+               if (AutoRegisterCode)
+                 __jit_debug_register_code();
                return Error::success();
              })
       .release();
 }