[PATCH] D58704: Initial (incomplete) implementation of JITLink - A replacement for RuntimeDyld.

[Stefan Gränitz via Phabricator via llvm-commits]

sgraenitz added a comment.

One crucial part that I found missing here is a way to connect `JITEventListener` (like GDBJITRegistration, PerfJITEvents, etc.), because the new `ObjectLinkingLayer::NotifyLoaded` function only provides the module key but no `ObjectFile` so we cannot create and pass over a `LoadedObjectInfo` as with `RuntimeDyld`. Please see my inline comments for some pointers.

While support for `JITEventListener` was never implemented for MachO, it has been working well for ELF and should be supported design-wise (while ELF support is in progress).

@lhames Can you advise a good way to pass out ownership of the `ObjectFile` created in `JITLinker::buildGraph()` back to `ObjectLinkingLayer`? IIUC we could allow to query it by ID from there and create something like a `LoadedObjectInfo` independently.



================
Comment at: llvm/trunk/lib/ExecutionEngine/JITLink/JITLink_MachO_x86_64.cpp:460
+  buildGraph(MemoryBufferRef ObjBuffer) override {
+    auto MachOObj = object::ObjectFile::createMachOObjectFile(ObjBuffer);
+    if (!MachOObj)
----------------
`ObjectFile` is instantiated here.


================
Comment at: llvm/trunk/lib/ExecutionEngine/JITLink/JITLink_MachO_x86_64.cpp:464
+    return MachOAtomGraphBuilder_x86_64(**MachOObj).buildGraph();
+  }
+
----------------
`ObjectFile` gets deleted here.


================
Comment at: llvm/trunk/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp:137
+    if (Layer.NotifyLoaded)
+      Layer.NotifyLoaded(MR.getVModuleKey());
+  }
----------------
Notification function only provides the module key.


================
Comment at: llvm/trunk/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp:154
+    Layer.notifyFinalized(
+        ObjectLinkingLayer::ObjectResources(std::move(A), EHFrameAddr));
+  }
----------------
Materialization can still fail in `notifyFinalized()` and ownership of allocation is only passed back to `ObjectLinkingLayer` here. 


================
Comment at: llvm/trunk/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp:346
+  jitLink(llvm::make_unique<ObjectLinkingLayerJITLinkContext>(
+      *this, std::move(R), std::move(O)));
+}
----------------
`jitLink()` instantiates the `JITLinker` that owns the memory buffer and creates an `ObjectFile`. None of this is accessible to the outside during materialization.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58704/new/

https://reviews.llvm.org/D58704