[lld] r316731 - De-template EhFrameSection. NFC.

Rafael Avila de Espindola via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 31 07:28:22 PDT 2017


Hi,

Have you benchmarked any of these patches? I am going over commits and
in the last two weeks I found two very noticeable regressions.

I don't have the time or the motivation to manually benchmark every
change. That burden has to be on the author until we setup a bot.

In fact, I think that we must slow down development until we have a
reasonable performance tracking.

Cheers,
Rafael

Rui Ueyama via llvm-commits <llvm-commits at lists.llvm.org> writes:

> Author: ruiu
> Date: Thu Oct 26 20:13:39 2017
> New Revision: 316731
>
> URL: http://llvm.org/viewvc/llvm-project?rev=316731&view=rev
> Log:
> De-template EhFrameSection. NFC.
>
> Modified:
>     lld/trunk/ELF/InputSection.h
>     lld/trunk/ELF/SyntheticSections.cpp
>     lld/trunk/ELF/SyntheticSections.h
>     lld/trunk/ELF/Writer.cpp
>
> Modified: lld/trunk/ELF/InputSection.h
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputSection.h?rev=316731&r1=316730&r2=316731&view=diff
> ==============================================================================
> --- lld/trunk/ELF/InputSection.h (original)
> +++ lld/trunk/ELF/InputSection.h Thu Oct 26 20:13:39 2017
> @@ -30,7 +30,6 @@ struct SectionPiece;
>  
>  class DefinedRegular;
>  class SyntheticSection;
> -template <class ELFT> class EhFrameSection;
>  class MergeSyntheticSection;
>  template <class ELFT> class ObjFile;
>  class OutputSection;
>
> Modified: lld/trunk/ELF/SyntheticSections.cpp
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/SyntheticSections.cpp?rev=316731&r1=316730&r2=316731&view=diff
> ==============================================================================
> --- lld/trunk/ELF/SyntheticSections.cpp (original)
> +++ lld/trunk/ELF/SyntheticSections.cpp Thu Oct 26 20:13:39 2017
> @@ -397,17 +397,14 @@ void BuildIdSection::writeBuildId(ArrayR
>    }
>  }
>  
> -template <class ELFT>
> -EhFrameSection<ELFT>::EhFrameSection()
> +EhFrameSection::EhFrameSection()
>      : SyntheticSection(SHF_ALLOC, SHT_PROGBITS, 1, ".eh_frame") {}
>  
>  // Search for an existing CIE record or create a new one.
>  // CIE records from input object files are uniquified by their contents
>  // and where their relocations point to.
> -template <class ELFT>
> -template <class RelTy>
> -CieRecord *EhFrameSection<ELFT>::addCie(EhSectionPiece &Cie,
> -                                        ArrayRef<RelTy> Rels) {
> +template <class ELFT, class RelTy>
> +CieRecord *EhFrameSection::addCie(EhSectionPiece &Cie, ArrayRef<RelTy> Rels) {
>    auto *Sec = cast<EhInputSection>(Cie.Sec);
>    if (read32(Cie.data().data() + 4, Config->Endianness) != 0)
>      fatal(toString(Sec) + ": CIE expected at beginning of .eh_frame");
> @@ -432,10 +429,8 @@ CieRecord *EhFrameSection<ELFT>::addCie(
>  
>  // There is one FDE per function. Returns true if a given FDE
>  // points to a live function.
> -template <class ELFT>
> -template <class RelTy>
> -bool EhFrameSection<ELFT>::isFdeLive(EhSectionPiece &Fde,
> -                                     ArrayRef<RelTy> Rels) {
> +template <class ELFT, class RelTy>
> +bool EhFrameSection::isFdeLive(EhSectionPiece &Fde, ArrayRef<RelTy> Rels) {
>    auto *Sec = cast<EhInputSection>(Fde.Sec);
>    unsigned FirstRelI = Fde.FirstRelocation;
>  
> @@ -461,10 +456,8 @@ bool EhFrameSection<ELFT>::isFdeLive(EhS
>  // is one CIE record per input object file which is followed by
>  // a list of FDEs. This function searches an existing CIE or create a new
>  // one and associates FDEs to the CIE.
> -template <class ELFT>
> -template <class RelTy>
> -void EhFrameSection<ELFT>::addSectionAux(EhInputSection *Sec,
> -                                         ArrayRef<RelTy> Rels) {
> +template <class ELFT, class RelTy>
> +void EhFrameSection::addSectionAux(EhInputSection *Sec, ArrayRef<RelTy> Rels) {
>    DenseMap<size_t, CieRecord *> OffsetToCie;
>    for (EhSectionPiece &Piece : Sec->Pieces) {
>      // The empty record is the end marker.
> @@ -474,7 +467,7 @@ void EhFrameSection<ELFT>::addSectionAux
>      size_t Offset = Piece.InputOff;
>      uint32_t ID = read32(Piece.data().data() + 4, Config->Endianness);
>      if (ID == 0) {
> -      OffsetToCie[Offset] = addCie(Piece, Rels);
> +      OffsetToCie[Offset] = addCie<ELFT>(Piece, Rels);
>        continue;
>      }
>  
> @@ -483,15 +476,14 @@ void EhFrameSection<ELFT>::addSectionAux
>      if (!Rec)
>        fatal(toString(Sec) + ": invalid CIE reference");
>  
> -    if (!isFdeLive(Piece, Rels))
> +    if (!isFdeLive<ELFT>(Piece, Rels))
>        continue;
>      Rec->Fdes.push_back(&Piece);
>      NumFdes++;
>    }
>  }
>  
> -template <class ELFT>
> -void EhFrameSection<ELFT>::addSection(InputSectionBase *C) {
> +template <class ELFT> void EhFrameSection::addSection(InputSectionBase *C) {
>    auto *Sec = cast<EhInputSection>(C);
>    Sec->Parent = this;
>  
> @@ -509,9 +501,9 @@ void EhFrameSection<ELFT>::addSection(In
>      return;
>  
>    if (Sec->AreRelocsRela)
> -    addSectionAux(Sec, Sec->template relas<ELFT>());
> +    addSectionAux<ELFT>(Sec, Sec->template relas<ELFT>());
>    else
> -    addSectionAux(Sec, Sec->template rels<ELFT>());
> +    addSectionAux<ELFT>(Sec, Sec->template rels<ELFT>());
>  }
>  
>  static void writeCieFde(uint8_t *Buf, ArrayRef<uint8_t> D) {
> @@ -526,7 +518,7 @@ static void writeCieFde(uint8_t *Buf, Ar
>    write32(Buf, Aligned - 4, Config->Endianness);
>  }
>  
> -template <class ELFT> void EhFrameSection<ELFT>::finalizeContents() {
> +void EhFrameSection::finalizeContents() {
>    if (this->Size)
>      return; // Already finalized.
>  
> @@ -554,8 +546,7 @@ template <class ELFT> void EhFrameSectio
>  // to get an FDE from an address to which FDE is applied. This function
>  // returns a list of such pairs.
>  template <class ELFT>
> -std::vector<typename EhFrameSection<ELFT>::FdeData>
> -EhFrameSection<ELFT>::getFdeData() const {
> +std::vector<EhFrameSection::FdeData> EhFrameSection::getFdeData() const {
>    uint8_t *Buf = getParent()->Loc + OutSecOff;
>    std::vector<FdeData> Ret;
>  
> @@ -586,9 +577,8 @@ static uint64_t readFdeAddr(uint8_t *Buf
>  
>  // Returns the VA to which a given FDE (on a mmap'ed buffer) is applied to.
>  // We need it to create .eh_frame_hdr section.
> -template <class ELFT>
> -uint64_t EhFrameSection<ELFT>::getFdePc(uint8_t *Buf, size_t FdeOff,
> -                                        uint8_t Enc) const {
> +uint64_t EhFrameSection::getFdePc(uint8_t *Buf, size_t FdeOff,
> +                                  uint8_t Enc) const {
>    // The starting address to which this FDE applies is
>    // stored at FDE + 8 byte.
>    size_t Off = FdeOff + 8;
> @@ -600,7 +590,7 @@ uint64_t EhFrameSection<ELFT>::getFdePc(
>    fatal("unknown FDE size relative encoding");
>  }
>  
> -template <class ELFT> void EhFrameSection<ELFT>::writeTo(uint8_t *Buf) {
> +void EhFrameSection::writeTo(uint8_t *Buf) {
>    // Write CIE and FDE records.
>    for (CieRecord *Rec : CieRecords) {
>      size_t CieOffset = Rec->Cie->OutputOff;
> @@ -1972,10 +1962,10 @@ EhFrameHeader<ELFT>::EhFrameHeader()
>  // the starting PC from where FDEs covers, and the FDE's address.
>  // It is sorted by PC.
>  template <class ELFT> void EhFrameHeader<ELFT>::writeTo(uint8_t *Buf) {
> -  typedef typename EhFrameSection<ELFT>::FdeData FdeData;
> +  typedef EhFrameSection::FdeData FdeData;
>    const endianness E = ELFT::TargetEndianness;
>  
> -  std::vector<FdeData> Fdes = In<ELFT>::EhFrame->getFdeData();
> +  std::vector<FdeData> Fdes = InX::EhFrame->getFdeData<ELFT>();
>  
>    // Sort the FDE list by their PC and uniqueify. Usually there is only
>    // one FDE for a PC (i.e. function), but if ICF merges two functions
> @@ -1989,7 +1979,7 @@ template <class ELFT> void EhFrameHeader
>    Buf[1] = DW_EH_PE_pcrel | DW_EH_PE_sdata4;
>    Buf[2] = DW_EH_PE_udata4;
>    Buf[3] = DW_EH_PE_datarel | DW_EH_PE_sdata4;
> -  write32<E>(Buf + 4, In<ELFT>::EhFrame->getParent()->Addr - this->getVA() - 4);
> +  write32<E>(Buf + 4, InX::EhFrame->getParent()->Addr - this->getVA() - 4);
>    write32<E>(Buf + 8, Fdes.size());
>    Buf += 12;
>  
> @@ -2003,11 +1993,11 @@ template <class ELFT> void EhFrameHeader
>  
>  template <class ELFT> size_t EhFrameHeader<ELFT>::getSize() const {
>    // .eh_frame_hdr has a 12 bytes header followed by an array of FDEs.
> -  return 12 + In<ELFT>::EhFrame->NumFdes * 8;
> +  return 12 + InX::EhFrame->NumFdes * 8;
>  }
>  
>  template <class ELFT> bool EhFrameHeader<ELFT>::empty() const {
> -  return In<ELFT>::EhFrame->empty();
> +  return InX::EhFrame->empty();
>  }
>  
>  template <class ELFT>
> @@ -2412,6 +2402,7 @@ InputSection *InX::ARMAttributes;
>  BssSection *InX::Bss;
>  BssSection *InX::BssRelRo;
>  BuildIdSection *InX::BuildId;
> +EhFrameSection *InX::EhFrame;
>  SyntheticSection *InX::Dynamic;
>  StringTableSection *InX::DynStrTab;
>  SymbolTableBaseSection *InX::DynSymTab;
> @@ -2435,6 +2426,11 @@ template GdbIndexSection *elf::createGdb
>  template GdbIndexSection *elf::createGdbIndex<ELF64LE>();
>  template GdbIndexSection *elf::createGdbIndex<ELF64BE>();
>  
> +template void EhFrameSection::addSection<ELF32LE>(InputSectionBase *);
> +template void EhFrameSection::addSection<ELF32BE>(InputSectionBase *);
> +template void EhFrameSection::addSection<ELF64LE>(InputSectionBase *);
> +template void EhFrameSection::addSection<ELF64BE>(InputSectionBase *);
> +
>  template void PltSection::addEntry<ELF32LE>(SymbolBody &Sym);
>  template void PltSection::addEntry<ELF32BE>(SymbolBody &Sym);
>  template void PltSection::addEntry<ELF64LE>(SymbolBody &Sym);
> @@ -2499,8 +2495,3 @@ template class elf::VersionDefinitionSec
>  template class elf::VersionDefinitionSection<ELF32BE>;
>  template class elf::VersionDefinitionSection<ELF64LE>;
>  template class elf::VersionDefinitionSection<ELF64BE>;
> -
> -template class elf::EhFrameSection<ELF32LE>;
> -template class elf::EhFrameSection<ELF32BE>;
> -template class elf::EhFrameSection<ELF64LE>;
> -template class elf::EhFrameSection<ELF64BE>;
>
> Modified: lld/trunk/ELF/SyntheticSections.h
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/SyntheticSections.h?rev=316731&r1=316730&r2=316731&view=diff
> ==============================================================================
> --- lld/trunk/ELF/SyntheticSections.h (original)
> +++ lld/trunk/ELF/SyntheticSections.h Thu Oct 26 20:13:39 2017
> @@ -62,11 +62,7 @@ struct CieRecord {
>  };
>  
>  // Section for .eh_frame.
> -template <class ELFT> class EhFrameSection final : public SyntheticSection {
> -  typedef typename ELFT::Shdr Elf_Shdr;
> -  typedef typename ELFT::Rel Elf_Rel;
> -  typedef typename ELFT::Rela Elf_Rela;
> -
> +class EhFrameSection final : public SyntheticSection {
>  public:
>    EhFrameSection();
>    void writeTo(uint8_t *Buf) override;
> @@ -74,7 +70,7 @@ public:
>    bool empty() const override { return Sections.empty(); }
>    size_t getSize() const override { return Size; }
>  
> -  void addSection(InputSectionBase *S);
> +  template <class ELFT> void addSection(InputSectionBase *S);
>  
>    std::vector<EhInputSection *> Sections;
>    size_t NumFdes = 0;
> @@ -84,17 +80,18 @@ public:
>      uint32_t FdeVA;
>    };
>  
> -  std::vector<FdeData> getFdeData() const;
> +  template <class ELFT> std::vector<FdeData> getFdeData() const;
>  
>  private:
>    uint64_t Size = 0;
> -  template <class RelTy>
> +
> +  template <class ELFT, class RelTy>
>    void addSectionAux(EhInputSection *S, llvm::ArrayRef<RelTy> Rels);
>  
> -  template <class RelTy>
> +  template <class ELFT, class RelTy>
>    CieRecord *addCie(EhSectionPiece &Piece, ArrayRef<RelTy> Rels);
>  
> -  template <class RelTy>
> +  template <class ELFT, class RelTy>
>    bool isFdeLive(EhSectionPiece &Piece, ArrayRef<RelTy> Rels);
>  
>    uint64_t getFdePc(uint8_t *Buf, size_t Off, uint8_t Enc) const;
> @@ -814,6 +811,7 @@ struct InX {
>    static BssSection *Bss;
>    static BssSection *BssRelRo;
>    static BuildIdSection *BuildId;
> +  static EhFrameSection *EhFrame;
>    static SyntheticSection *Dynamic;
>    static StringTableSection *DynStrTab;
>    static SymbolTableBaseSection *DynSymTab;
> @@ -835,7 +833,6 @@ struct InX {
>  
>  template <class ELFT> struct In {
>    static EhFrameHeader<ELFT> *EhFrameHdr;
> -  static EhFrameSection<ELFT> *EhFrame;
>    static RelocationSection<ELFT> *RelaDyn;
>    static RelocationSection<ELFT> *RelaPlt;
>    static RelocationSection<ELFT> *RelaIplt;
> @@ -845,7 +842,6 @@ template <class ELFT> struct In {
>  };
>  
>  template <class ELFT> EhFrameHeader<ELFT> *In<ELFT>::EhFrameHdr;
> -template <class ELFT> EhFrameSection<ELFT> *In<ELFT>::EhFrame;
>  template <class ELFT> RelocationSection<ELFT> *In<ELFT>::RelaDyn;
>  template <class ELFT> RelocationSection<ELFT> *In<ELFT>::RelaPlt;
>  template <class ELFT> RelocationSection<ELFT> *In<ELFT>::RelaIplt;
>
> Modified: lld/trunk/ELF/Writer.cpp
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Writer.cpp?rev=316731&r1=316730&r2=316731&view=diff
> ==============================================================================
> --- lld/trunk/ELF/Writer.cpp (original)
> +++ lld/trunk/ELF/Writer.cpp Thu Oct 26 20:13:39 2017
> @@ -139,7 +139,7 @@ template <class ELFT> static void combin
>      if (!ES || !ES->Live)
>        continue;
>  
> -    In<ELFT>::EhFrame->addSection(ES);
> +    InX::EhFrame->addSection<ELFT>(ES);
>      S = nullptr;
>    }
>  
> @@ -376,8 +376,8 @@ template <class ELFT> void Writer<ELFT>:
>        In<ELFT>::EhFrameHdr = make<EhFrameHeader<ELFT>>();
>        Add(In<ELFT>::EhFrameHdr);
>      }
> -    In<ELFT>::EhFrame = make<EhFrameSection<ELFT>>();
> -    Add(In<ELFT>::EhFrame);
> +    InX::EhFrame = make<EhFrameSection>();
> +    Add(InX::EhFrame);
>    }
>  
>    if (InX::SymTab)
> @@ -853,7 +853,7 @@ void Writer<ELFT>::forEachRelSec(std::fu
>    for (InputSectionBase *IS : InputSections)
>      if (IS->Live && isa<InputSection>(IS) && (IS->Flags & SHF_ALLOC))
>        Fn(*IS);
> -  for (EhInputSection *ES : In<ELFT>::EhFrame->Sections)
> +  for (EhInputSection *ES : InX::EhFrame->Sections)
>      Fn(*ES);
>  }
>  
> @@ -1252,7 +1252,7 @@ template <class ELFT> void Writer<ELFT>:
>    // This responsible for splitting up .eh_frame section into
>    // pieces. The relocation scan uses those pieces, so this has to be
>    // earlier.
> -  applySynthetic({In<ELFT>::EhFrame},
> +  applySynthetic({InX::EhFrame},
>                   [](SyntheticSection *SS) { SS->finalizeContents(); });
>  
>    for (Symbol *S : Symtab->getSymbols())
> @@ -1532,8 +1532,8 @@ template <class ELFT> std::vector<PhdrEn
>      Ret.push_back(RelRo);
>  
>    // PT_GNU_EH_FRAME is a special section pointing on .eh_frame_hdr.
> -  if (!In<ELFT>::EhFrame->empty() && In<ELFT>::EhFrameHdr &&
> -      In<ELFT>::EhFrame->getParent() && In<ELFT>::EhFrameHdr->getParent())
> +  if (!InX::EhFrame->empty() && In<ELFT>::EhFrameHdr &&
> +      InX::EhFrame->getParent() && In<ELFT>::EhFrameHdr->getParent())
>      AddHdr(PT_GNU_EH_FRAME, In<ELFT>::EhFrameHdr->getParent()->getPhdrFlags())
>          ->add(In<ELFT>::EhFrameHdr->getParent());
>  
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list