[lld-link] Replace LazyObjFile with lazy ObjFile/BitcodeFile

[MaskRay]

Summary

Similar to ELF 3a5fb57393c3bc77be9e7afc2ec9d4ec3c9bbf70.

• previously when a LazyObjFile was extracted, a new ObjFile/BitcodeFile was created; now the file is reused, just with lazy cleared
• avoid the confusing transfer of symbols from LazyObjFile to the new file
• simpler code, smaller executable (5200+ bytes smaller on x86-64)
• make eager parsing feasible (for parallel section/symbol table initialization)

[aganea]

lld/COFF/Driver.cpp

211

Add , bool lazy) to constructor and then this code should read: ctx.symtab.addFile(make<BitcodeFile>(ctx, mbref, "", 0, lazy));.
Same below for ObjFile.

lld/COFF/InputFiles.cpp

138

Same comment as BitcodeFile, move code into ObjFile::parse()

1081

Can we move this to BitcodeFile::parse() and do something like:

void BitcodeFile::parse() {
  if (lazy) {
    std::unique_ptr<lto::InputFile> obj = CHECK(lto::InputFile::create(mb), this);
    for (const lto::InputFile::Symbol &sym : obj->symbols())
      if (!sym.isUndefined())
        ctx.symtab.addLazyObject(this, sym.getName());
    return;
  }
  ...

lld/COFF/InputFiles.h

98

Can you somehow move this below after fileKind to prevent excessive padding? Otherwise the bool will take 64-bits :-)
See my other comments, this doesn't need to be public.

368

Add , bool lazy) to constructor.

lld/COFF/SymbolTable.cpp

40

I wonder if it's not better to leave types to do their own testing of lazy internally, in Type::parse(), and act accordingly (instead of doing it outside like here).
Same thing applies to pushing to the instances, shouldn't parse() do it internally? (but that could be done later)
We should be ending with just file->parse(); here.

585

You shouldn't be needing this test & assignment here (or it could be an assert?).

[MaskRay]

lld/COFF/SymbolTable.cpp

40

The reason parse calling parseLazy may feel better is because the function is in SymbolTable (while it can be placed into InputFile, too), so lazy has to be public to be accessed here.

If the function is in InputFile, this function dispatching to parse and parseLazy will be superior in my view as we can avoid the slightly unnecessary abstraction that parse returns parseLazy...

---

If PE/COFF needs the lazyBitcodeFiles as ELF does (Bazel distributed ThinLTO requirement), handling lazy in this function may be better, too.

[MaskRay]

Add lazy parameter to InputFile/ObjFile/BitcodeFile

[MaskRay]

address some comments

lld/COFF/InputFiles.cpp

138

Sam reply - the function will likely become more complex. A separate function will justify in the future.

[aganea]

LGTM, a few minor comments:

lld/COFF/SymbolTable.cpp

40

The reason parse calling parseLazy may feel better is because the function is in SymbolTable (while it can be placed into InputFile, too), so lazy has to be public to be accessed here.

If the function is in InputFile, this function dispatching to parse and parseLazy will be superior in my view as we can avoid the slightly unnecessary abstraction that parse returns parseLazy...

Fair enough, let's keep them apart.

If PE/COFF needs the lazyBitcodeFiles as ELF does (Bazel distributed ThinLTO requirement),

Do you need that (ie.thinLTOCreateEmptyIndexFiles()) because you have to emit exactly the same number of index files as there are input objects? If LLD exits with no error, emits less index files than inputs, could the build system assume that only those files are to be distributed?

585–588

Is it more appropriate to move this assert at the top?

587

You can just write addFile(f);

[MaskRay]

address comments

[MaskRay]

lld/COFF/SymbolTable.cpp

40

lld/ELF needs thinLTOCreateEmptyIndexFiles because Bazel's distributed ThinLTO requires it (--thinlto-index-only). Bazel needs it because it's the build system that requires all specified input and output and the output needs to be unconditional.

OK, it seems that lld/COFF uses a different strategy (D64461) which avoids lazyBitcodeFiles...

[rnk]

