[llvm] 7565b20 - [ORC] Switch ObjectLinkingLayer::Plugins to shared ownership, copy pipeline.

Lang Hames via llvm-commits llvm-commits at lists.llvm.org
Wed May 1 01:40:17 PDT 2024


Author: Lang Hames
Date: 2024-04-30T23:10:06-09:30
New Revision: 7565b20b50b254a72efa9d505e92be65c664b1b2

URL: https://github.com/llvm/llvm-project/commit/7565b20b50b254a72efa9d505e92be65c664b1b2
DIFF: https://github.com/llvm/llvm-project/commit/7565b20b50b254a72efa9d505e92be65c664b1b2.diff

LOG: [ORC] Switch ObjectLinkingLayer::Plugins to shared ownership, copy pipeline.

Previously ObjectLinkingLayer held unique ownership of Plugins, and links
always used the Layer's plugin list at each step. This can cause problems if
plugins are added while links are in progress however, as the newly added
plugin may receive only some of the callbacks for links that are already
running.

In this patch each link gets its own copy of the pipeline that remains
consistent throughout the link's lifetime, and it is guaranteed that Plugin
objects (now with shared ownership) will remain valid until the link completes.

Coding my way home: 9.80469S, 139.03167W

Added: 
    

Modified: 
    llvm/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h
    llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h b/llvm/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h
index 34f2e0789462c9..e452b90598a0ad 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h
@@ -122,7 +122,7 @@ class ObjectLinkingLayer : public RTTIExtends<ObjectLinkingLayer, ObjectLayer>,
   }
 
   /// Add a pass-config modifier.
