"raw" patch adding comdat support to lld elf2

[Rui Ueyama via llvm-commits]

Overall looking good. Nice use of PointerIntPair.

   for (const Elf_Shdr &Sec : Obj.sections()) {
+    ++I;
+    if (Sections[I] == &InputSection<ELFT>::Discarded)
+      continue;
+
     switch (Sec.sh_type) {
+    case SHT_GROUP: {
+      Sections[I] = &InputSection<ELFT>::Discarded;
+      uint32_t SymtabdSectionIndex = Sec.sh_link;
+      ErrorOr<const Elf_Shdr *> SecOrErr = Obj.getSection(SymtabdSectionIndex);
+      error(SecOrErr);
+      const Elf_Shdr *SymtabSec = *SecOrErr;
+      uint32_t SymIndex = Sec.sh_info;
+      const Elf_Sym *Sym = Obj.getSymbol(SymtabSec, SymIndex);
+      ErrorOr<StringRef> StringTableOrErr =
+          Obj.getStringTableForSymtab(*SymtabSec);
+      error(StringTableOrErr);
+      ErrorOr<StringRef> SignatureOrErr = Sym->getName(*StringTableOrErr);
+      error(SignatureOrErr);
+      if (Symtab.insertComdat(*SignatureOrErr))
+        continue;
+      typedef support::detail::packed_endian_specific_integral<
+          uint32_t, ELFT::TargetEndianness, 2> EntryType;
+      ErrorOr<ArrayRef<EntryType>> EntriesOrErr =
+          Obj.template getSectionContentsAsArray<EntryType>(&Sec);
+      error(EntriesOrErr.getError());
+      ArrayRef<EntryType> Entries = *EntriesOrErr;
+      if (Entries.empty() || Entries[0] != GRP_COMDAT)
+        error("Unsupported SHT_GROUP format");
+      for (EntryType E : Entries.slice(1)) {
+        uint32_t SecIndex = E;
+        if (SecIndex >= Size)
+          error("Invalid section index in group");
+        Sections[SecIndex] = &InputSection<ELFT>::Discarded;
+      }
+      break;
+    }


I'd define

  StringRef ObjectFile<ELFT>::getShtGroupSignature(const Elf_Shdr &)

and

  ArrayRef<EntryType> ObjectFile<ELFT>::getShtGroupEntries(const Elf_Shdr &)

so that I can write like this

  Sections[I] = &InputSection<ELFT>::Discarded;
  if (Symtab.insertComdat(getShtGroupSignature(Sec))
    continue;
  for (EntryType E : getShtGroupEntries(Sec)) {
    uint32_t Idx = E;
    if (Idx >= Size)
      error(...);
    Sections[Idx] = &InputSection<ELFT>::Discarded;
  }

+  typedef llvm::MapVector<StringRef, llvm::PointerIntPair<Symbol *,
1>> MapType;
+  typedef MapType::const_iterator raw_iter;
+  struct iter : public llvm::iterator_adaptor_base<
+                    iter, raw_iter,
+                    typename std::iterator_traits<raw_iter>::iterator_category,
+                    const std::pair<StringRef, Symbol *>> {
+    iter(raw_iter u) : iter::iterator_adaptor_base(u) {}
+    const std::pair<StringRef, Symbol *> operator*() const {
+      const std::pair<StringRef, llvm::PointerIntPair<Symbol *, 1>> &V = *I;
+      std::pair<StringRef, Symbol *> V2(V.first, V.second.getPointer());
+      return V2;

Why don't you return V2 directly (without using V2)?

On Thu, Oct 8, 2015 at 3:06 PM, Rafael Espíndola <rafael.espindola at gmail.com
> wrote:

> The attached patch adds comdat handling to the new elf lld.
>
> The implementation is direct translation to c++ of the rules in
>
> http://www.sco.com/developers/gabi/latest/ch4.sheader.html#section_groups
>
> The patch is "raw" is that it still has duplicated code and is missing
> comments.
>
> I have attached the patch, run script and benchmark results linking
> clang, but the summary is
>
> gold:
> * clang is 77 898 792 bytes.
> * 1.125165215 seconds time elapsed
>
> trunk:
> * clang is 92 442 088 bytes.
> * 0.495383463 seconds time elapsed
>
> patch:
> * clang is 87 491 896 bytes.
> * 0.475477433 seconds time elapsed
>
> Cheers,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151008/eca6022f/attachment.html>