Revert "[NFCI][LTO][lld] Optimize away symbol copies within LTO global resolution in ELF" (#107788)

Reverts #106193 while investigating bot failures
https://lab.llvm.org/buildbot/#/builders/169/builds/2989/steps/9/logs/stdio

Author: mingmingl-llvm

lld/ELF/InputFiles.cpp

@@ -1804,10 +1804,6 @@ void BitcodeFile::parseLazy() {
       auto *sym = symtab.insert(unique_saver().save(irSym.getName()));
       sym->resolve(LazySymbol{*this});
       symbols[i] = sym;
-    } else {
-      // Keep copies of per-module undefined symbols for LTO::GlobalResolutions
-      // usage.
-      unique_saver().save(irSym.getName());
     }
 }
 

lld/ELF/LTO.cpp

@@ -135,7 +135,6 @@ static lto::Config createConfig() {
       config->ltoValidateAllVtablesHaveTypeInfos;
   c.AllVtablesHaveTypeInfos = ctx.ltoAllVtablesHaveTypeInfos;
   c.AlwaysEmitRegularLTOObj = !config->ltoObjPath.empty();
-  c.KeepSymbolNameCopies = false;
 
   for (const llvm::StringRef &name : config->thinLTOModulesToCompile)
     c.ThinLTOModulesToCompile.emplace_back(name);

llvm/include/llvm/LTO/Config.h

@@ -88,11 +88,6 @@ struct Config {
   /// want to know a priori all possible output files.
   bool AlwaysEmitRegularLTOObj = false;
 
-  /// If true, the LTO instance creates copies of the symbol names for LTO::run.
-  /// The lld linker uses string saver to keep symbol names alive and doesn't
-  /// need to create copies, so it can set this field to false.
-  bool KeepSymbolNameCopies = true;
-
   /// Allows non-imported definitions to get the potentially more constraining
   /// visibility from the prevailing definition. FromPrevailing is the default
   /// because it works for many binary formats. ELF can use the more optimized

llvm/include/llvm/LTO/LTO.h

@@ -15,9 +15,6 @@
 #ifndef LLVM_LTO_LTO_H
 #define LLVM_LTO_LTO_H
 
-#include <memory>
-
-#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Bitcode/BitcodeReader.h"
@@ -26,7 +23,6 @@
 #include "llvm/Object/IRSymtab.h"
 #include "llvm/Support/Caching.h"
 #include "llvm/Support/Error.h"
-#include "llvm/Support/StringSaver.h"
 #include "llvm/Support/thread.h"
 #include "llvm/Transforms/IPO/FunctionAttrs.h"
 #include "llvm/Transforms/IPO/FunctionImport.h"
@@ -407,19 +403,10 @@ class LTO {
     };
   };
 
-  // GlobalResolutionSymbolSaver allocator.
-  std::unique_ptr<llvm::BumpPtrAllocator> Alloc;
-
-  // Symbol saver for global resolution map.
-  std::unique_ptr<llvm::StringSaver> GlobalResolutionSymbolSaver;
-
   // Global mapping from mangled symbol names to resolutions.
-  // Make this an unique_ptr to guard against accessing after it has been reset
+  // Make this an optional to guard against accessing after it has been reset
   // (to reduce memory after we're done with it).
-  std::unique_ptr<llvm::DenseMap<StringRef, GlobalResolution>>
-      GlobalResolutions;
-
-  void releaseGlobalResolutionsMemory();
+  std::optional<StringMap<GlobalResolution>> GlobalResolutions;
 
   void addModuleToGlobalRes(ArrayRef<InputFile::Symbol> Syms,
                             ArrayRef<SymbolResolution> Res, unsigned Partition,

llvm/lib/LTO/LTO.cpp

@@ -77,10 +77,6 @@ cl::opt<bool> EnableLTOInternalization(
     "enable-lto-internalization", cl::init(true), cl::Hidden,
     cl::desc("Enable global value internalization in LTO"));
 
-static cl::opt<bool>
-    LTOKeepSymbolCopies("lto-keep-symbol-copies", cl::init(false), cl::Hidden,
-                        cl::desc("Keep copies of symbols in LTO indexing"));
-
 /// Indicate we are linking with an allocator that supports hot/cold operator
 /// new interfaces.
 extern cl::opt<bool> SupportsHotColdNew;
@@ -591,14 +587,8 @@ LTO::LTO(Config Conf, ThinBackend Backend,
     : Conf(std::move(Conf)),
       RegularLTO(ParallelCodeGenParallelismLevel, this->Conf),
       ThinLTO(std::move(Backend)),
-      GlobalResolutions(
-          std::make_unique<DenseMap<StringRef, GlobalResolution>>()),
-      LTOMode(LTOMode) {
-  if (Conf.KeepSymbolNameCopies || LTOKeepSymbolCopies) {
-    Alloc = std::make_unique<BumpPtrAllocator>();
-    GlobalResolutionSymbolSaver = std::make_unique<llvm::StringSaver>(*Alloc);
-  }
-}
+      GlobalResolutions(std::make_optional<StringMap<GlobalResolution>>()),
+      LTOMode(LTOMode) {}
 
 // Requires a destructor for MapVector<BitcodeModule>.
 LTO::~LTO() = default;
@@ -616,12 +606,7 @@ void LTO::addModuleToGlobalRes(ArrayRef<InputFile::Symbol> Syms,
     assert(ResI != ResE);
     SymbolResolution Res = *ResI++;
 
-    StringRef SymbolName = Sym.getName();
-    // Keep copies of symbols if the client of LTO says so.
-    if (GlobalResolutionSymbolSaver && !GlobalResolutions->contains(SymbolName))
-      SymbolName = GlobalResolutionSymbolSaver->save(SymbolName);
-
-    auto &GlobalRes = (*GlobalResolutions)[SymbolName];
+    auto &GlobalRes = (*GlobalResolutions)[Sym.getName()];
     GlobalRes.UnnamedAddr &= Sym.isUnnamedAddr();
     if (Res.Prevailing) {
       assert(!GlobalRes.Prevailing &&
@@ -675,14 +660,6 @@ void LTO::addModuleToGlobalRes(ArrayRef<InputFile::Symbol> Syms,
   }
 }
 
-void LTO::releaseGlobalResolutionsMemory() {
-  // Release GlobalResolutions dense-map itself.
-  GlobalResolutions.reset();
-  // Release the string saver memory.
-  GlobalResolutionSymbolSaver.reset();
-  Alloc.reset();
-}
-
 static void writeToResolutionFile(raw_ostream &OS, InputFile *Input,
                                   ArrayRef<SymbolResolution> Res) {
   StringRef Path = Input->getName();
@@ -1794,7 +1771,7 @@ Error LTO::runThinLTO(AddStreamFn AddStream, FileCache Cache,
   // are no further accesses. We specifically want to do this before computing
   // cross module importing, which adds to peak memory via the computed import
   // and export lists.
-  releaseGlobalResolutionsMemory();
+  GlobalResolutions.reset();
 
   if (Conf.OptLevel > 0)
     ComputeCrossModuleImport(ThinLTO.CombinedIndex, ModuleToDefinedGVSummaries,