[lld] r316731 - De-template EhFrameSection. NFC.
Rui Ueyama via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 31 19:52:11 PDT 2017
I did not benchmark these particular patches as it seems neutral to
performance to me. Do you have any concerns? I think as we improved
performance in the last few months, we actually have some room to aim for
readability and simplicity even if it slows down the linker a little bit.
On Tue, Oct 31, 2017 at 7:28 AM, Rafael Avila de Espindola <
rafael.espindola at gmail.com> wrote:
> 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/InputSecti
> on.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/SyntheticS
> ections.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/SyntheticS
> ections.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->getParen
> t())
> > + if (!InX::EhFrame->empty() && In<ELFT>::EhFrameHdr &&
> > + InX::EhFrame->getParent() && In<ELFT>::EhFrameHdr->getParent())
> > AddHdr(PT_GNU_EH_FRAME, In<ELFT>::EhFrameHdr->getParen
> t()->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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171031/ed24ee83/attachment.html>
More information about the llvm-commits
mailing list