Looks good to me

lld/COFF/SymbolTable.cpp

42

If parse is virtual, should parseLazy be virtual as well? Doing O(objects) virtual calls shouldn't matter for performance, it's the O(symbols) actions that matter.

[MaskRay]

lld/COFF/SymbolTable.cpp

42

The virtual dispatch cost doesn't matter.

If parseLazy is made virtual, virtual void InputFile::parseLazy() = 0; will be needed as many file kinds don't have reasonable parseLazy semantics. That may not be clearer than open coding the two possible file kinds here.

lld/COFF/Driver.cpp

if (wholeArchive) {
 
       int memberIndex = 0;
       for (MemoryBufferRef m : getArchiveMembers(archive))
         addArchiveBuffer(m, "<whole-archive>", filename, memberIndex++);
       return;
     }
     ctx.symtab.addFile(make<ArchiveFile>(ctx, mbref));
     break;
-  case file_magic::bitcode:
+  case file_magic::bitcode: {
+    auto *file = make<BitcodeFile>(ctx, mbref, "", 0);
     if (lazy)
-      ctx.symtab.addFile(make<LazyObjFile>(ctx, mbref));
-    else
-      ctx.symtab.addFile(make<BitcodeFile>(ctx, mbref, "", 0));
+      file->lazy = true;
+    ctx.symtab.addFile(file);
     break;
+  }
   case file_magic::coff_object:
-  case file_magic::coff_import_library:
+  case file_magic::coff_import_library: {
+    auto *file = make<ObjFile>(ctx, mbref);
     if (lazy)
-      ctx.symtab.addFile(make<LazyObjFile>(ctx, mbref));
-    else
-      ctx.symtab.addFile(make<ObjFile>(ctx, mbref));
+      file->lazy = true;
+    ctx.symtab.addFile(file);
     break;
+  }
   case file_magic::pdb:
     ctx.symtab.addFile(make<PDBInputFile>(ctx, mbref));
     break;
   case file_magic::coff_cl_gl_object:
     error(filename + ": is not a native COFF file. Recompile without /GL");
     break;
   case file_magic::pecoff_executable:
     if (config->mingw) {

lld/COFF/InputFiles.h

public:
   // An archive file name if this file is created from an archive.
   StringRef parentName;
 
   // Returns .drectve section contents if exist.
   StringRef getDirectives() { return directives; }
 
   COFFLinkerContext &ctx;
 
+  // True if this is a lazy ObjFile or BitcodeFile.
+  bool lazy = false;
+
 protected:
   InputFile(COFFLinkerContext &c, Kind k, MemoryBufferRef m)
       : mb(m), ctx(c), fileKind(k) {}
 
   StringRef directives;
 
 private:
   const Kind fileKind;
public:
   // which ensures that we don't load the same member more than once.
   void addMember(const Archive::Symbol &sym);
 
 private:
   std::unique_ptr<Archive> file;
   llvm::DenseSet<uint64_t> seen;
 };
 