-  ObjectLinkingLayer &addPlugin(std::unique_ptr<Plugin> P) {
+  ObjectLinkingLayer &addPlugin(std::shared_ptr<Plugin> P) {
     std::lock_guard<std::mutex> Lock(LayerMutex);
     Plugins.push_back(std::move(P));
     return *this;
@@ -181,11 +181,8 @@ class ObjectLinkingLayer : public RTTIExtends<ObjectLinkingLayer, ObjectLayer>,
 private:
   using FinalizedAlloc = jitlink::JITLinkMemoryManager::FinalizedAlloc;
 
-  void modifyPassConfig(MaterializationResponsibility &MR,
-                        jitlink::LinkGraph &G,
-                        jitlink::PassConfiguration &PassConfig);
-  void notifyLoaded(MaterializationResponsibility &MR);
-  Error notifyEmitted(MaterializationResponsibility &MR, FinalizedAlloc FA);
+  Error recordFinalizedAlloc(MaterializationResponsibility &MR,
+                             FinalizedAlloc FA);
 
   Error handleRemoveResources(JITDylib &JD, ResourceKey K) override;
   void handleTransferResources(JITDylib &JD, ResourceKey DstKey,
@@ -198,7 +195,7 @@ class ObjectLinkingLayer : public RTTIExtends<ObjectLinkingLayer, ObjectLayer>,
   bool AutoClaimObjectSymbols = false;
   ReturnObjectBufferFunction ReturnObjectBuffer;
   DenseMap<ResourceKey, std::vector<FinalizedAlloc>> Allocs;
-  std::vector<std::unique_ptr<Plugin>> Plugins;
+  std::vector<std::shared_ptr<Plugin>> Plugins;
 };
 
 class EHFrameRegistrationPlugin : public ObjectLinkingLayer::Plugin {

diff  --git a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
index 131728fd7e7e4c..57ade947678657 100644
--- a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
@@ -156,7 +156,10 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
       std::unique_ptr<MaterializationResponsibility> MR,
       std::unique_ptr<MemoryBuffer> ObjBuffer)
       : JITLinkContext(&MR->getTargetJITDylib()), Layer(Layer),
-        MR(std::move(MR)), ObjBuffer(std::move(ObjBuffer)) {}
+        MR(std::move(MR)), ObjBuffer(std::move(ObjBuffer)) {
+    std::lock_guard<std::mutex> Lock(Layer.LayerMutex);
+    Plugins = Layer.Plugins;
+  }
 
   ~ObjectLinkingLayerJITLinkContext() {
     // If there is an object buffer return function then use it to
@@ -168,14 +171,14 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
   JITLinkMemoryManager &getMemoryManager() override { return Layer.MemMgr; }
 
   void notifyMaterializing(LinkGraph &G) {
-    for (auto &P : Layer.Plugins)
+    for (auto &P : Plugins)
       P->notifyMaterializing(*MR, G, *this,
                              ObjBuffer ? ObjBuffer->getMemBufferRef()
                              : MemoryBufferRef());
   }
 
   void notifyFailed(Error Err) override {
-    for (auto &P : Layer.Plugins)
+    for (auto &P : Plugins)
       Err = joinErrors(std::move(Err), P->notifyFailed(*MR));
     Layer.getExecutionSession().reportError(std::move(Err));
     MR->failMaterialization();
@@ -317,12 +320,12 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
     if (auto Err = MR->notifyResolved(InternedResult))
       return Err;
 
-    Layer.notifyLoaded(*MR);
+    notifyLoaded();
     return Error::success();
   }
 
   void notifyFinalized(JITLinkMemoryManager::FinalizedAlloc A) override {
-    if (auto Err = Layer.notifyEmitted(*MR, std::move(A))) {
+    if (auto Err = notifyEmitted(std::move(A))) {
       Layer.getExecutionSession().reportError(std::move(Err));
       MR->failMaterialization();
       return;
@@ -344,7 +347,8 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
       return claimOrExternalizeWeakAndCommonSymbols(G);
     });
 
-    Layer.modifyPassConfig(*MR, LG, Config);
+    for (auto &P : Plugins)
+      P->modifyPassConfig(*MR, LG, Config);
 
     Config.PreFixupPasses.push_back(
         [this](LinkGraph &G) { return registerDependencies(G); });
@@ -352,6 +356,29 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
     return Error::success();
   }
 
+  void notifyLoaded() {
+    for (auto &P : Plugins)
+      P->notifyLoaded(*MR);
+  }
+
+  Error notifyEmitted(jitlink::JITLinkMemoryManager::FinalizedAlloc FA) {
+    Error Err = Error::success();
+    for (auto &P : Plugins)
+      Err = joinErrors(std::move(Err), P->notifyEmitted(*MR));
+
+    if (Err) {
+      if (FA)
+        Err =
+            joinErrors(std::move(Err), Layer.MemMgr.deallocate(std::move(FA)));
+      return Err;
+    }
+
+    if (FA)
+      return Layer.recordFinalizedAlloc(*MR, std::move(FA));
+
+    return Error::success();
+  }
+
 private:
   // Symbol name dependencies:
   // Internal: Defined in this graph.
@@ -522,7 +549,7 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
 
     SymbolDependenceGroup SynthSDG;
 
-    for (auto &P : Layer.Plugins) {
+    for (auto &P : Plugins) {
       auto SynthDeps = P->getSyntheticSymbolDependencies(*MR);
       if (SynthDeps.empty())
         continue;
@@ -636,6 +663,7 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
   }
 
   ObjectLinkingLayer &Layer;
+  std::vector<std::shared_ptr<ObjectLinkingLayer::Plugin>> Plugins;
   std::unique_ptr<MaterializationResponsibility> MR;
   std::unique_ptr<MemoryBuffer> ObjBuffer;
   DenseMap<Block *, SymbolNameSet> ExternalBlockDeps;
@@ -702,34 +730,9 @@ void ObjectLinkingLayer::emit(std::unique_ptr<MaterializationResponsibility> R,
   link(std::move(G), std::move(Ctx));
 }
 
-void ObjectLinkingLayer::modifyPassConfig(MaterializationResponsibility &MR,
-                                          LinkGraph &G,
-                                          PassConfiguration &PassConfig) {
-  for (auto &P : Plugins)
-    P->modifyPassConfig(MR, G, PassConfig);
-}
-
-void ObjectLinkingLayer::notifyLoaded(MaterializationResponsibility &MR) {
-  for (auto &P : Plugins)
-    P->notifyLoaded(MR);
-}
-
-Error ObjectLinkingLayer::notifyEmitted(MaterializationResponsibility &MR,
-                                        FinalizedAlloc FA) {
-  Error Err = Error::success();
-  for (auto &P : Plugins)
-    Err = joinErrors(std::move(Err), P->notifyEmitted(MR));
-
-  if (Err) {
-    if (FA)
-      Err = joinErrors(std::move(Err), MemMgr.deallocate(std::move(FA)));
-    return Err;
-  }
-
-  if (!FA)
-    return Error::success();
-
-  Err = MR.withResourceKeyDo(
+Error ObjectLinkingLayer::recordFinalizedAlloc(
+    MaterializationResponsibility &MR, FinalizedAlloc FA) {
+  auto Err = MR.withResourceKeyDo(
       [&](ResourceKey K) { Allocs[K].push_back(std::move(FA)); });
 
   if (Err)


        


More information about the llvm-commits mailing list