[JITLink] Switch to SymbolStringPtr for Symbol names (#115796)

Use SymbolStringPtr for Symbol names in LinkGraph. This reduces string interning
on the boundary between JITLink and ORC, and allows pointer comparisons (rather
than string comparisons) between Symbol names. This should improve the
performance and readability of code that bridges between JITLink and ORC (e.g.
ObjectLinkingLayer and ObjectLinkingLayer::Plugins).

To enable use of SymbolStringPtr a std::shared_ptr<SymbolStringPool> is added to
LinkGraph and threaded through to its construction sites in LLVM and Bolt. All
LinkGraphs that are to have symbol names compared by pointer equality must point
to the same SymbolStringPool instance, which in ORC sessions should be the pool
attached to the ExecutionSession.
---------

Co-authored-by: Lang Hames <lhames@gmail.com>

1 files changed, 20 insertions, 37 deletions

diff --git a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
index c5342c4..dd15499 100644
--- a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
@@ -100,8 +100,7 @@ private:
         return;
       assert(Sym->hasName() && "Anonymous non-local symbol?");
 
-      LGI.SymbolFlags[ES.intern(Sym->getName())] =
-          getJITSymbolFlagsForSymbol(*Sym);
+      LGI.SymbolFlags[Sym->getName()] = getJITSymbolFlagsForSymbol(*Sym);
     };
 
     for (auto *Sym : G.defined_symbols())
@@ -129,7 +128,7 @@ private:
 
   void discard(const JITDylib &JD, const SymbolStringPtr &Name) override {
     for (auto *Sym : G->defined_symbols())
-      if (Sym->getName() == *Name) {
+      if (Sym->getName() == Name) {
         assert(Sym->getLinkage() == Linkage::Weak &&
                "Discarding non-weak definition");
         G->makeExternal(*Sym);
@@ -204,7 +203,7 @@ public:
         LookupFlags = orc::SymbolLookupFlags::WeaklyReferencedSymbol;
         break;
       }
-      LookupSet.add(ES.intern(KV.first), LookupFlags);
+      LookupSet.add(KV.first, LookupFlags);
     }
 
     // OnResolve -- De-intern the symbols and pass the result to the linker.
@@ -215,7 +214,7 @@ public:
       else {
         AsyncLookupResult LR;
         for (auto &KV : *Result)
-          LR[*KV.first] = KV.second;
+          LR[KV.first] = KV.second;
         LookupContinuation->run(std::move(LR));
       }
     };
