[JITLink] Switch to SymbolStringPtr for Symbol names (llvm#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 <[email protected]>

.gitignore

@@ -59,6 +59,8 @@ autoconf/autom4te.cache
 # VS2017 and VSCode config files.
 .vscode
 .vs
+#zed config files
+.zed
 # pythonenv for github Codespaces
 pythonenv*
 # clangd index. (".clangd" is a config file now, thus trailing slash)

bolt/include/bolt/Core/BinaryContext.h

@@ -28,6 +28,7 @@
 #include "llvm/ADT/iterator.h"
 #include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/BinaryFormat/MachO.h"
+#include "llvm/ExecutionEngine/Orc/SymbolStringPool.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCCodeEmitter.h"
 #include "llvm/MC/MCContext.h"
@@ -276,11 +277,10 @@ class BinaryContext {
   void deregisterSectionName(const BinarySection &Section);
 
 public:
-  static Expected<std::unique_ptr<BinaryContext>>
-  createBinaryContext(Triple TheTriple, StringRef InputFileName,
-                      SubtargetFeatures *Features, bool IsPIC,
-                      std::unique_ptr<DWARFContext> DwCtx,
-                      JournalingStreams Logger);
+  static Expected<std::unique_ptr<BinaryContext>> createBinaryContext(
+      Triple TheTriple, std::shared_ptr<orc::SymbolStringPool> SSP,
+      StringRef InputFileName, SubtargetFeatures *Features, bool IsPIC,
+      std::unique_ptr<DWARFContext> DwCtx, JournalingStreams Logger);
 
   /// Superset of compiler units that will contain overwritten code that needs
   /// new debug info. In a few cases, functions may end up not being
@@ -372,6 +372,7 @@ class BinaryContext {
   bool hasSymbolsWithFileName() const { return HasSymbolsWithFileName; }
   void setHasSymbolsWithFileName(bool Value) { HasSymbolsWithFileName = Value; }
 
+  std::shared_ptr<orc::SymbolStringPool> getSymbolStringPool() { return SSP; }
   /// Return true if relocations against symbol with a given name
   /// must be created.
   bool forceSymbolRelocations(StringRef SymbolName) const;
@@ -631,6 +632,8 @@ class BinaryContext {
 
   std::unique_ptr<Triple> TheTriple;
 
+  std::shared_ptr<orc::SymbolStringPool> SSP;
+
   const Target *TheTarget;
 
   std::string TripleName;
@@ -807,8 +810,10 @@ class BinaryContext {
 
   BinaryContext(std::unique_ptr<MCContext> Ctx,
                 std::unique_ptr<DWARFContext> DwCtx,
-                std::unique_ptr<Triple> TheTriple, const Target *TheTarget,
-                std::string TripleName, std::unique_ptr<MCCodeEmitter> MCE,
+                std::unique_ptr<Triple> TheTriple,
+                std::shared_ptr<orc::SymbolStringPool> SSP,
+                const Target *TheTarget, std::string TripleName,
+                std::unique_ptr<MCCodeEmitter> MCE,
                 std::unique_ptr<MCObjectFileInfo> MOFI,
                 std::unique_ptr<const MCAsmInfo> AsmInfo,
                 std::unique_ptr<const MCInstrInfo> MII,

bolt/lib/Core/BinaryContext.cpp

@@ -123,6 +123,7 @@ void BinaryContext::logBOLTErrorsAndQuitOnFatal(Error E) {
 BinaryContext::BinaryContext(std::unique_ptr<MCContext> Ctx,
                              std::unique_ptr<DWARFContext> DwCtx,
                              std::unique_ptr<Triple> TheTriple,
+                             std::shared_ptr<orc::SymbolStringPool> SSP,
                              const Target *TheTarget, std::string TripleName,
                              std::unique_ptr<MCCodeEmitter> MCE,
                              std::unique_ptr<MCObjectFileInfo> MOFI,
@@ -136,12 +137,12 @@ BinaryContext::BinaryContext(std::unique_ptr<MCContext> Ctx,
                              std::unique_ptr<MCDisassembler> DisAsm,
                              JournalingStreams Logger)
     : Ctx(std::move(Ctx)), DwCtx(std::move(DwCtx)),
-      TheTriple(std::move(TheTriple)), TheTarget(TheTarget),
-      TripleName(TripleName), MCE(std::move(MCE)), MOFI(std::move(MOFI)),
-      AsmInfo(std::move(AsmInfo)), MII(std::move(MII)), STI(std::move(STI)),
-      InstPrinter(std::move(InstPrinter)), MIA(std::move(MIA)),
-      MIB(std::move(MIB)), MRI(std::move(MRI)), DisAsm(std::move(DisAsm)),
-      Logger(Logger), InitialDynoStats(isAArch64()) {
+      TheTriple(std::move(TheTriple)), SSP(std::move(SSP)),
+      TheTarget(TheTarget), TripleName(TripleName), MCE(std::move(MCE)),
+      MOFI(std::move(MOFI)), AsmInfo(std::move(AsmInfo)), MII(std::move(MII)),
+      STI(std::move(STI)), InstPrinter(std::move(InstPrinter)),
+      MIA(std::move(MIA)), MIB(std::move(MIB)), MRI(std::move(MRI)),
+      DisAsm(std::move(DisAsm)), Logger(Logger), InitialDynoStats(isAArch64()) {
   RegularPageSize = isAArch64() ? RegularPageSizeAArch64 : RegularPageSizeX86;
   PageAlign = opts::NoHugePages ? RegularPageSize : HugePageSize;
 }
@@ -159,8 +160,9 @@ BinaryContext::~BinaryContext() {
 /// Create BinaryContext for a given architecture \p ArchName and
 /// triple \p TripleName.
 Expected<std::unique_ptr<BinaryContext>> BinaryContext::createBinaryContext(
-    Triple TheTriple, StringRef InputFileName, SubtargetFeatures *Features,
-    bool IsPIC, std::unique_ptr<DWARFContext> DwCtx, JournalingStreams Logger) {
+    Triple TheTriple, std::shared_ptr<orc::SymbolStringPool> SSP,
+    StringRef InputFileName, SubtargetFeatures *Features, bool IsPIC,
+    std::unique_ptr<DWARFContext> DwCtx, JournalingStreams Logger) {
   StringRef ArchName = "";
   std::string FeaturesStr = "";
   switch (TheTriple.getArch()) {
@@ -283,8 +285,8 @@ Expected<std::unique_ptr<BinaryContext>> BinaryContext::createBinaryContext(
 
   auto BC = std::make_unique<BinaryContext>(
       std::move(Ctx), std::move(DwCtx), std::make_unique<Triple>(TheTriple),
-      TheTarget, std::string(TripleName), std::move(MCE), std::move(MOFI),
-      std::move(AsmInfo), std::move(MII), std::move(STI),
+      std::move(SSP), TheTarget, std::string(TripleName), std::move(MCE),
+      std::move(MOFI), std::move(AsmInfo), std::move(MII), std::move(STI),
       std::move(InstructionPrinter), std::move(MIA), nullptr, std::move(MRI),
       std::move(DisAsm), Logger);
 

bolt/lib/Rewrite/DWARFRewriter.cpp

@@ -1691,7 +1691,8 @@ namespace {
 std::unique_ptr<BinaryContext>
 createDwarfOnlyBC(const object::ObjectFile &File) {
   return cantFail(BinaryContext::createBinaryContext(
-      File.makeTriple(), File.getFileName(), nullptr, false,
+      File.makeTriple(), std::make_shared<orc::SymbolStringPool>(),
+      File.getFileName(), nullptr, false,
       DWARFContext::create(File, DWARFContext::ProcessDebugRelocations::Ignore,
                            nullptr, "", WithColor::defaultErrorHandler,
                            WithColor::defaultWarningHandler),

bolt/lib/Rewrite/JITLinkLinker.cpp

@@ -122,7 +122,7 @@ struct JITLinkLinker::Context : jitlink::JITLinkContext {
     jitlink::AsyncLookupResult AllResults;
 
     for (const auto &Symbol : Symbols) {
-      std::string SymName = Symbol.first.str();
+      std::string SymName = (*Symbol.first).str();
       LLVM_DEBUG(dbgs() << "BOLT: looking for " << SymName << "\n");
 
       if (auto Address = Linker.lookupSymbol(SymName)) {
@@ -167,7 +167,9 @@ struct JITLinkLinker::Context : jitlink::JITLinkContext {
   Error notifyResolved(jitlink::LinkGraph &G) override {
     for (auto *Symbol : G.defined_symbols()) {
       SymbolInfo Info{Symbol->getAddress().getValue(), Symbol->getSize()};
-      Linker.Symtab.insert({Symbol->getName().str(), Info});
+      auto Name =
+          Symbol->hasName() ? (*Symbol->getName()).str() : std::string();
+      Linker.Symtab.insert({std::move(Name), Info});
     }
 
     return Error::success();
@@ -189,7 +191,7 @@ JITLinkLinker::~JITLinkLinker() { cantFail(MM->deallocate(std::move(Allocs))); }
 
 void JITLinkLinker::loadObject(MemoryBufferRef Obj,
                                SectionsMapper MapSections) {
-  auto LG = jitlink::createLinkGraphFromObject(Obj);
+  auto LG = jitlink::createLinkGraphFromObject(Obj, BC.getSymbolStringPool());
   if (auto E = LG.takeError()) {
     errs() << "BOLT-ERROR: JITLink failed: " << E << '\n';
     exit(1);

bolt/lib/Rewrite/MachORewriteInstance.cpp

@@ -74,7 +74,8 @@ MachORewriteInstance::MachORewriteInstance(object::MachOObjectFile *InputFile,
   ErrorAsOutParameter EAO(&Err);
   Relocation::Arch = InputFile->makeTriple().getArch();
   auto BCOrErr = BinaryContext::createBinaryContext(
-      InputFile->makeTriple(), InputFile->getFileName(), nullptr,
+      InputFile->makeTriple(), std::make_shared<orc::SymbolStringPool>(),
+      InputFile->getFileName(), nullptr,
       /* IsPIC */ true, DWARFContext::create(*InputFile),
       {llvm::outs(), llvm::errs()});
   if (Error E = BCOrErr.takeError()) {

bolt/lib/Rewrite/RewriteInstance.cpp

@@ -356,7 +356,8 @@ RewriteInstance::RewriteInstance(ELFObjectFileBase *File, const int Argc,
 
   Relocation::Arch = TheTriple.getArch();
   auto BCOrErr = BinaryContext::createBinaryContext(
-      TheTriple, File->getFileName(), Features.get(), IsPIC,
+      TheTriple, std::make_shared<orc::SymbolStringPool>(), File->getFileName(),
+      Features.get(), IsPIC,
       DWARFContext::create(*File, DWARFContext::ProcessDebugRelocations::Ignore,
                            nullptr, opts::DWPPathName,
                            WithColor::defaultErrorHandler,

bolt/unittests/Core/BinaryContext.cpp

@@ -48,7 +48,8 @@ struct BinaryContextTester : public testing::TestWithParam<Triple::ArchType> {
   void initializeBOLT() {
     Relocation::Arch = ObjFile->makeTriple().getArch();
     BC = cantFail(BinaryContext::createBinaryContext(
-        ObjFile->makeTriple(), ObjFile->getFileName(), nullptr, true,
+        ObjFile->makeTriple(), std::make_shared<orc::SymbolStringPool>(),
+        ObjFile->getFileName(), nullptr, true,
         DWARFContext::create(*ObjFile.get()), {llvm::outs(), llvm::errs()}));
     ASSERT_FALSE(!BC);
   }
@@ -216,4 +217,4 @@ TEST_P(BinaryContextTester, BaseAddressSegmentsSmallerThanAlignment) {
       BC->getBaseAddressForMapping(0xaaaaaaab1000, 0x1000);
   ASSERT_TRUE(BaseAddress.has_value());
   ASSERT_EQ(*BaseAddress, 0xaaaaaaaa0000ULL);
-}
+}

bolt/unittests/Core/MCPlusBuilder.cpp

@@ -58,7 +58,8 @@ struct MCPlusBuilderTester : public testing::TestWithParam<Triple::ArchType> {
   void initializeBolt() {
     Relocation::Arch = ObjFile->makeTriple().getArch();
     BC = cantFail(BinaryContext::createBinaryContext(
-        ObjFile->makeTriple(), ObjFile->getFileName(), nullptr, true,
+        ObjFile->makeTriple(), std::make_shared<orc::SymbolStringPool>(),
+        ObjFile->getFileName(), nullptr, true,
         DWARFContext::create(*ObjFile.get()), {llvm::outs(), llvm::errs()}));
     ASSERT_FALSE(!BC);
     BC->initializeTarget(std::unique_ptr<MCPlusBuilder>(

bolt/unittests/Core/MemoryMaps.cpp

@@ -59,7 +59,8 @@ struct MemoryMapsTester : public testing::TestWithParam<Triple::ArchType> {
   void initializeBOLT() {
     Relocation::Arch = ObjFile->makeTriple().getArch();
     BC = cantFail(BinaryContext::createBinaryContext(
-        ObjFile->makeTriple(), ObjFile->getFileName(), nullptr, true,
+        ObjFile->makeTriple(), std::make_shared<orc::SymbolStringPool>(),
+        ObjFile->getFileName(), nullptr, true,
         DWARFContext::create(*ObjFile.get()), {llvm::outs(), llvm::errs()}));
     ASSERT_FALSE(!BC);
   }

llvm/include/llvm/ExecutionEngine/JITLink/COFF.h

@@ -24,7 +24,8 @@ namespace jitlink {
 /// its contents. The caller is responsible for ensuring that the object buffer
 /// outlives the graph.
 Expected<std::unique_ptr<LinkGraph>>
-createLinkGraphFromCOFFObject(MemoryBufferRef ObjectBuffer);
+createLinkGraphFromCOFFObject(MemoryBufferRef ObjectBuffer,
+                              std::shared_ptr<orc::SymbolStringPool> SSP);
 
 /// Link the given graph.
 ///

llvm/include/llvm/ExecutionEngine/JITLink/COFF_x86_64.h

@@ -23,8 +23,8 @@ namespace jitlink {
 /// Note: The graph does not take ownership of the underlying buffer, nor copy
 /// its contents. The caller is responsible for ensuring that the object buffer
 /// outlives the graph.
-Expected<std::unique_ptr<LinkGraph>>
-createLinkGraphFromCOFFObject_x86_64(MemoryBufferRef ObjectBuffer);
+Expected<std::unique_ptr<LinkGraph>> createLinkGraphFromCOFFObject_x86_64(
+    MemoryBufferRef ObjectBuffer, std::shared_ptr<orc::SymbolStringPool> SSP);
 
 /// jit-link the given object buffer, which must be a COFF x86-64 object file.
 void link_COFF_x86_64(std::unique_ptr<LinkGraph> G,

llvm/include/llvm/ExecutionEngine/JITLink/ELF.h

@@ -24,7 +24,8 @@ namespace jitlink {
 /// its contents. The caller is responsible for ensuring that the object buffer
 /// outlives the graph.
 Expected<std::unique_ptr<LinkGraph>>
-createLinkGraphFromELFObject(MemoryBufferRef ObjectBuffer);
+createLinkGraphFromELFObject(MemoryBufferRef ObjectBuffer,
+                             std::shared_ptr<orc::SymbolStringPool> SSP);
 
 /// Link the given graph.
 ///

llvm/include/llvm/ExecutionEngine/JITLink/ELF_aarch32.h

@@ -24,8 +24,8 @@ namespace jitlink {
 /// Note: The graph does not take ownership of the underlying buffer, nor copy
 /// its contents. The caller is responsible for ensuring that the object buffer
 /// outlives the graph.
-Expected<std::unique_ptr<LinkGraph>>
-createLinkGraphFromELFObject_aarch32(MemoryBufferRef ObjectBuffer);
+Expected<std::unique_ptr<LinkGraph>> createLinkGraphFromELFObject_aarch32(
+    MemoryBufferRef ObjectBuffer, std::shared_ptr<orc::SymbolStringPool> SSP);
 
 /// jit-link the given object buffer, which must be an ELF arm/thumb object
 /// file.

llvm/include/llvm/ExecutionEngine/JITLink/ELF_aarch64.h

@@ -25,8 +25,8 @@ namespace jitlink {
 /// Note: The graph does not take ownership of the underlying buffer, nor copy
 /// its contents. The caller is responsible for ensuring that the object buffer
 /// outlives the graph.
-Expected<std::unique_ptr<LinkGraph>>
-createLinkGraphFromELFObject_aarch64(MemoryBufferRef ObjectBuffer);
+Expected<std::unique_ptr<LinkGraph>> createLinkGraphFromELFObject_aarch64(
+    MemoryBufferRef ObjectBuffer, std::shared_ptr<orc::SymbolStringPool> SSP);
 
 /// jit-link the given object buffer, which must be a ELF aarch64 relocatable
 /// object file.

llvm/include/llvm/ExecutionEngine/JITLink/ELF_i386.h

@@ -26,7 +26,8 @@ namespace jitlink {
 /// its contents. The caller is responsible for ensuring that the object buffer
 /// outlives the graph.
 Expected<std::unique_ptr<LinkGraph>>
-createLinkGraphFromELFObject_i386(MemoryBufferRef ObjectBuffer);
+createLinkGraphFromELFObject_i386(MemoryBufferRef ObjectBuffer,
+                                  std::shared_ptr<orc::SymbolStringPool> SSP);
 
 /// jit-link the given object buffer, which must be a ELF i386 relocatable
 /// object file.

llvm/include/llvm/ExecutionEngine/JITLink/ELF_loongarch.h

@@ -25,8 +25,8 @@ namespace jitlink {
 /// Note: The graph does not take ownership of the underlying buffer, nor copy
 /// its contents. The caller is responsible for ensuring that the object buffer
 /// outlives the graph.
-Expected<std::unique_ptr<LinkGraph>>
-createLinkGraphFromELFObject_loongarch(MemoryBufferRef ObjectBuffer);
+Expected<std::unique_ptr<LinkGraph>> createLinkGraphFromELFObject_loongarch(
+    MemoryBufferRef ObjectBuffer, std::shared_ptr<orc::SymbolStringPool> SSP);
 
 /// jit-link the given object buffer, which must be an ELF loongarch object
 /// file.

llvm/include/llvm/ExecutionEngine/JITLink/ELF_ppc64.h

@@ -25,15 +25,16 @@ namespace llvm::jitlink {
 ///
 /// WARNING: The big-endian backend has not been tested yet.
 Expected<std::unique_ptr<LinkGraph>>
-createLinkGraphFromELFObject_ppc64(MemoryBufferRef ObjectBuffer);
+createLinkGraphFromELFObject_ppc64(MemoryBufferRef ObjectBuffer,
+                                   std::shared_ptr<orc::SymbolStringPool> SSP);
 
 /// Create a LinkGraph from an ELF/ppc64le relocatable object.
 ///
 /// Note: The graph does not take ownership of the underlying buffer, nor copy
 /// its contents. The caller is responsible for ensuring that the object buffer
 /// outlives the graph.
-Expected<std::unique_ptr<LinkGraph>>
-createLinkGraphFromELFObject_ppc64le(MemoryBufferRef ObjectBuffer);
+Expected<std::unique_ptr<LinkGraph>> createLinkGraphFromELFObject_ppc64le(
+    MemoryBufferRef ObjectBuffer, std::shared_ptr<orc::SymbolStringPool> SSP);
 
 /// jit-link the given object buffer, which must be a ELF ppc64le object file.
 ///

llvm/include/llvm/ExecutionEngine/JITLink/ELF_riscv.h

@@ -26,7 +26,8 @@ namespace jitlink {
 /// its contents. The caller is responsible for ensuring that the object buffer
 /// outlives the graph.
 Expected<std::unique_ptr<LinkGraph>>
-createLinkGraphFromELFObject_riscv(MemoryBufferRef ObjectBuffer);
+createLinkGraphFromELFObject_riscv(MemoryBufferRef ObjectBuffer,
+                                   std::shared_ptr<orc::SymbolStringPool> SSP);
 
 /// jit-link the given object buffer, which must be a ELF riscv object file.
 void link_ELF_riscv(std::unique_ptr<LinkGraph> G,

llvm/include/llvm/ExecutionEngine/JITLink/ELF_x86_64.h

@@ -24,7 +24,8 @@ namespace jitlink {
 /// its contents. The caller is responsible for ensuring that the object buffer
 /// outlives the graph.
 Expected<std::unique_ptr<LinkGraph>>
-createLinkGraphFromELFObject_x86_64(MemoryBufferRef ObjectBuffer);
+createLinkGraphFromELFObject_x86_64(MemoryBufferRef ObjectBuffer,
+                                    std::shared_ptr<orc::SymbolStringPool> SSP);
 
 /// jit-link the given object buffer, which must be a ELF x86-64 object file.
 void link_ELF_x86_64(std::unique_ptr<LinkGraph> G,