[llvm] [Orc] Move SymbolStringPool from EPC to ExecutionSession (NFC) (PR #85056)

Stefan Gränitz via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 13 03:40:20 PDT 2024


https://github.com/weliveindetail updated https://github.com/llvm/llvm-project/pull/85056

>From 23a9ec6ce3fa87e0cdaabab420d9f9938f246d0d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Wed, 13 Mar 2024 11:02:46 +0100
Subject: [PATCH 1/3] [Orc] Move SymbolStringPool from EPC to ExecutionSession
 (NFC)

---
 llvm/include/llvm/ExecutionEngine/Orc/Core.h  | 10 +++---
 .../Orc/ExecutorProcessControl.h              | 18 +++--------
 .../ExecutionEngine/Orc/SimpleRemoteEPC.h     |  9 ++----
 llvm/lib/ExecutionEngine/Orc/Core.cpp         | 32 ++++++++-----------
 .../Orc/EPCDebugObjectRegistrar.cpp           |  4 +--
 .../Orc/ExecutorProcessControl.cpp            | 23 +++++++------
 llvm/tools/lli/lli.cpp                        |  3 +-
 llvm/tools/llvm-jitlink/llvm-jitlink.cpp      |  1 -
 .../ExecutionEngine/Orc/CoreAPIsTest.cpp      |  3 +-
 .../Orc/ObjectLinkingLayerTest.cpp            |  3 +-
 .../ExecutionEngine/Orc/OrcTestCommon.h       |  3 +-
 11 files changed, 43 insertions(+), 66 deletions(-)

diff --git a/llvm/include/llvm/ExecutionEngine/Orc/Core.h b/llvm/include/llvm/ExecutionEngine/Orc/Core.h
index 45bf9adcf2a4ec..e991f7e6cdb2cb 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/Core.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/Core.h
@@ -1453,7 +1453,8 @@ class ExecutionSession {
 
   /// Construct an ExecutionSession with the given ExecutorProcessControl
   /// object.
-  ExecutionSession(std::unique_ptr<ExecutorProcessControl> EPC);
+  ExecutionSession(std::unique_ptr<ExecutorProcessControl> EPC,
+                   std::shared_ptr<SymbolStringPool> SSP = nullptr);
 
   /// Destroy an ExecutionSession. Verifies that endSession was called prior to
   /// destruction.
@@ -1474,12 +1475,10 @@ class ExecutionSession {
   size_t getPageSize() const { return EPC->getPageSize(); }
 
   /// Get the SymbolStringPool for this instance.
-  std::shared_ptr<SymbolStringPool> getSymbolStringPool() {
-    return EPC->getSymbolStringPool();
-  }
+  std::shared_ptr<SymbolStringPool> getSymbolStringPool() { return SSP; }
 
   /// Add a symbol name to the SymbolStringPool and return a pointer to it.
-  SymbolStringPtr intern(StringRef SymName) { return EPC->intern(SymName); }
+  SymbolStringPtr intern(StringRef SymName) { return SSP->intern(SymName); }
 
   /// Set the Platform for this ExecutionSession.
   void setPlatform(std::unique_ptr<Platform> P) { this->P = std::move(P); }
@@ -1856,6 +1855,7 @@ class ExecutionSession {
   mutable std::recursive_mutex SessionMutex;
   bool SessionOpen = true;
   std::unique_ptr<ExecutorProcessControl> EPC;
+  std::shared_ptr<SymbolStringPool> SSP;
   std::unique_ptr<Platform> P;
   ErrorReporter ReportError = logErrorsToStdErr;
   DispatchTaskFunction DispatchTask = runOnCurrentThread;
diff --git a/llvm/include/llvm/ExecutionEngine/Orc/ExecutorProcessControl.h b/llvm/include/llvm/ExecutionEngine/Orc/ExecutorProcessControl.h
index 6468f2dfc11ad0..1673e3a0fe6b59 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/ExecutorProcessControl.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/ExecutorProcessControl.h
@@ -187,10 +187,7 @@ class ExecutorProcessControl {
     ExecutorAddr JITDispatchContext;
   };
 
-  ExecutorProcessControl(std::shared_ptr<SymbolStringPool> SSP,
-                         std::unique_ptr<TaskDispatcher> D)
-    : SSP(std::move(SSP)), D(std::move(D)) {}
-
+  ExecutorProcessControl(std::unique_ptr<TaskDispatcher> D) : D(std::move(D)) {}
   virtual ~ExecutorProcessControl();
 
   /// Return the ExecutionSession associated with this instance.
@@ -200,11 +197,8 @@ class ExecutorProcessControl {
     return *ES;
   }
 
-  /// Intern a symbol name in the SymbolStringPool.
-  SymbolStringPtr intern(StringRef SymName) { return SSP->intern(SymName); }
-
   /// Return a shared pointer to the SymbolStringPool for this instance.
-  std::shared_ptr<SymbolStringPool> getSymbolStringPool() const { return SSP; }
+  std::shared_ptr<SymbolStringPool> getSymbolStringPool() const;
 
   TaskDispatcher &getDispatcher() { return *D; }
 
@@ -464,11 +458,9 @@ class UnsupportedExecutorProcessControl : public ExecutorProcessControl,
                                           private InProcessMemoryAccess {
 public:
   UnsupportedExecutorProcessControl(
-      std::shared_ptr<SymbolStringPool> SSP = nullptr,
       std::unique_ptr<TaskDispatcher> D = nullptr, const std::string &TT = "",
       unsigned PageSize = 0)
       : ExecutorProcessControl(
-            SSP ? std::move(SSP) : std::make_shared<SymbolStringPool>(),
             D ? std::move(D) : std::make_unique<InPlaceTaskDispatcher>()),
         InProcessMemoryAccess(Triple(TT).isArch64Bit()) {
     this->TargetTriple = Triple(TT);
@@ -512,8 +504,7 @@ class SelfExecutorProcessControl : public ExecutorProcessControl,
                                    private InProcessMemoryAccess {
 public:
   SelfExecutorProcessControl(
-      std::shared_ptr<SymbolStringPool> SSP, std::unique_ptr<TaskDispatcher> D,
-      Triple TargetTriple, unsigned PageSize,
+      std::unique_ptr<TaskDispatcher> D, Triple TargetTriple, unsigned PageSize,
       std::unique_ptr<jitlink::JITLinkMemoryManager> MemMgr);
 
   /// Create a SelfExecutorProcessControl with the given symbol string pool and
@@ -522,8 +513,7 @@ class SelfExecutorProcessControl : public ExecutorProcessControl,
   /// If no memory manager is given a jitlink::InProcessMemoryManager will
   /// be created and used by default.
   static Expected<std::unique_ptr<SelfExecutorProcessControl>>
-  Create(std::shared_ptr<SymbolStringPool> SSP = nullptr,
-         std::unique_ptr<TaskDispatcher> D = nullptr,
+  Create(std::unique_ptr<TaskDispatcher> D = nullptr,
          std::unique_ptr<jitlink::JITLinkMemoryManager> MemMgr = nullptr);
 
   Expected<tpctypes::DylibHandle> loadDylib(const char *DylibPath) override;
diff --git a/llvm/include/llvm/ExecutionEngine/Orc/SimpleRemoteEPC.h b/llvm/include/llvm/ExecutionEngine/Orc/SimpleRemoteEPC.h
index c10b8df01cc0a4..4f78fc2f7c4200 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/SimpleRemoteEPC.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/SimpleRemoteEPC.h
@@ -50,9 +50,7 @@ class SimpleRemoteEPC : public ExecutorProcessControl,
   static Expected<std::unique_ptr<SimpleRemoteEPC>>
   Create(std::unique_ptr<TaskDispatcher> D, Setup S,
          TransportTCtorArgTs &&...TransportTCtorArgs) {
-    std::unique_ptr<SimpleRemoteEPC> SREPC(
-        new SimpleRemoteEPC(std::make_shared<SymbolStringPool>(),
-                            std::move(D)));
+    std::unique_ptr<SimpleRemoteEPC> SREPC(new SimpleRemoteEPC(std::move(D)));
     auto T = TransportT::Create(
         *SREPC, std::forward<TransportTCtorArgTs>(TransportTCtorArgs)...);
     if (!T)
@@ -94,9 +92,8 @@ class SimpleRemoteEPC : public ExecutorProcessControl,
   void handleDisconnect(Error Err) override;
 
 private:
-  SimpleRemoteEPC(std::shared_ptr<SymbolStringPool> SSP,
-                  std::unique_ptr<TaskDispatcher> D)
-    : ExecutorProcessControl(std::move(SSP), std::move(D)) {}
+  SimpleRemoteEPC(std::unique_ptr<TaskDispatcher> D)
+      : ExecutorProcessControl(std::move(D)) {}
 
   static Expected<std::unique_ptr<jitlink::JITLinkMemoryManager>>
   createDefaultMemoryManager(SimpleRemoteEPC &SREPC);
diff --git a/llvm/lib/ExecutionEngine/Orc/Core.cpp b/llvm/lib/ExecutionEngine/Orc/Core.cpp
index 37c32f86e8d873..93e4581a356f0e 100644
--- a/llvm/lib/ExecutionEngine/Orc/Core.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/Core.cpp
@@ -1589,9 +1589,11 @@ void LookupTask::printDescription(raw_ostream &OS) { OS << "Lookup task"; }
 
 void LookupTask::run() { LS.continueLookup(Error::success()); }
 
-ExecutionSession::ExecutionSession(std::unique_ptr<ExecutorProcessControl> EPC)
-    : EPC(std::move(EPC)) {
-  // Associated EPC and this.
+ExecutionSession::ExecutionSession(std::unique_ptr<ExecutorProcessControl> EPC,
+                                   std::shared_ptr<SymbolStringPool> SSP)
+    : EPC(std::move(EPC)),
+      SSP(SSP ? std::move(SSP) : std::make_unique<SymbolStringPool>()) {
+  // Associate EPC with this session.
   this->EPC->ES = this;
 }
 
@@ -2001,8 +2003,7 @@ Error ExecutionSession::removeResourceTracker(ResourceTracker &RT) {
                      L->handleRemoveResources(JD, RT.getKeyUnsafe()));
 
   for (auto &Q : QueriesToFail)
-    Q->handleFailed(
-        make_error<FailedToMaterialize>(getSymbolStringPool(), FailedSymbols));
+    Q->handleFailed(make_error<FailedToMaterialize>(SSP, FailedSymbols));
 
   return Err;
 }
@@ -2075,15 +2076,14 @@ Error ExecutionSession::IL_updateCandidatesFor(
         // weakly referenced" specific error here to reduce confusion.
         if (SymI->second.getFlags().hasMaterializationSideEffectsOnly() &&
             SymLookupFlags != SymbolLookupFlags::WeaklyReferencedSymbol)
-          return make_error<SymbolsNotFound>(getSymbolStringPool(),
-                                             SymbolNameVector({Name}));
+          return make_error<SymbolsNotFound>(SSP, SymbolNameVector({Name}));
 
         // If we matched against this symbol but it is in the error state
         // then bail out and treat it as a failure to materialize.
         if (SymI->second.getFlags().hasError()) {
           auto FailedSymbolsMap = std::make_shared<SymbolDependenceMap>();
           (*FailedSymbolsMap)[&JD] = {Name};
-          return make_error<FailedToMaterialize>(getSymbolStringPool(),
+          return make_error<FailedToMaterialize>(SSP,
                                                  std::move(FailedSymbolsMap));
         }
 
@@ -2341,7 +2341,7 @@ void ExecutionSession::OL_applyQueryPhase1(
   } else {
     LLVM_DEBUG(dbgs() << "Phase 1 failed with unresolved symbols.\n");
     IPLS->fail(make_error<SymbolsNotFound>(
-        getSymbolStringPool(), IPLS->DefGeneratorCandidates.getSymbolNames()));
+        SSP, IPLS->DefGeneratorCandidates.getSymbolNames()));
   }
 }
 
@@ -2411,8 +2411,7 @@ void ExecutionSession::OL_completeLookup(
                 dbgs() << "error: "
                           "required, but symbol is has-side-effects-only\n";
               });
-              return make_error<SymbolsNotFound>(getSymbolStringPool(),
-                                                 SymbolNameVector({Name}));
+              return make_error<SymbolsNotFound>(SSP, SymbolNameVector({Name}));
             }
 
             // If we matched against this symbol but it is in the error state
@@ -2422,7 +2421,7 @@ void ExecutionSession::OL_completeLookup(
               auto FailedSymbolsMap = std::make_shared<SymbolDependenceMap>();
               (*FailedSymbolsMap)[&JD] = {Name};
               return make_error<FailedToMaterialize>(
-                  getSymbolStringPool(), std::move(FailedSymbolsMap));
+                  SSP, std::move(FailedSymbolsMap));
             }
 
             // Otherwise this is a match.
@@ -2526,8 +2525,7 @@ void ExecutionSession::OL_completeLookup(
 
     if (!IPLS->LookupSet.empty()) {
       LLVM_DEBUG(dbgs() << "Failing due to unresolved symbols\n");
-      return make_error<SymbolsNotFound>(getSymbolStringPool(),
-                                         IPLS->LookupSet.getSymbolNames());
+      return make_error<SymbolsNotFound>(SSP, IPLS->LookupSet.getSymbolNames());
     }
 
     // Record whether the query completed.
@@ -2652,8 +2650,7 @@ void ExecutionSession::OL_completeLookupFlags(
 
     if (!IPLS->LookupSet.empty()) {
       LLVM_DEBUG(dbgs() << "Failing due to unresolved symbols\n");
-      return make_error<SymbolsNotFound>(getSymbolStringPool(),
-                                         IPLS->LookupSet.getSymbolNames());
+      return make_error<SymbolsNotFound>(SSP, IPLS->LookupSet.getSymbolNames());
     }
 
     LLVM_DEBUG(dbgs() << "Succeded, result = " << Result << "\n");
@@ -3465,8 +3462,7 @@ void ExecutionSession::OL_notifyFailed(MaterializationResponsibility &MR) {
   });
 
   for (auto &Q : FailedQueries)
-    Q->handleFailed(
-        make_error<FailedToMaterialize>(getSymbolStringPool(), FailedSymbols));
+    Q->handleFailed(make_error<FailedToMaterialize>(SSP, FailedSymbols));
 }
 
 Error ExecutionSession::OL_replace(MaterializationResponsibility &MR,
diff --git a/llvm/lib/ExecutionEngine/Orc/EPCDebugObjectRegistrar.cpp b/llvm/lib/ExecutionEngine/Orc/EPCDebugObjectRegistrar.cpp
index acd7e5a409fc57..7d483191a46253 100644
--- a/llvm/lib/ExecutionEngine/Orc/EPCDebugObjectRegistrar.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/EPCDebugObjectRegistrar.cpp
@@ -30,8 +30,8 @@ Expected<std::unique_ptr<EPCDebugObjectRegistrar>> createJITLoaderGDBRegistrar(
 
   SymbolStringPtr RegisterFn =
       EPC.getTargetTriple().isOSBinFormatMachO()
-          ? EPC.intern("_llvm_orc_registerJITLoaderGDBWrapper")
-          : EPC.intern("llvm_orc_registerJITLoaderGDBWrapper");
+          ? ES.intern("_llvm_orc_registerJITLoaderGDBWrapper")
+          : ES.intern("llvm_orc_registerJITLoaderGDBWrapper");
 
   SymbolLookupSet RegistrationSymbols;
   RegistrationSymbols.add(RegisterFn);
diff --git a/llvm/lib/ExecutionEngine/Orc/ExecutorProcessControl.cpp b/llvm/lib/ExecutionEngine/Orc/ExecutorProcessControl.cpp
index efafca949e61ef..ff9cba7ea9995d 100644
--- a/llvm/lib/ExecutionEngine/Orc/ExecutorProcessControl.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/ExecutorProcessControl.cpp
@@ -25,11 +25,16 @@ ExecutorProcessControl::MemoryAccess::~MemoryAccess() = default;
 
 ExecutorProcessControl::~ExecutorProcessControl() = default;
 
+std::shared_ptr<SymbolStringPool>
+ExecutorProcessControl::getSymbolStringPool() const {
+  assert(ES && "Create an ExecutionSession with this EPC first");
+  return ES->getSymbolStringPool();
+}
+
 SelfExecutorProcessControl::SelfExecutorProcessControl(
-    std::shared_ptr<SymbolStringPool> SSP, std::unique_ptr<TaskDispatcher> D,
-    Triple TargetTriple, unsigned PageSize,
+    std::unique_ptr<TaskDispatcher> D, Triple TargetTriple, unsigned PageSize,
     std::unique_ptr<jitlink::JITLinkMemoryManager> MemMgr)
-    : ExecutorProcessControl(std::move(SSP), std::move(D)),
+    : ExecutorProcessControl(std::move(D)),
       InProcessMemoryAccess(TargetTriple.isArch64Bit()) {
 
   OwnedMemMgr = std::move(MemMgr);
@@ -54,13 +59,8 @@ SelfExecutorProcessControl::SelfExecutorProcessControl(
 
 Expected<std::unique_ptr<SelfExecutorProcessControl>>
 SelfExecutorProcessControl::Create(
-    std::shared_ptr<SymbolStringPool> SSP,
     std::unique_ptr<TaskDispatcher> D,
     std::unique_ptr<jitlink::JITLinkMemoryManager> MemMgr) {
-
-  if (!SSP)
-    SSP = std::make_shared<SymbolStringPool>();
-
   if (!D) {
 #if LLVM_ENABLE_THREADS
     D = std::make_unique<DynamicThreadPoolTaskDispatcher>();
@@ -76,8 +76,7 @@ SelfExecutorProcessControl::Create(
   Triple TT(sys::getProcessTriple());
 
   return std::make_unique<SelfExecutorProcessControl>(
-      std::move(SSP), std::move(D), std::move(TT), *PageSize,
-      std::move(MemMgr));
+      std::move(D), std::move(TT), *PageSize, std::move(MemMgr));
 }
 
 Expected<tpctypes::DylibHandle>
@@ -106,8 +105,8 @@ void SelfExecutorProcessControl::lookupSymbolsAsync(
         // FIXME: Collect all failing symbols before erroring out.
         SymbolNameVector MissingSymbols;
         MissingSymbols.push_back(Sym);
-        return Complete(
-            make_error<SymbolsNotFound>(SSP, std::move(MissingSymbols)));
+        return Complete(make_error<SymbolsNotFound>(ES->getSymbolStringPool(),
+                                                    std::move(MissingSymbols)));
       }
       // FIXME: determine accurate JITSymbolFlags.
       R.back().push_back(
diff --git a/llvm/tools/lli/lli.cpp b/llvm/tools/lli/lli.cpp
index 25f43a4bb6811b..348caab9608ee4 100644
--- a/llvm/tools/lli/lli.cpp
+++ b/llvm/tools/lli/lli.cpp
@@ -1024,8 +1024,7 @@ int runOrcJIT(const char *ProgName) {
 
   std::unique_ptr<orc::ExecutorProcessControl> EPC = nullptr;
   if (JITLinker == JITLinkerKind::JITLink) {
-    EPC = ExitOnErr(orc::SelfExecutorProcessControl::Create(
-        std::make_shared<orc::SymbolStringPool>()));
+    EPC = ExitOnErr(orc::SelfExecutorProcessControl::Create());
 
     Builder.getJITTargetMachineBuilder()
         ->setRelocationModel(Reloc::PIC_)
diff --git a/llvm/tools/llvm-jitlink/llvm-jitlink.cpp b/llvm/tools/llvm-jitlink/llvm-jitlink.cpp
index 09b2a5900eb0b7..b6c0c319e4548c 100644
--- a/llvm/tools/llvm-jitlink/llvm-jitlink.cpp
+++ b/llvm/tools/llvm-jitlink/llvm-jitlink.cpp
@@ -936,7 +936,6 @@ Expected<std::unique_ptr<Session>> Session::Create(Triple TT,
     if (!PageSize)
       return PageSize.takeError();
     EPC = std::make_unique<SelfExecutorProcessControl>(
-        std::make_shared<SymbolStringPool>(),
         std::make_unique<InPlaceTaskDispatcher>(), std::move(TT), *PageSize,
         createInProcessMemoryManager());
   }
diff --git a/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp b/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
index 5e2b5f35bcf471..28da8952e08069 100644
--- a/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
+++ b/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
@@ -1711,8 +1711,7 @@ TEST_F(CoreAPIsStandardTest, RemoveJITDylibs) {
 TEST(CoreAPIsExtraTest, SessionTeardownByFailedToMaterialize) {
 
   auto RunTestCase = []() -> Error {
-    ExecutionSession ES{std::make_unique<UnsupportedExecutorProcessControl>(
-        std::make_shared<SymbolStringPool>())};
+    ExecutionSession ES{std::make_unique<UnsupportedExecutorProcessControl>()};
     auto Foo = ES.intern("foo");
     auto FooFlags = JITSymbolFlags::Exported;
 
diff --git a/llvm/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp b/llvm/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp
index 7ab3e40df7459d..3b073b684687d7 100644
--- a/llvm/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp
+++ b/llvm/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp
@@ -182,8 +182,7 @@ TEST(ObjectLinkingLayerSearchGeneratorTest, AbsoluteSymbolsObjectLayer) {
   class TestEPC : public UnsupportedExecutorProcessControl {
   public:
     TestEPC()
-        : UnsupportedExecutorProcessControl(nullptr, nullptr,
-                                            "x86_64-apple-darwin") {}
+        : UnsupportedExecutorProcessControl(nullptr, "x86_64-apple-darwin") {}
 
     Expected<tpctypes::DylibHandle> loadDylib(const char *DylibPath) override {
       return ExecutorAddr::fromPtr((void *)nullptr);
diff --git a/llvm/unittests/ExecutionEngine/Orc/OrcTestCommon.h b/llvm/unittests/ExecutionEngine/Orc/OrcTestCommon.h
index ce7da76c9653a3..2e5ce50d2fea2a 100644
--- a/llvm/unittests/ExecutionEngine/Orc/OrcTestCommon.h
+++ b/llvm/unittests/ExecutionEngine/Orc/OrcTestCommon.h
@@ -52,8 +52,7 @@ class CoreAPIsBasedStandardTest : public testing::Test {
   }
 
 protected:
-  std::shared_ptr<SymbolStringPool> SSP = std::make_shared<SymbolStringPool>();
-  ExecutionSession ES{std::make_unique<UnsupportedExecutorProcessControl>(SSP)};
+  ExecutionSession ES{std::make_unique<UnsupportedExecutorProcessControl>()};
   JITDylib &JD = ES.createBareJITDylib("JD");
   SymbolStringPtr Foo = ES.intern("foo");
   SymbolStringPtr Bar = ES.intern("bar");

>From 573733ac30aa8b51122a507c92d700213f4596e6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Wed, 13 Mar 2024 11:11:57 +0100
Subject: [PATCH 2/3] Drop SSP default value (optional)

---
 .../Kaleidoscope/include/KaleidoscopeJIT.h       |  4 +++-
 llvm/lib/ExecutionEngine/Orc/LLJIT.cpp           | 10 ++++++----
 llvm/tools/lli/lli.cpp                           |  3 ++-
 llvm/tools/llvm-jitlink/llvm-jitlink.cpp         |  2 +-
 .../ExecutionEngine/Orc/CoreAPIsTest.cpp         | 16 ++++++++++++----
 .../ExecutionSessionWrapperFunctionCallsTest.cpp | 12 ++++++++----
 .../Orc/ObjectLinkingLayerTest.cpp               |  6 ++++--
 .../ExecutionEngine/Orc/OrcTestCommon.h          |  4 +++-
 .../Orc/RTDyldObjectLinkingLayerTest.cpp         | 12 +++++++++---
 9 files changed, 48 insertions(+), 21 deletions(-)

diff --git a/llvm/examples/Kaleidoscope/include/KaleidoscopeJIT.h b/llvm/examples/Kaleidoscope/include/KaleidoscopeJIT.h
index 778437cd8a3d65..163f0833edd043 100644
--- a/llvm/examples/Kaleidoscope/include/KaleidoscopeJIT.h
+++ b/llvm/examples/Kaleidoscope/include/KaleidoscopeJIT.h
@@ -71,7 +71,9 @@ class KaleidoscopeJIT {
     if (!EPC)
       return EPC.takeError();
 
-    auto ES = std::make_unique<ExecutionSession>(std::move(*EPC));
+    auto SSP = std::make_shared<SymbolStringPool>();
+    auto ES =
+        std::make_unique<ExecutionSession>(std::move(*EPC), std::move(SSP));
 
     JITTargetMachineBuilder JTMB(
         ES->getExecutorProcessControl().getTargetTriple());
diff --git a/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp b/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
index 79adda5b7bc034..e01607ea4879bb 100644
--- a/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
@@ -936,12 +936,14 @@ LLJIT::LLJIT(LLJITBuilderState &S, Error &Err)
   assert(!(S.EPC && S.ES) && "EPC and ES should not both be set");
 
   if (S.EPC) {
-    ES = std::make_unique<ExecutionSession>(std::move(S.EPC));
-  } else if (S.ES)
+    auto SSP = std::make_shared<SymbolStringPool>();
+    ES = std::make_unique<ExecutionSession>(std::move(S.EPC), std::move(SSP));
+  } else if (S.ES) {
     ES = std::move(S.ES);
-  else {
+  } else {
     if (auto EPC = SelfExecutorProcessControl::Create()) {
-      ES = std::make_unique<ExecutionSession>(std::move(*EPC));
+      auto SSP = std::make_shared<SymbolStringPool>();
+      ES = std::make_unique<ExecutionSession>(std::move(*EPC), std::move(SSP));
     } else {
       Err = EPC.takeError();
       return;
diff --git a/llvm/tools/lli/lli.cpp b/llvm/tools/lli/lli.cpp
index 348caab9608ee4..aa993eaaecb616 100644
--- a/llvm/tools/lli/lli.cpp
+++ b/llvm/tools/lli/lli.cpp
@@ -964,7 +964,8 @@ int runOrcJIT(const char *ProgName) {
   // unsupported architectures).
   if (UseJITKind != JITKind::OrcLazy) {
     auto ES = std::make_unique<orc::ExecutionSession>(
-        ExitOnErr(orc::SelfExecutorProcessControl::Create()));
+        ExitOnErr(orc::SelfExecutorProcessControl::Create()),
+        std::make_shared<orc::SymbolStringPool>());
     Builder.setLazyCallthroughManager(
         std::make_unique<orc::LazyCallThroughManager>(*ES, orc::ExecutorAddr(),
                                                       nullptr));
diff --git a/llvm/tools/llvm-jitlink/llvm-jitlink.cpp b/llvm/tools/llvm-jitlink/llvm-jitlink.cpp
index b6c0c319e4548c..05811e15a8f010 100644
--- a/llvm/tools/llvm-jitlink/llvm-jitlink.cpp
+++ b/llvm/tools/llvm-jitlink/llvm-jitlink.cpp
@@ -954,7 +954,7 @@ Session::~Session() {
 }
 
 Session::Session(std::unique_ptr<ExecutorProcessControl> EPC, Error &Err)
-    : ES(std::move(EPC)),
+    : ES(std::move(EPC), std::make_shared<SymbolStringPool>()),
       ObjLayer(ES, ES.getExecutorProcessControl().getMemMgr()) {
 
   /// Local ObjectLinkingLayer::Plugin class to forward modifyPassConfig to the
diff --git a/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp b/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
index 28da8952e08069..9be44d8a1ac54a 100644
--- a/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
+++ b/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
@@ -1561,7 +1561,9 @@ TEST(JITDylibTest, GetDFSLinkOrderTree) {
   // Test that DFS ordering behaves as expected when the linkage relationships
   // form a tree.
 
-  ExecutionSession ES{std::make_unique<UnsupportedExecutorProcessControl>()};
+  auto SSP = std::make_shared<SymbolStringPool>();
+  ExecutionSession ES{std::make_unique<UnsupportedExecutorProcessControl>(),
+                      SSP};
   auto _ = make_scope_exit([&]() { cantFail(ES.endSession()); });
 
   auto &LibA = ES.createBareJITDylib("A");
@@ -1603,7 +1605,9 @@ TEST(JITDylibTest, GetDFSLinkOrderDiamond) {
   // Test that DFS ordering behaves as expected when the linkage relationships
   // contain a diamond.
 
-  ExecutionSession ES{std::make_unique<UnsupportedExecutorProcessControl>()};
+  auto SSP = std::make_shared<SymbolStringPool>();
+  ExecutionSession ES{std::make_unique<UnsupportedExecutorProcessControl>(),
+                      SSP};
   auto _ = make_scope_exit([&]() { cantFail(ES.endSession()); });
 
   auto &LibA = ES.createBareJITDylib("A");
@@ -1627,7 +1631,9 @@ TEST(JITDylibTest, GetDFSLinkOrderCycle) {
   // Test that DFS ordering behaves as expected when the linkage relationships
   // contain a cycle.
 
-  ExecutionSession ES{std::make_unique<UnsupportedExecutorProcessControl>()};
+  auto SSP = std::make_shared<SymbolStringPool>();
+  ExecutionSession ES{std::make_unique<UnsupportedExecutorProcessControl>(),
+                      SSP};
   auto _ = make_scope_exit([&]() { cantFail(ES.endSession()); });
 
   auto &LibA = ES.createBareJITDylib("A");
@@ -1711,7 +1717,9 @@ TEST_F(CoreAPIsStandardTest, RemoveJITDylibs) {
 TEST(CoreAPIsExtraTest, SessionTeardownByFailedToMaterialize) {
 
   auto RunTestCase = []() -> Error {
-    ExecutionSession ES{std::make_unique<UnsupportedExecutorProcessControl>()};
+    auto SSP = std::make_shared<SymbolStringPool>();
+    ExecutionSession ES{std::make_unique<UnsupportedExecutorProcessControl>(),
+                        SSP};
     auto Foo = ES.intern("foo");
     auto FooFlags = JITSymbolFlags::Exported;
 
diff --git a/llvm/unittests/ExecutionEngine/Orc/ExecutionSessionWrapperFunctionCallsTest.cpp b/llvm/unittests/ExecutionEngine/Orc/ExecutionSessionWrapperFunctionCallsTest.cpp
index 1b79e12ee168c8..02113f304b1774 100644
--- a/llvm/unittests/ExecutionEngine/Orc/ExecutionSessionWrapperFunctionCallsTest.cpp
+++ b/llvm/unittests/ExecutionEngine/Orc/ExecutionSessionWrapperFunctionCallsTest.cpp
@@ -36,7 +36,8 @@ voidWrapper(const char *ArgData, size_t ArgSize) {
 }
 
 TEST(ExecutionSessionWrapperFunctionCalls, RunWrapperTemplate) {
-  ExecutionSession ES(cantFail(SelfExecutorProcessControl::Create()));
+  auto SSP = std::make_shared<SymbolStringPool>();
+  ExecutionSession ES(cantFail(SelfExecutorProcessControl::Create()), SSP);
 
   int32_t Result;
   EXPECT_THAT_ERROR(ES.callSPSWrapper<int32_t(int32_t, int32_t)>(
@@ -47,7 +48,8 @@ TEST(ExecutionSessionWrapperFunctionCalls, RunWrapperTemplate) {
 }
 
 TEST(ExecutionSessionWrapperFunctionCalls, RunVoidWrapperAsyncTemplate) {
-  ExecutionSession ES(cantFail(SelfExecutorProcessControl::Create()));
+  auto SSP = std::make_shared<SymbolStringPool>();
+  ExecutionSession ES(cantFail(SelfExecutorProcessControl::Create()), SSP);
 
   std::promise<MSVCPError> RP;
   ES.callSPSWrapperAsync<void()>(ExecutorAddr::fromPtr(voidWrapper),
@@ -60,7 +62,8 @@ TEST(ExecutionSessionWrapperFunctionCalls, RunVoidWrapperAsyncTemplate) {
 }
 
 TEST(ExecutionSessionWrapperFunctionCalls, RunNonVoidWrapperAsyncTemplate) {
-  ExecutionSession ES(cantFail(SelfExecutorProcessControl::Create()));
+  auto SSP = std::make_shared<SymbolStringPool>();
+  ExecutionSession ES(cantFail(SelfExecutorProcessControl::Create()), SSP);
 
   std::promise<MSVCPExpected<int32_t>> RP;
   ES.callSPSWrapperAsync<int32_t(int32_t, int32_t)>(
@@ -80,7 +83,8 @@ TEST(ExecutionSessionWrapperFunctionCalls, RegisterAsyncHandlerAndRun) {
 
   constexpr ExecutorAddr AddAsyncTagAddr(0x01);
 
-  ExecutionSession ES(cantFail(SelfExecutorProcessControl::Create()));
+  auto SSP = std::make_shared<SymbolStringPool>();
+  ExecutionSession ES(cantFail(SelfExecutorProcessControl::Create()), SSP);
   auto &JD = ES.createBareJITDylib("JD");
 
   auto AddAsyncTag = ES.intern("addAsync_tag");
diff --git a/llvm/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp b/llvm/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp
index 3b073b684687d7..7106d12012f3ff 100644
--- a/llvm/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp
+++ b/llvm/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp
@@ -37,7 +37,8 @@ class ObjectLinkingLayerTest : public testing::Test {
   }
 
 protected:
-  ExecutionSession ES{std::make_unique<UnsupportedExecutorProcessControl>()};
+  ExecutionSession ES{std::make_unique<UnsupportedExecutorProcessControl>(),
+                      std::make_shared<SymbolStringPool>()};
   JITDylib &JD = ES.createBareJITDylib("main");
   ObjectLinkingLayer ObjLinkingLayer{
       ES, std::make_unique<InProcessMemoryManager>(4096)};
@@ -208,7 +209,8 @@ TEST(ObjectLinkingLayerSearchGeneratorTest, AbsoluteSymbolsObjectLayer) {
     }
   };
 
-  ExecutionSession ES{std::make_unique<TestEPC>()};
+  auto SSP = std::make_shared<SymbolStringPool>();
+  ExecutionSession ES{std::make_unique<TestEPC>(), SSP};
   JITDylib &JD = ES.createBareJITDylib("main");
   ObjectLinkingLayer ObjLinkingLayer{
       ES, std::make_unique<InProcessMemoryManager>(4096)};
diff --git a/llvm/unittests/ExecutionEngine/Orc/OrcTestCommon.h b/llvm/unittests/ExecutionEngine/Orc/OrcTestCommon.h
index 2e5ce50d2fea2a..cfcc01f88bab59 100644
--- a/llvm/unittests/ExecutionEngine/Orc/OrcTestCommon.h
+++ b/llvm/unittests/ExecutionEngine/Orc/OrcTestCommon.h
@@ -52,7 +52,9 @@ class CoreAPIsBasedStandardTest : public testing::Test {
   }
 
 protected:
-  ExecutionSession ES{std::make_unique<UnsupportedExecutorProcessControl>()};
+  std::shared_ptr<SymbolStringPool> SSP = std::make_shared<SymbolStringPool>();
+  ExecutionSession ES{std::make_unique<UnsupportedExecutorProcessControl>(),
+                      SSP};
   JITDylib &JD = ES.createBareJITDylib("JD");
   SymbolStringPtr Foo = ES.intern("foo");
   SymbolStringPtr Bar = ES.intern("bar");
diff --git a/llvm/unittests/ExecutionEngine/Orc/RTDyldObjectLinkingLayerTest.cpp b/llvm/unittests/ExecutionEngine/Orc/RTDyldObjectLinkingLayerTest.cpp
index 82d6968ea9874e..124c59701ffb3b 100644
--- a/llvm/unittests/ExecutionEngine/Orc/RTDyldObjectLinkingLayerTest.cpp
+++ b/llvm/unittests/ExecutionEngine/Orc/RTDyldObjectLinkingLayerTest.cpp
@@ -46,7 +46,9 @@ static bool testSetProcessAllSections(std::unique_ptr<MemoryBuffer> Obj,
 
   bool NonAllocSectionSeen = false;
 
-  ExecutionSession ES(std::make_unique<UnsupportedExecutorProcessControl>());
+  std::shared_ptr<SymbolStringPool> SSP = std::make_shared<SymbolStringPool>();
+  ExecutionSession ES{std::make_unique<UnsupportedExecutorProcessControl>(),
+                      SSP};
   auto &JD = ES.createBareJITDylib("main");
   auto Foo = ES.intern("foo");
 
@@ -153,7 +155,9 @@ TEST(RTDyldObjectLinkingLayerTest, TestOverrideObjectFlags) {
   }
 
   // Create a simple stack and set the override flags option.
-  ExecutionSession ES{std::make_unique<UnsupportedExecutorProcessControl>()};
+  std::shared_ptr<SymbolStringPool> SSP = std::make_shared<SymbolStringPool>();
+  ExecutionSession ES{std::make_unique<UnsupportedExecutorProcessControl>(),
+                      SSP};
   auto &JD = ES.createBareJITDylib("main");
   auto Foo = ES.intern("foo");
   RTDyldObjectLinkingLayer ObjLayer(
@@ -223,7 +227,9 @@ TEST(RTDyldObjectLinkingLayerTest, TestAutoClaimResponsibilityForSymbols) {
   }
 
   // Create a simple stack and set the override flags option.
-  ExecutionSession ES{std::make_unique<UnsupportedExecutorProcessControl>()};
+  std::shared_ptr<SymbolStringPool> SSP = std::make_shared<SymbolStringPool>();
+  ExecutionSession ES{std::make_unique<UnsupportedExecutorProcessControl>(),
+                      SSP};
   auto &JD = ES.createBareJITDylib("main");
   auto Foo = ES.intern("foo");
   RTDyldObjectLinkingLayer ObjLayer(

>From e93e9a676858728626bd8951c6e6468e613d9022 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Wed, 13 Mar 2024 11:39:58 +0100
Subject: [PATCH 3/3] fixup! [Orc] Move SymbolStringPool from EPC to
 ExecutionSession (NFC)

---
 .../llvm/ExecutionEngine/Orc/ExecutorProcessControl.h  | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/llvm/include/llvm/ExecutionEngine/Orc/ExecutorProcessControl.h b/llvm/include/llvm/ExecutionEngine/Orc/ExecutorProcessControl.h
index 1673e3a0fe6b59..392ca7b20eaa91 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/ExecutorProcessControl.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/ExecutorProcessControl.h
@@ -457,11 +457,11 @@ class InProcessMemoryAccess : public ExecutorProcessControl::MemoryAccess {
 class UnsupportedExecutorProcessControl : public ExecutorProcessControl,
                                           private InProcessMemoryAccess {
 public:
-  UnsupportedExecutorProcessControl(
-      std::unique_ptr<TaskDispatcher> D = nullptr, const std::string &TT = "",
-      unsigned PageSize = 0)
-      : ExecutorProcessControl(
-            D ? std::move(D) : std::make_unique<InPlaceTaskDispatcher>()),
+  UnsupportedExecutorProcessControl(std::unique_ptr<TaskDispatcher> D = nullptr,
+                                    const std::string &TT = "",
+                                    unsigned PageSize = 0)
+      : ExecutorProcessControl(D ? std::move(D)
+                                 : std::make_unique<InPlaceTaskDispatcher>()),
         InProcessMemoryAccess(Triple(TT).isArch64Bit()) {
     this->TargetTriple = Triple(TT);
     this->PageSize = PageSize;



More information about the llvm-commits mailing list