@@ -231,7 +230,6 @@ public:
   }
 
   Error notifyResolved(LinkGraph &G) override {
-    auto &ES = Layer.getExecutionSession();
 
     SymbolFlagsMap ExtraSymbolsToClaim;
     bool AutoClaim = Layer.AutoClaimObjectSymbols;
@@ -239,27 +237,25 @@ public:
     SymbolMap InternedResult;
     for (auto *Sym : G.defined_symbols())
       if (Sym->getScope() < Scope::SideEffectsOnly) {
-        auto InternedName = ES.intern(Sym->getName());
         auto Ptr = getJITSymbolPtrForSymbol(*Sym, G.getTargetTriple());
         auto Flags = getJITSymbolFlagsForSymbol(*Sym);
-        InternedResult[InternedName] = {Ptr, Flags};
-        if (AutoClaim && !MR->getSymbols().count(InternedName)) {
-          assert(!ExtraSymbolsToClaim.count(InternedName) &&
+        InternedResult[Sym->getName()] = {Ptr, Flags};
+        if (AutoClaim && !MR->getSymbols().count(Sym->getName())) {
+          assert(!ExtraSymbolsToClaim.count(Sym->getName()) &&
                  "Duplicate symbol to claim?");
-          ExtraSymbolsToClaim[InternedName] = Flags;
+          ExtraSymbolsToClaim[Sym->getName()] = Flags;
         }
       }
 
     for (auto *Sym : G.absolute_symbols())
       if (Sym->getScope() < Scope::SideEffectsOnly) {
-        auto InternedName = ES.intern(Sym->getName());
         auto Ptr = getJITSymbolPtrForSymbol(*Sym, G.getTargetTriple());
         auto Flags = getJITSymbolFlagsForSymbol(*Sym);
-        InternedResult[InternedName] = {Ptr, Flags};
-        if (AutoClaim && !MR->getSymbols().count(InternedName)) {
-          assert(!ExtraSymbolsToClaim.count(InternedName) &&
+        InternedResult[Sym->getName()] = {Ptr, Flags};
+        if (AutoClaim && !MR->getSymbols().count(Sym->getName())) {
+          assert(!ExtraSymbolsToClaim.count(Sym->getName()) &&
                  "Duplicate symbol to claim?");
-          ExtraSymbolsToClaim[InternedName] = Flags;
+          ExtraSymbolsToClaim[Sym->getName()] = Flags;
         }
       }
 
@@ -379,19 +375,16 @@ public:
 
 private:
   Error claimOrExternalizeWeakAndCommonSymbols(LinkGraph &G) {
-    auto &ES = Layer.getExecutionSession();
-
     SymbolFlagsMap NewSymbolsToClaim;
     std::vector<std::pair<SymbolStringPtr, Symbol *>> NameToSym;
 
     auto ProcessSymbol = [&](Symbol *Sym) {
       if (Sym->hasName() && Sym->getLinkage() == Linkage::Weak &&
           Sym->getScope() != Scope::Local) {
-        auto Name = ES.intern(Sym->getName());
-        if (!MR->getSymbols().count(ES.intern(Sym->getName()))) {
-          NewSymbolsToClaim[Name] =
+        if (!MR->getSymbols().count(Sym->getName())) {
+          NewSymbolsToClaim[Sym->getName()] =
               getJITSymbolFlagsForSymbol(*Sym) | JITSymbolFlags::Weak;
-          NameToSym.push_back(std::make_pair(std::move(Name), Sym));
+          NameToSym.push_back(std::make_pair(Sym->getName(), Sym));
         }
       }
     };
@@ -421,9 +414,8 @@ private:
   }
 
   Error markResponsibilitySymbolsLive(LinkGraph &G) const {
-    auto &ES = Layer.getExecutionSession();
     for (auto *Sym : G.defined_symbols())
-      if (Sym->hasName() && MR->getSymbols().count(ES.intern(Sym->getName())))
+      if (Sym->hasName() && MR->getSymbols().count(Sym->getName()))
         Sym->setLive(true);
     return Error::success();
   }
@@ -558,15 +550,6 @@ private:
     // SymbolDependenceGroups (in the SymbolDepGroups member), ready for use in
     // the upcoming notifyFinalized call.
     auto &TargetJD = MR->getTargetJITDylib();
-    auto &ES = TargetJD.getExecutionSession();
-
-    DenseMap<Symbol *, SymbolStringPtr> InternedNames;
-    auto GetInternedName = [&](Symbol *S) {
-      auto &Name = InternedNames[S];
-      if (!Name)
-        Name = ES.intern(S->getName());
-      return Name;
-    };
 
     for (auto &[B, BI] : BlockInfos) {
       if (!BI.Defs.empty()) {
@@ -574,10 +557,10 @@ private:
         auto &SDG = SymbolDepGroups.back();
 
         for (auto *Def : BI.Defs)
-          SDG.Symbols.insert(GetInternedName(Def));
+          SDG.Symbols.insert(Def->getName());
 
         for (auto *Dep : BI.SymbolDeps) {
-          auto DepName = GetInternedName(Dep);
+          auto DepName = Dep->getName();
           if (Dep->isDefined())
             SDG.Dependencies[&TargetJD].insert(std::move(DepName));
           else {
@@ -643,8 +626,8 @@ void ObjectLinkingLayer::emit(std::unique_ptr<MaterializationResponsibility> R,
 
   auto Ctx = std::make_unique<ObjectLinkingLayerJITLinkContext>(
       *this, std::move(R), std::move(O));
-
-  if (auto G = createLinkGraphFromObject(ObjBuffer)) {
+  if (auto G = createLinkGraphFromObject(
+          ObjBuffer, getExecutionSession().getSymbolStringPool())) {
     Ctx->notifyMaterializing(**G);
     link(std::move(*G), std::move(Ctx));
   } else {