-// .obj or .o file between -start-lib and -end-lib.
-class LazyObjFile : public InputFile {
-public:
-  explicit LazyObjFile(COFFLinkerContext &ctx, MemoryBufferRef m)
-      : InputFile(ctx, LazyObjectKind, m) {}
-  static bool classof(const InputFile *f) {
-    return f->kind() == LazyObjectKind;
-  }
-  // Makes this object file part of the link.
-  void fetch();
-  // Adds the symbols in this file to the symbol table as LazyObject symbols.
-  void parse() override;
-
-private:
-  std::vector<Symbol *> symbols;
-};
-
 // .obj or .o file. This may be a member of an archive file.
 class ObjFile : public InputFile {
 public:
   explicit ObjFile(COFFLinkerContext &ctx, MemoryBufferRef m)
       : InputFile(ctx, ObjectKind, m) {}
-  explicit ObjFile(COFFLinkerContext &ctx, MemoryBufferRef m,
-                   std::vector<Symbol *> &&symbols)
-      : InputFile(ctx, ObjectKind, m), symbols(std::move(symbols)) {}
   static bool classof(const InputFile *f) { return f->kind() == ObjectKind; }
   void parse() override;
+  void parseLazy();
   MachineTypes getMachineType() override;
   ArrayRef<Chunk *> getChunks() { return chunks; }
   ArrayRef<SectionChunk *> getDebugChunks() { return debugChunks; }
   ArrayRef<SectionChunk *> getSXDataChunks() { return sxDataChunks; }
   ArrayRef<SectionChunk *> getGuardFidChunks() { return guardFidChunks; }
   ArrayRef<SectionChunk *> getGuardIATChunks() { return guardIATChunks; }
   ArrayRef<SectionChunk *> getGuardLJmpChunks() { return guardLJmpChunks; }
   ArrayRef<SectionChunk *> getGuardEHContChunks() { return guardEHContChunks; }
public:
   // to avoid creating unnecessary thunks.
   bool live = !config->doGC;
   bool thunkLive = !config->doGC;
 };
 
 // Used for LTO.
 class BitcodeFile : public InputFile {
 public:
-  BitcodeFile(COFFLinkerContext &ctx, MemoryBufferRef mb, StringRef archiveName,
-              uint64_t offsetInArchive);
-  explicit BitcodeFile(COFFLinkerContext &ctx, MemoryBufferRef m,
-                       StringRef archiveName, uint64_t offsetInArchive,
-                       std::vector<Symbol *> &&symbols);
+  explicit BitcodeFile(COFFLinkerContext &ctx, MemoryBufferRef mb,
+                       StringRef archiveName, uint64_t offsetInArchive);
   ~BitcodeFile();
   static bool classof(const InputFile *f) { return f->kind() == BitcodeKind; }
   ArrayRef<Symbol *> getSymbols() { return symbols; }
   MachineTypes getMachineType() override;
+  void parseLazy();
   std::unique_ptr<llvm::lto::InputFile> obj;
 
 private:
   void parse() override;
 
   std::vector<Symbol *> symbols;
 };
 

lld/COFF/InputFiles.cpp

for (const Archive::Child &c : file->children(err)) {
     v.push_back(mbref);
   }
   if (err)
     fatal(file->getFileName() +
           ": Archive::children failed: " + toString(std::move(err)));
   return v;
 }
 
