[llvm] 28e2a89 - Revert "[ORC][JITLink] Add jitlink::Scope::SideEffectsOnly, use it in ORC Platforms."

[Lang Hames via llvm-commits]

Author: Lang Hames
Date: 2024-12-03T16:56:23+11:00
New Revision: 28e2a8912102ac442166a593f2efb15c5bd30346

URL: https://github.com/llvm/llvm-project/commit/28e2a8912102ac442166a593f2efb15c5bd30346
DIFF: https://github.com/llvm/llvm-project/commit/28e2a8912102ac442166a593f2efb15c5bd30346.diff

LOG: Revert "[ORC][JITLink] Add jitlink::Scope::SideEffectsOnly, use it in ORC Platforms."

This reverts commit aba6bb0820b247d4caf4b5e00810909214a58053 while I investigate bot
failures (e.g. https://lab.llvm.org/buildbot/#/builders/143/builds/3848)

Added: 
    

Modified: 
    llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
    llvm/lib/ExecutionEngine/Orc/COFFPlatform.cpp
    llvm/lib/ExecutionEngine/Orc/ELFNixPlatform.cpp
    llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
    llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
index 2831ebb3be798f..9844214c537a06 100644
--- a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
+++ b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
@@ -393,13 +393,10 @@ const char *getLinkageName(Linkage L);
 /// Defines the scope in which this symbol should be visible:
 ///   Default -- Visible in the public interface of the linkage unit.
 ///   Hidden -- Visible within the linkage unit, but not exported from it.
-///   SideEffectsOnly -- Like hidden, but symbol can only be looked up once
-///                      to trigger materialization of the containing graph.
 ///   Local -- Visible only within the LinkGraph.
 enum class Scope : uint8_t {
   Default,
   Hidden,
-  SideEffectsOnly,
   Local
 };
 

diff  --git a/llvm/lib/ExecutionEngine/Orc/COFFPlatform.cpp b/llvm/lib/ExecutionEngine/Orc/COFFPlatform.cpp
index 9d655c2e1e8be4..007e18e3073997 100644
--- a/llvm/lib/ExecutionEngine/Orc/COFFPlatform.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/COFFPlatform.cpp
@@ -76,7 +76,7 @@ class COFFHeaderMaterializationUnit : public MaterializationUnit {
     // Init symbol is __ImageBase symbol.
     auto &ImageBaseSymbol = G->addDefinedSymbol(
         HeaderBlock, 0, *R->getInitializerSymbol(), HeaderBlock.getSize(),
-        jitlink::Linkage::Strong, jitlink::Scope::SideEffectsOnly, false, true);
+        jitlink::Linkage::Strong, jitlink::Scope::Default, false, true);
 
     addImageBaseRelocationEdge(HeaderBlock, ImageBaseSymbol);
 

diff  --git a/llvm/lib/ExecutionEngine/Orc/ELFNixPlatform.cpp b/llvm/lib/ExecutionEngine/Orc/ELFNixPlatform.cpp
index 44d282fc6ff677..431c64996b2c56 100644
--- a/llvm/lib/ExecutionEngine/Orc/ELFNixPlatform.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/ELFNixPlatform.cpp
@@ -197,7 +197,7 @@ class DSOHandleMaterializationUnit : public MaterializationUnit {
         8, 0);
     auto &DSOHandleSymbol = G->addDefinedSymbol(
         DSOHandleBlock, 0, *R->getInitializerSymbol(), DSOHandleBlock.getSize(),
-        jitlink::Linkage::Strong, jitlink::Scope::SideEffectsOnly, false, true);
+        jitlink::Linkage::Strong, jitlink::Scope::Default, false, true);
     DSOHandleBlock.addEdge(EdgeKind, 0, DSOHandleSymbol, 0);
 
     ENP.getObjectLinkingLayer().emit(std::move(R), std::move(G));

diff  --git a/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp b/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
index 1b18a4d0596c18..3e02beb0baa868 100644
--- a/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
@@ -1001,9 +1001,9 @@ Error MachOPlatform::MachOPlatformPlugin::preserveImportantSections(
       // to the first block.
       if (!InitSym) {
         auto &B = **InitSection->blocks().begin();
-        InitSym = &G.addDefinedSymbol(
-            B, 0, *InitSymName, B.getSize(), jitlink::Linkage::Strong,
-            jitlink::Scope::SideEffectsOnly, false, true);
+        InitSym = &G.addDefinedSymbol(B, 0, *InitSymName, B.getSize(),
+                                      jitlink::Linkage::Strong,
+                                      jitlink::Scope::Default, false, true);
       }
 
       // Add keep-alive edges to anonymous symbols in all other init blocks.

diff  --git a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
index c5342c4f4deb34..c1c55408c78584 100644
--- a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
@@ -65,8 +65,6 @@ JITSymbolFlags getJITSymbolFlagsForSymbol(Symbol &Sym) {
 
   if (Sym.getScope() == Scope::Default)
     Flags |= JITSymbolFlags::Exported;
-  else if (Sym.getScope() == Scope::SideEffectsOnly)
-    Flags |= JITSymbolFlags::MaterializationSideEffectsOnly;
 
   if (Sym.isCallable())
     Flags |= JITSymbolFlags::Callable;
@@ -238,7 +236,7 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
 
     SymbolMap InternedResult;
     for (auto *Sym : G.defined_symbols())
-      if (Sym->getScope() < Scope::SideEffectsOnly) {
+      if (Sym->getScope() != Scope::Local) {
         auto InternedName = ES.intern(Sym->getName());
         auto Ptr = getJITSymbolPtrForSymbol(*Sym, G.getTargetTriple());
         auto Flags = getJITSymbolFlagsForSymbol(*Sym);
@@ -251,7 +249,7 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
       }
 
     for (auto *Sym : G.absolute_symbols())
-      if (Sym->getScope() < Scope::SideEffectsOnly) {
+      if (Sym->getScope() != Scope::Local) {
         auto InternedName = ES.intern(Sym->getName());
         auto Ptr = getJITSymbolPtrForSymbol(*Sym, G.getTargetTriple());
         auto Flags = getJITSymbolFlagsForSymbol(*Sym);
@@ -283,9 +281,11 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
         // If this is a materialization-side-effects only symbol then bump
         // the counter and remove in from the result, otherwise make sure that
         // it's defined.
-        if (Flags.hasMaterializationSideEffectsOnly())
+        if (Flags.hasMaterializationSideEffectsOnly()) {
           ++NumMaterializationSideEffectsOnlySymbols;
-        else if (I == InternedResult.end())
+          InternedResult.erase(Sym);
+          continue;
+        } else if (I == InternedResult.end())
           MissingSymbols.push_back(Sym);
         else if (Layer.OverrideObjectFlags)
           I->second.setFlags(Flags);