[lld] r319090 - COFF: Do not create SectionChunks for discarded comdat sections.

Peter Collingbourne via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 27 12:42:34 PST 2017


Author: pcc
Date: Mon Nov 27 12:42:34 2017
New Revision: 319090

URL: http://llvm.org/viewvc/llvm-project?rev=319090&view=rev
Log:
COFF: Do not create SectionChunks for discarded comdat sections.

With this change, instead of creating a SectionChunk for each section
in the object file, we only create them when we encounter a prevailing
comdat section.

Also change how symbol resolution occurs between comdat symbols. Now
only the comdat leader participates in comdat resolution, and not any
other external associated symbols. This is more in line with how COFF
semantics are defined, and should allow for a more straightforward
implementation of non-ANY comdat types.

On my machine, this change reduces our runtime linking a release
build of chrome_child.dll with /nopdb from 5.65s to 4.54s (median of
50 runs).

Differential Revision: https://reviews.llvm.org/D40238

Modified:
    lld/trunk/COFF/Chunks.cpp
    lld/trunk/COFF/Chunks.h
    lld/trunk/COFF/InputFiles.cpp
    lld/trunk/COFF/InputFiles.h
    lld/trunk/COFF/MarkLive.cpp
    lld/trunk/COFF/SymbolTable.cpp
    lld/trunk/COFF/SymbolTable.h
    lld/trunk/COFF/Symbols.h
    lld/trunk/test/COFF/reloc-discarded.s