-void LazyObjFile::fetch() {
-  if (mb.getBuffer().empty())
-    return;
-
-  InputFile *file;
-  if (isBitcode(mb))
-    file = make<BitcodeFile>(ctx, mb, "", 0, std::move(symbols));
-  else
-    file = make<ObjFile>(ctx, mb, std::move(symbols));
-  mb = {};
-  ctx.symtab.addFile(file);
-}
-
-void LazyObjFile::parse() {
-  if (isBitcode(this->mb)) {
-    // Bitcode file.
-    std::unique_ptr<lto::InputFile> obj =
-        CHECK(lto::InputFile::create(this->mb), this);
-    for (const lto::InputFile::Symbol &sym : obj->symbols()) {
-      if (!sym.isUndefined())
-        ctx.symtab.addLazyObject(this, sym.getName());
-    }
-    return;
-  }
-
+void ObjFile::parseLazy() {
   // Native object file.
   std::unique_ptr<Binary> coffObjPtr = CHECK(createBinary(mb), this);
   COFFObjectFile *coffObj = cast<COFFObjectFile>(coffObjPtr.get());
   uint32_t numSymbols = coffObj->getNumberOfSymbols();
   for (uint32_t i = 0; i < numSymbols; ++i) {
     COFFSymbolRef coffSym = check(coffObj->getSymbol(i));
     if (coffSym.isUndefined() || !coffSym.isExternal() ||
         coffSym.isWeakExternal())
void ImportFile::parse() {
   // DLL functions just like regular non-DLL functions.)
   if (hdr->getType() == llvm::COFF::IMPORT_CODE)
     thunkSym = ctx.symtab.addImportThunk(
         name, cast_or_null<DefinedImportData>(impSym), hdr->Machine);
 }
 
 BitcodeFile::BitcodeFile(COFFLinkerContext &ctx, MemoryBufferRef mb,
                          StringRef archiveName, uint64_t offsetInArchive)
-    : BitcodeFile(ctx, mb, archiveName, offsetInArchive, {}) {}
-
-BitcodeFile::BitcodeFile(COFFLinkerContext &ctx, MemoryBufferRef mb,
-                         StringRef archiveName, uint64_t offsetInArchive,
-                         std::vector<Symbol *> &&symbols)
-    : InputFile(ctx, BitcodeKind, mb), symbols(std::move(symbols)) {
+    : InputFile(ctx, BitcodeKind, mb) {
   std::string path = mb.getBufferIdentifier().str();
   if (config->thinLTOIndexOnly)
     path = replaceThinLTOSuffix(mb.getBufferIdentifier());
 
   // ThinLTO assumes that all MemoryBufferRefs given to it have a unique
   // name. If two archives define two members with the same name, this
   // causes a collision which result in only one of the objects being taken
   // into consideration at LTO time (which very likely causes undefined
for (const lto::InputFile::Symbol &objSym : obj->symbols()) {
     }
     symbols.push_back(sym);
     if (objSym.isUsed())
       config->gcroot.push_back(sym);
   }
   directives = obj->getCOFFLinkerOpts();
 }
 
+void BitcodeFile::parseLazy() {
+  std::unique_ptr<lto::InputFile> obj = CHECK(lto::InputFile::create(mb), this);
+  for (const lto::InputFile::Symbol &sym : obj->symbols())
+    if (!sym.isUndefined())
+      ctx.symtab.addLazyObject(this, sym.getName());
+}
+
 MachineTypes BitcodeFile::getMachineType() {
   switch (Triple(obj->getTargetTriple()).getArch()) {
   case Triple::x86_64:
     return AMD64;
   case Triple::x86:
     return I386;
   case Triple::arm:
     return ARMNT;

lld/COFF/SymbolTable.h

public:
   // Creates an Undefined symbol for a given name.
   Symbol *addUndefined(StringRef name);
 
   Symbol *addSynthetic(StringRef n, Chunk *c);
   Symbol *addAbsolute(StringRef n, uint64_t va);
 
   Symbol *addUndefined(StringRef name, InputFile *f, bool isWeakAlias);
   void addLazyArchive(ArchiveFile *f, const Archive::Symbol &sym);
-  void addLazyObject(LazyObjFile *f, StringRef n);
+  void addLazyObject(InputFile *f, StringRef n);
   void addLazyDLLSymbol(DLLFile *f, DLLFile::Symbol *sym, StringRef n);
   Symbol *addAbsolute(StringRef n, COFFSymbolRef s);
   Symbol *addRegular(InputFile *f, StringRef n,
                      const llvm::object::coff_symbol_generic *s = nullptr,
                      SectionChunk *c = nullptr, uint32_t sectionOffset = 0);
   std::pair<DefinedRegular *, bool>
   addComdat(InputFile *f, StringRef n,
             const llvm::object::coff_symbol_generic *s = nullptr);

lld/COFF/SymbolTable.cpp

 StringRef ltrim1(StringRef s, const char *chars) {
   if (!s.empty() && strchr(chars, s[0]))
     return s.substr(1);
   return s;
 }
 
 void SymbolTable::addFile(InputFile *file) {
   log("Reading " + toString(file));
+  if (file->lazy) {
+    if (auto *f = dyn_cast<BitcodeFile>(file))
+      f->parseLazy();
+    else
+      cast<ObjFile>(file)->parseLazy();
+  } else {
     file->parse();
+    if (auto *f = dyn_cast<ObjFile>(file)) {
+      ctx.objFileInstances.push_back(f);
+    } else if (auto *f = dyn_cast<BitcodeFile>(file)) {
+      ctx.bitcodeFileInstances.push_back(f);
+    } else if (auto *f = dyn_cast<ImportFile>(file)) {
+      ctx.importFileInstances.push_back(f);
+    }
+  }
 
   MachineTypes mt = file->getMachineType();
   if (config->machine == IMAGE_FILE_MACHINE_UNKNOWN) {
     config->machine = mt;
   } else if (mt != IMAGE_FILE_MACHINE_UNKNOWN && config->machine != mt) {
     error(toString(file) + ": machine type " + machineToStr(mt) +
           " conflicts with " + machineToStr(config->machine));
     return;
   }
 
-  if (auto *f = dyn_cast<ObjFile>(file)) {
-    ctx.objFileInstances.push_back(f);
-  } else if (auto *f = dyn_cast<BitcodeFile>(file)) {
-    ctx.bitcodeFileInstances.push_back(f);
-  } else if (auto *f = dyn_cast<ImportFile>(file)) {
-    ctx.importFileInstances.push_back(f);
-  }
-
   driver->parseDirectives(file);
 }
 
 static void errorOrWarn(const Twine &s) {
   if (config->forceUnresolved)
     warn(s);
   else
     error(s);
 }
 
 // Causes the file associated with a lazy symbol to be linked in.
 static void forceLazy(Symbol *s) {
   s->pendingArchiveLoad = true;
   switch (s->kind()) {
   case Symbol::Kind::LazyArchiveKind: {
     auto *l = cast<LazyArchive>(s);
     l->file->addMember(l->sym);
     break;
   }
-  case Symbol::Kind::LazyObjectKind:
-    cast<LazyObject>(s)->file->fetch();
+  case Symbol::Kind::LazyObjectKind: {
+    InputFile *file = cast<LazyObject>(s)->file;
+    file->ctx.symtab.addFile(file);
     break;
+  }
   case Symbol::Kind::LazyDLLSymbolKind: {
     auto *l = cast<LazyDLLSymbol>(s);
     l->file->makeImport(l->sym);
     break;
   }
   default:
     llvm_unreachable(
         "symbol passed to forceLazy is not a LazyArchive or LazyObject");
void SymbolTable::addLazyArchive(ArchiveFile *f, const Archive::Symbol &sym) {
   }
   auto *u = dyn_cast<Undefined>(s);
   if (!u || u->weakAlias || s->pendingArchiveLoad)
     return;
   s->pendingArchiveLoad = true;
   f->addMember(sym);
 }
 
-void SymbolTable::addLazyObject(LazyObjFile *f, StringRef n) {
+void SymbolTable::addLazyObject(InputFile *f, StringRef n) {
   Symbol *s;
   bool wasInserted;
   std::tie(s, wasInserted) = insert(n, f);
   if (wasInserted) {
     replaceSymbol<LazyObject>(s, f, n);
     return;
   }
   auto *u = dyn_cast<Undefined>(s);
   if (!u || u->weakAlias || s->pendingArchiveLoad)
     return;
   s->pendingArchiveLoad = true;
-  f->fetch();
+  if (!f->lazy)
+    return;
+  f->lazy = false;
+  ctx.symtab.addFile(f);
 }
 
 void SymbolTable::addLazyDLLSymbol(DLLFile *f, DLLFile::Symbol *sym,
                                    StringRef n) {
   Symbol *s;
   bool wasInserted;
   std::tie(s, wasInserted) = insert(n);
   if (wasInserted) {

lld/COFF/Symbols.h

public:
   MemoryBufferRef getMemberBuffer();
 
   ArchiveFile *file;
   const Archive::Symbol sym;
 };
 
 class LazyObject : public Symbol {
 public:
-  LazyObject(LazyObjFile *f, StringRef n)
-      : Symbol(LazyObjectKind, n), file(f) {}
+  LazyObject(InputFile *f, StringRef n) : Symbol(LazyObjectKind, n), file(f) {}
   static bool classof(const Symbol *s) { return s->kind() == LazyObjectKind; }
-  LazyObjFile *file;
+  InputFile *file;
 };
 
 // MinGW only.
 class LazyDLLSymbol : public Symbol {
 public:
   LazyDLLSymbol(DLLFile *f, DLLFile::Symbol *s, StringRef n)
       : Symbol(LazyDLLSymbolKind, n), file(f), sym(s) {}
   static bool classof(const Symbol *s) {