Modified: lld/trunk/COFF/Chunks.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Chunks.cpp?rev=319090&r1=319089&r2=319090&view=diff
==============================================================================
--- lld/trunk/COFF/Chunks.cpp (original)
+++ lld/trunk/COFF/Chunks.cpp Mon Nov 27 12:42:34 2017
@@ -38,9 +38,6 @@ SectionChunk::SectionChunk(ObjFile *F, c
 
   Alignment = Header->getAlignment();
 
-  // Chunks may be discarded during comdat merging.
-  Discarded = false;
-
   // If linker GC is disabled, every chunk starts out alive.  If linker GC is
   // enabled, treat non-comdat sections as roots. Generally optimized object
   // files will be built with -ffunction-sections or /Gy, so most things worth
@@ -362,12 +359,8 @@ bool SectionChunk::isCOMDAT() const {
 void SectionChunk::printDiscardedMessage() const {
   // Removed by dead-stripping. If it's removed by ICF, ICF already
   // printed out the name, so don't repeat that here.
-  if (Sym && this == Repl) {
-    if (Discarded)
-      message("Discarded comdat symbol " + Sym->getName());
-    else if (!Live)
-      message("Discarded " + Sym->getName());
-  }
+  if (Sym && this == Repl)
+    message("Discarded " + Sym->getName());
 }
 
 StringRef SectionChunk::getDebugName() {

Modified: lld/trunk/COFF/Chunks.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Chunks.h?rev=319090&r1=319089&r2=319090&view=diff
==============================================================================
--- lld/trunk/COFF/Chunks.h (original)
+++ lld/trunk/COFF/Chunks.h Mon Nov 27 12:42:34 2017
@@ -159,10 +159,9 @@ public:
   void addAssociative(SectionChunk *Child);
 
   StringRef getDebugName() override;
-  void setSymbol(DefinedRegular *S) { if (!Sym) Sym = S; }
 
-  // Returns true if the chunk was not dropped by GC or COMDAT deduplication.
-  bool isLive() { return Live && !Discarded; }
+  // Returns true if the chunk was not dropped by GC.
+  bool isLive() { return Live; }
 
   // Used by the garbage collector.
   void markLive() {
@@ -171,13 +170,6 @@ public:
     Live = true;
   }
 
-  // Returns true if this chunk was dropped by COMDAT deduplication.
-  bool isDiscarded() const { return Discarded; }
-
-  // Used by the SymbolTable when discarding unused comdat sections. This is
-  // redundant when GC is enabled, as all comdat sections will start out dead.
-  void markDiscarded() { Discarded = true; }
-
   // True if this is a codeview debug info chunk. These will not be laid out in
   // the image. Instead they will end up in the PDB, if one is requested.
   bool isCodeView() const {
@@ -213,24 +205,21 @@ public:
   // The file that this chunk was created from.
   ObjFile *File;
 
+  // The COMDAT leader symbol if this is a COMDAT chunk.
+  DefinedRegular *Sym = nullptr;
+
 private:
   StringRef SectionName;
   std::vector<SectionChunk *> AssocChildren;
   llvm::iterator_range<const coff_relocation *> Relocs;
   size_t NumRelocs;
 
-  // True if this chunk was discarded because it was a duplicate comdat section.
-  bool Discarded;
-
   // Used by the garbage collector.
   bool Live;
 
   // Used for ICF (Identical COMDAT Folding)
   void replace(SectionChunk *Other);
   uint32_t Class[2] = {0, 0};
-
-  // Sym points to a section symbol if this is a COMDAT chunk.
-  DefinedRegular *Sym = nullptr;
 };
 
 // A chunk for common symbols. Common chunks don't have actual data.

Modified: lld/trunk/COFF/InputFiles.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/InputFiles.cpp?rev=319090&r1=319089&r2=319090&view=diff
==============================================================================
--- lld/trunk/COFF/InputFiles.cpp (original)
+++ lld/trunk/COFF/InputFiles.cpp Mon Nov 27 12:42:34 2017
@@ -119,86 +119,158 @@ void ObjFile::parse() {
   initializeSEH();
 }
 
+// We set SectionChunk pointers in the SparseChunks vector to this value
+// temporarily to mark comdat sections as having an unknown resolution. As we
+// walk the object file's symbol table, once we visit either a leader symbol or
+// an associative section definition together with the parent comdat's leader,
+// we set the pointer to either nullptr (to mark the section as discarded) or a
+// valid SectionChunk for that section.
+static SectionChunk *const PendingComdat = reinterpret_cast<SectionChunk *>(1);
+
 void ObjFile::initializeChunks() {
   uint32_t NumSections = COFFObj->getNumberOfSections();
   Chunks.reserve(NumSections);
   SparseChunks.resize(NumSections + 1);
   for (uint32_t I = 1; I < NumSections + 1; ++I) {
     const coff_section *Sec;
-    StringRef Name;
     if (auto EC = COFFObj->getSection(I, Sec))
       fatal("getSection failed: #" + Twine(I) + ": " + EC.message());
-    if (auto EC = COFFObj->getSectionName(Sec, Name))
-      fatal("getSectionName failed: #" + Twine(I) + ": " + EC.message());
-    if (Name == ".sxdata") {
-      SXData = Sec;
-      continue;
-    }
-    if (Name == ".drectve") {
-      ArrayRef<uint8_t> Data;
-      COFFObj->getSectionContents(Sec, Data);
-      Directives = std::string((const char *)Data.data(), Data.size());
-      continue;
-    }
 
-    // Object files may have DWARF debug info or MS CodeView debug info
-    // (or both).
-    //
-    // DWARF sections don't need any special handling from the perspective
-    // of the linker; they are just a data section containing relocations.
-    // We can just link them to complete debug info.
-    //
-    // CodeView needs a linker support. We need to interpret and debug
-    // info, and then write it to a separate .pdb file.
-
-    // Ignore debug info unless /debug is given.
-    if (!Config->Debug && Name.startswith(".debug"))
-      continue;
-
-    if (Sec->Characteristics & llvm::COFF::IMAGE_SCN_LNK_REMOVE)
-      continue;
-    auto *C = make<SectionChunk>(this, Sec);
-
-    // CodeView sections are stored to a different vector because they are not
-    // linked in the regular manner.
-    if (C->isCodeView())
-      DebugChunks.push_back(C);
+    if (Sec->Characteristics & IMAGE_SCN_LNK_COMDAT)
+      SparseChunks[I] = PendingComdat;
     else
-      Chunks.push_back(C);
+      SparseChunks[I] = readSection(I, nullptr);
+  }
+}
+
+SectionChunk *ObjFile::readSection(uint32_t SectionNumber,
+                                   const coff_aux_section_definition *Def) {
+  const coff_section *Sec;
+  StringRef Name;
+  if (auto EC = COFFObj->getSection(SectionNumber, Sec))
+    fatal("getSection failed: #" + Twine(SectionNumber) + ": " + EC.message());
+  if (auto EC = COFFObj->getSectionName(Sec, Name))
+    fatal("getSectionName failed: #" + Twine(SectionNumber) + ": " +
+          EC.message());
+  if (Name == ".sxdata") {
+    SXData = Sec;
+    return nullptr;
+  }
+  if (Name == ".drectve") {
+    ArrayRef<uint8_t> Data;
+    COFFObj->getSectionContents(Sec, Data);
+    Directives = std::string((const char *)Data.data(), Data.size());
+    return nullptr;
+  }
 
-    SparseChunks[I] = C;
+  // Object files may have DWARF debug info or MS CodeView debug info
+  // (or both).
+  //
+  // DWARF sections don't need any special handling from the perspective
+  // of the linker; they are just a data section containing relocations.
+  // We can just link them to complete debug info.
+  //
+  // CodeView needs a linker support. We need to interpret and debug
+  // info, and then write it to a separate .pdb file.
+
+  // Ignore debug info unless /debug is given.
+  if (!Config->Debug && Name.startswith(".debug"))
+    return nullptr;
+
+  if (Sec->Characteristics & llvm::COFF::IMAGE_SCN_LNK_REMOVE)
+    return nullptr;
+  auto *C = make<SectionChunk>(this, Sec);
+  if (Def)
+    C->Checksum = Def->CheckSum;
+
+  // CodeView sections are stored to a different vector because they are not
+  // linked in the regular manner.
+  if (C->isCodeView())
+    DebugChunks.push_back(C);
+  else
+    Chunks.push_back(C);
+
+  return C;
+}
+
+void ObjFile::readAssociativeDefinition(
+    COFFSymbolRef Sym, const coff_aux_section_definition *Def) {
+  SectionChunk *Parent = SparseChunks[Def->getNumber(Sym.isBigObj())];
+
+  // If the parent is pending, it probably means that its section definition
+  // appears after us in the symbol table. Leave the associated section as
+  // pending; we will handle it during the second pass in initializeSymbols().
+  if (Parent == PendingComdat)
+    return;
+
+  // Check whether the parent is prevailing. If it is, so are we, and we read
+  // the section; otherwise mark it as discarded.
+  int32_t SectionNumber = Sym.getSectionNumber();
+  if (Parent) {
+    SparseChunks[SectionNumber] = readSection(SectionNumber, Def);
+    Parent->addAssociative(SparseChunks[SectionNumber]);
+  } else {
+    SparseChunks[SectionNumber] = nullptr;
   }
 }
 
+Symbol *ObjFile::createRegular(COFFSymbolRef Sym) {
+  SectionChunk *SC = SparseChunks[Sym.getSectionNumber()];
+  if (Sym.isExternal()) {
+    StringRef Name;
+    COFFObj->getSymbolName(Sym, Name);
+    if (SC)
+      return Symtab->addRegular(this, Name, Sym.getGeneric(), SC);
+    return Symtab->addUndefined(Name, this, false);
+  }
+  if (SC)
+    return make<DefinedRegular>(this, /*Name*/ "", false,
+                                /*IsExternal*/ false, Sym.getGeneric(), SC);
+  return nullptr;
+}
+
 void ObjFile::initializeSymbols() {
   uint32_t NumSymbols = COFFObj->getNumberOfSymbols();
   Symbols.resize(NumSymbols);
 
   SmallVector<std::pair<Symbol *, uint32_t>, 8> WeakAliases;
-  int32_t LastSectionNumber = 0;
+  std::vector<uint32_t> PendingIndexes;
+  PendingIndexes.reserve(NumSymbols);
+
+  std::vector<const coff_aux_section_definition *> ComdatDefs(
+      COFFObj->getNumberOfSections() + 1);
 
   for (uint32_t I = 0; I < NumSymbols; ++I) {
     COFFSymbolRef COFFSym = check(COFFObj->getSymbol(I));
-
-    const void *AuxP = nullptr;
-    if (COFFSym.getNumberOfAuxSymbols())
-      AuxP = check(COFFObj->getSymbol(I + 1)).getRawPtr();
-    bool IsFirst = (LastSectionNumber != COFFSym.getSectionNumber());
-
-    Symbol *Sym = nullptr;
     if (COFFSym.isUndefined()) {
-      Sym = createUndefined(COFFSym);
+      Symbols[I] = createUndefined(COFFSym);
     } else if (COFFSym.isWeakExternal()) {
-      Sym = createUndefined(COFFSym);
-      uint32_t TagIndex =
-          static_cast<const coff_aux_weak_external *>(AuxP)->TagIndex;
-      WeakAliases.emplace_back(Sym, TagIndex);
+      Symbols[I] = createUndefined(COFFSym);
+      uint32_t TagIndex = COFFSym.getAux<coff_aux_weak_external>()->TagIndex;
+      WeakAliases.emplace_back(Symbols[I], TagIndex);
+    } else if (Optional<Symbol *> OptSym = createDefined(COFFSym, ComdatDefs)) {
+      Symbols[I] = *OptSym;
     } else {
-      Sym = createDefined(COFFSym, AuxP, IsFirst);
+      // createDefined() returns None if a symbol belongs to a section that
+      // was pending at the point when the symbol was read. This can happen in
+      // two cases:
+      // 1) section definition symbol for a comdat leader;
+      // 2) symbol belongs to a comdat section associated with a section whose
+      //    section definition symbol appears later in the symbol table.
+      // In both of these cases, we can expect the section to be resolved by
+      // the time we finish visiting the remaining symbols in the symbol
+      // table. So we postpone the handling of this symbol until that time.
+      PendingIndexes.push_back(I);
     }
-    Symbols[I] = Sym;
     I += COFFSym.getNumberOfAuxSymbols();
-    LastSectionNumber = COFFSym.getSectionNumber();
+  }
+
+  for (uint32_t I : PendingIndexes) {
+    COFFSymbolRef Sym = check(COFFObj->getSymbol(I));
+    if (auto *Def = Sym.getSectionDefinition())
+      if (Def->Selection == IMAGE_COMDAT_SELECT_ASSOCIATIVE)
+        readAssociativeDefinition(Sym, Def);
+    Symbols[I] = createRegular(Sym);
   }
 
   for (auto &KV : WeakAliases) {
@@ -214,8 +286,9 @@ Symbol *ObjFile::createUndefined(COFFSym
   return Symtab->addUndefined(Name, this, Sym.isWeakExternal());
 }
 
-Symbol *ObjFile::createDefined(COFFSymbolRef Sym, const void *AuxP,
-                               bool IsFirst) {
+Optional<Symbol *> ObjFile::createDefined(
+    COFFSymbolRef Sym,
+    std::vector<const coff_aux_section_definition *> &ComdatDefs) {
   StringRef Name;
   if (Sym.isCommon()) {
     auto *C = make<CommonChunk>(Sym);
@@ -254,37 +327,46 @@ Symbol *ObjFile::createDefined(COFFSymbo
   if ((uint32_t)SectionNumber >= SparseChunks.size())
     fatal("broken object file: " + toString(this));
 
-  // Nothing else to do without a section chunk.
-  auto *SC = SparseChunks[SectionNumber];
-  if (!SC)
-    return nullptr;
-
-  // Handle section definitions
-  if (IsFirst && AuxP) {
-    auto *Aux = reinterpret_cast<const coff_aux_section_definition *>(AuxP);
-    if (Aux->Selection == IMAGE_COMDAT_SELECT_ASSOCIATIVE)
-      if (auto *ParentSC = SparseChunks[Aux->getNumber(Sym.isBigObj())]) {
-        ParentSC->addAssociative(SC);
-        // If we already discarded the parent, discard the child.
-        if (ParentSC->isDiscarded())
-          SC->markDiscarded();
-      }
-    SC->Checksum = Aux->CheckSum;
+  // Handle comdat leader symbols.
+  if (const coff_aux_section_definition *Def = ComdatDefs[SectionNumber]) {
+    ComdatDefs[SectionNumber] = nullptr;
+    Symbol *Leader;
+    bool Prevailing;
+    if (Sym.isExternal()) {
+      COFFObj->getSymbolName(Sym, Name);
+      std::tie(Leader, Prevailing) =
+          Symtab->addComdat(this, Name, Sym.getGeneric());
+    } else {
+      Leader = make<DefinedRegular>(this, /*Name*/ "", false,
+                                    /*IsExternal*/ false, Sym.getGeneric());
+      Prevailing = true;
+    }
+    if (Prevailing) {
+      SectionChunk *C = readSection(SectionNumber, Def);
+      SparseChunks[SectionNumber] = C;
+      C->Sym = cast<DefinedRegular>(Leader);
+      cast<DefinedRegular>(Leader)->Data = &C->Repl;
+    } else {
+      SparseChunks[SectionNumber] = nullptr;
+    }
+    return Leader;
   }
 
-  DefinedRegular *B;
-  if (Sym.isExternal()) {
-    COFFObj->getSymbolName(Sym, Name);
-    Symbol *S =
-        Symtab->addRegular(this, Name, SC->isCOMDAT(), Sym.getGeneric(), SC);
-    B = cast<DefinedRegular>(S);
-  } else
-    B = make<DefinedRegular>(this, /*Name*/ "", SC->isCOMDAT(),
-                             /*IsExternal*/ false, Sym.getGeneric(), SC);
-  if (SC->isCOMDAT() && Sym.getValue() == 0 && !AuxP)
-    SC->setSymbol(B);
+  // Read associative section definitions and prepare to handle the comdat
+  // leader symbol by setting the section's ComdatDefs pointer if we encounter a
+  // non-associative comdat.
+  if (SparseChunks[SectionNumber] == PendingComdat) {
+    if (auto *Def = Sym.getSectionDefinition()) {
+      if (Def->Selection == IMAGE_COMDAT_SELECT_ASSOCIATIVE)
+        readAssociativeDefinition(Sym, Def);
+      else
+        ComdatDefs[SectionNumber] = Def;
+    }
+  }
 
-  return B;
+  if (SparseChunks[SectionNumber] == PendingComdat)
+    return None;
+  return createRegular(Sym);
 }
 
 void ObjFile::initializeSEH() {
@@ -361,8 +443,12 @@ void ImportFile::parse() {
 void BitcodeFile::parse() {
   Obj = check(lto::InputFile::create(MemoryBufferRef(
       MB.getBuffer(), Saver.save(ParentName + MB.getBufferIdentifier()))));
+  std::vector<std::pair<Symbol *, bool>> Comdat(Obj->getComdatTable().size());
+  for (size_t I = 0; I != Obj->getComdatTable().size(); ++I)
+    Comdat[I] = Symtab->addComdat(this, Saver.save(Obj->getComdatTable()[I]));
   for (const lto::InputFile::Symbol &ObjSym : Obj->symbols()) {
     StringRef SymName = Saver.save(ObjSym.getName());
+    int ComdatIndex = ObjSym.getComdatIndex();
     Symbol *Sym;
     if (ObjSym.isUndefined()) {
       Sym = Symtab->addUndefined(SymName, this, false);
@@ -374,9 +460,15 @@ void BitcodeFile::parse() {
       std::string Fallback = ObjSym.getCOFFWeakExternalFallback();
       Symbol *Alias = Symtab->addUndefined(Saver.save(Fallback));
       checkAndSetWeakAlias(Symtab, this, Sym, Alias);
+    } else if (ComdatIndex != -1) {
+      if (SymName == Obj->getComdatTable()[ComdatIndex])
+        Sym = Comdat[ComdatIndex].first;
+      else if (Comdat[ComdatIndex].second)
+        Sym = Symtab->addRegular(this, SymName);
+      else
+        Sym = Symtab->addUndefined(SymName, this, false);
     } else {
-      bool IsCOMDAT = ObjSym.getComdatIndex() != -1;
-      Sym = Symtab->addRegular(this, SymName, IsCOMDAT);
+      Sym = Symtab->addRegular(this, SymName);
     }
     SymbolBodies.push_back(Sym);
   }

Modified: lld/trunk/COFF/InputFiles.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/InputFiles.h?rev=319090&r1=319089&r2=319090&view=diff
==============================================================================
--- lld/trunk/COFF/InputFiles.h (original)
+++ lld/trunk/COFF/InputFiles.h Mon Nov 27 12:42:34 2017
@@ -142,7 +142,19 @@ private:
   void initializeSymbols();
   void initializeSEH();
 
-  Symbol *createDefined(COFFSymbolRef Sym, const void *Aux, bool IsFirst);
+  SectionChunk *
+  readSection(uint32_t SectionNumber,
+              const llvm::object::coff_aux_section_definition *Def);
+
+  void readAssociativeDefinition(
+      COFFSymbolRef COFFSym,
+      const llvm::object::coff_aux_section_definition *Def);
+
+  llvm::Optional<Symbol *>
+  createDefined(COFFSymbolRef Sym,
+                std::vector<const llvm::object::coff_aux_section_definition *>
+                    &ComdatDefs);
+  Symbol *createRegular(COFFSymbolRef Sym);
   Symbol *createUndefined(COFFSymbolRef Sym);
 
   std::unique_ptr<COFFObjectFile> COFFObj;

Modified: lld/trunk/COFF/MarkLive.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/MarkLive.cpp?rev=319090&r1=319089&r2=319090&view=diff
==============================================================================
--- lld/trunk/COFF/MarkLive.cpp (original)
+++ lld/trunk/COFF/MarkLive.cpp Mon Nov 27 12:42:34 2017
@@ -52,13 +52,6 @@ void markLive(const std::vector<Chunk *>
 
   while (!Worklist.empty()) {
     SectionChunk *SC = Worklist.pop_back_val();
-
-    // If this section was discarded, there are relocations referring to
-    // discarded sections. Ignore these sections to avoid crashing. They will be
-    // diagnosed during relocation processing.
-    if (SC->isDiscarded())
-      continue;
-
     assert(SC->isLive() && "We mark as live when pushing onto the worklist!");
 
     // Mark all symbols listed in the relocation table for this section.

Modified: lld/trunk/COFF/SymbolTable.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/SymbolTable.cpp?rev=319090&r1=319089&r2=319090&view=diff
==============================================================================
--- lld/trunk/COFF/SymbolTable.cpp (original)
+++ lld/trunk/COFF/SymbolTable.cpp Mon Nov 27 12:42:34 2017
@@ -24,36 +24,6 @@ using namespace llvm;
 namespace lld {
 namespace coff {
 
-enum SymbolPreference {
-  SP_EXISTING = -1,
-  SP_CONFLICT = 0,
-  SP_NEW = 1,
-};
-
-/// Checks if an existing symbol S should be kept or replaced by a new symbol.
-/// Returns SP_EXISTING when S should be kept, SP_NEW when the new symbol
-/// should be kept, and SP_CONFLICT if no valid resolution exists.
-static SymbolPreference compareDefined(Symbol *S, bool WasInserted,
-                                       bool NewIsCOMDAT) {
-  // If the symbol wasn't previously known, the new symbol wins by default.
-  if (WasInserted || !isa<Defined>(S))
-    return SP_NEW;
-
-  // If the existing symbol is a DefinedRegular, both it and the new symbol
-  // must be comdats. In that case, we have no reason to prefer one symbol
-  // over the other, and we keep the existing one. If one of the symbols
-  // is not a comdat, we report a conflict.
-  if (auto *R = dyn_cast<DefinedRegular>(S)) {
-    if (NewIsCOMDAT && R->isCOMDAT())
-      return SP_EXISTING;
-    else
-      return SP_CONFLICT;
-  }
-
-  // Existing symbol is not a DefinedRegular; new symbol wins.
-  return SP_NEW;
-}
-
 SymbolTable *Symtab;
 
 void SymbolTable::addFile(InputFile *File) {
@@ -240,7 +210,7 @@ Symbol *SymbolTable::addSynthetic(String
   return S;
 }
 
-Symbol *SymbolTable::addRegular(InputFile *F, StringRef N, bool IsCOMDAT,
+Symbol *SymbolTable::addRegular(InputFile *F, StringRef N,
                                 const coff_symbol_generic *Sym,
                                 SectionChunk *C) {
   Symbol *S;
@@ -248,22 +218,32 @@ Symbol *SymbolTable::addRegular(InputFil
   std::tie(S, WasInserted) = insert(N);
   if (!isa<BitcodeFile>(F))
     S->IsUsedInRegularObj = true;
-  SymbolPreference SP = compareDefined(S, WasInserted, IsCOMDAT);
-  if (SP == SP_CONFLICT) {
+  if (WasInserted || !isa<DefinedRegular>(S))
+    replaceSymbol<DefinedRegular>(S, F, N, /*IsCOMDAT*/ false,
+                                  /*IsExternal*/ true, Sym, C);
+  else
     reportDuplicate(S, F);
-  } else if (SP == SP_NEW) {
-    replaceSymbol<DefinedRegular>(S, F, N, IsCOMDAT, /*IsExternal*/ true, Sym,
-                                  C);
-  } else if (SP == SP_EXISTING && IsCOMDAT && C) {
-    C->markDiscarded();
-    // Discard associative chunks that we've parsed so far. No need to recurse
-    // because an associative section cannot have children.
-    for (SectionChunk *Child : C->children())
-      Child->markDiscarded();
-  }
   return S;
 }
 
+std::pair<Symbol *, bool>
+SymbolTable::addComdat(InputFile *F, StringRef N,
+                       const coff_symbol_generic *Sym) {
+  Symbol *S;
+  bool WasInserted;
+  std::tie(S, WasInserted) = insert(N);
+  if (!isa<BitcodeFile>(F))
+    S->IsUsedInRegularObj = true;
+  if (WasInserted || !isa<DefinedRegular>(S)) {
+    replaceSymbol<DefinedRegular>(S, F, N, /*IsCOMDAT*/ true,
+                                  /*IsExternal*/ true, Sym, nullptr);
+    return {S, true};
+  }
+  if (!cast<DefinedRegular>(S)->isCOMDAT())
+    reportDuplicate(S, F);
+  return {S, false};
+}
+
 Symbol *SymbolTable::addCommon(InputFile *F, StringRef N, uint64_t Size,
                                const coff_symbol_generic *Sym, CommonChunk *C) {
   Symbol *S;

Modified: lld/trunk/COFF/SymbolTable.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/SymbolTable.h?rev=319090&r1=319089&r2=319090&view=diff
==============================================================================
--- lld/trunk/COFF/SymbolTable.h (original)
+++ lld/trunk/COFF/SymbolTable.h Mon Nov 27 12:42:34 2017
@@ -83,9 +83,12 @@ public:
   Symbol *addUndefined(StringRef Name, InputFile *F, bool IsWeakAlias);
   void addLazy(ArchiveFile *F, const Archive::Symbol Sym);
   Symbol *addAbsolute(StringRef N, COFFSymbolRef S);
-  Symbol *addRegular(InputFile *F, StringRef N, bool IsCOMDAT,
+  Symbol *addRegular(InputFile *F, StringRef N,
                      const llvm::object::coff_symbol_generic *S = nullptr,
                      SectionChunk *C = nullptr);
+  std::pair<Symbol *, bool>
+  addComdat(InputFile *F, StringRef N,
+            const llvm::object::coff_symbol_generic *S = nullptr);
   Symbol *addCommon(InputFile *F, StringRef N, uint64_t Size,
                     const llvm::object::coff_symbol_generic *S = nullptr,
                     CommonChunk *C = nullptr);

Modified: lld/trunk/COFF/Symbols.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Symbols.h?rev=319090&r1=319089&r2=319090&view=diff
==============================================================================
--- lld/trunk/COFF/Symbols.h (original)
+++ lld/trunk/COFF/Symbols.h Mon Nov 27 12:42:34 2017
@@ -169,7 +169,6 @@ public:
   SectionChunk *getChunk() const { return *Data; }
   uint32_t getValue() const { return Sym->Value; }
 
-private:
   SectionChunk **Data;
 };
 

Modified: lld/trunk/test/COFF/reloc-discarded.s
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/reloc-discarded.s?rev=319090&r1=319089&r2=319090&view=diff
==============================================================================
--- lld/trunk/test/COFF/reloc-discarded.s (original)
+++ lld/trunk/test/COFF/reloc-discarded.s Mon Nov 27 12:42:34 2017
@@ -18,7 +18,6 @@ main_global:
 
 	.section	.CRT$XCU,"dr",associative,main_global
 	.p2align	3
-	.globl assoc_global
 assoc_global:
 	.quad	main_global
 




More information about the llvm-commits mailing list