[PATCH] D40966: [LLD][ELF] Refactor to remove loop copying all Sections in OS->finalize() [NFC]

[Rafael Avila de Espindola via llvm-commits]

LGTM

Thanks!

Peter Smith via Phabricator <reviews at reviews.llvm.org> writes:

> peter.smith created this revision.
> Herald added a subscriber: emaste.
>
> Moving the SHF_LINK_ORDER processing out of OutputSection::finalize() means that we no longer need to copy all InputSections as we now only need the first InputSection.
>
>
> https://reviews.llvm.org/D40966
>
> Files:
>   ELF/OutputSections.cpp
>
>
> Index: ELF/OutputSections.cpp
> ===================================================================
> --- ELF/OutputSections.cpp
> +++ ELF/OutputSections.cpp
> @@ -273,47 +273,47 @@
>  
>  template <class ELFT>
>  static void finalizeShtGroup(OutputSection *OS,
> -                             ArrayRef<InputSection *> Sections) {
> -  assert(Config->Relocatable && Sections.size() == 1);
> +                             InputSection *Section) {
> +  assert(Config->Relocatable);
>  
>    // sh_link field for SHT_GROUP sections should contain the section index of
>    // the symbol table.
>    OS->Link = InX::SymTab->getParent()->SectionIndex;
>  
>    // sh_info then contain index of an entry in symbol table section which
>    // provides signature of the section group.
> -  ObjFile<ELFT> *Obj = Sections[0]->getFile<ELFT>();
> +  ObjFile<ELFT> *Obj = Section->getFile<ELFT>();
>    ArrayRef<Symbol *> Symbols = Obj->getSymbols();
> -  OS->Info = InX::SymTab->getSymbolIndex(Symbols[Sections[0]->Info]);
> +  OS->Info = InX::SymTab->getSymbolIndex(Symbols[Section->Info]);
>  }
>  
>  template <class ELFT> void OutputSection::finalize() {
> -  std::vector<InputSection *> Sections;
> -  for (BaseCommand *Base : SectionCommands) {
> +  InputSection *First = nullptr;
> +  for (BaseCommand *Base : SectionCommands)
>      if (auto *ISD = dyn_cast<InputSectionDescription>(Base)) {
> -      for (InputSection *&IS : ISD->Sections)
> -        Sections.push_back(IS);
> +      if (ISD->Sections.empty())
> +        continue;
> +      First = ISD->Sections.front();
> +      break;
>      }
> -  }
>  
>    if (Flags & SHF_LINK_ORDER) {
>      // We must preserve the link order dependency of sections with the
>      // SHF_LINK_ORDER flag. The dependency is indicated by the sh_link field. We
>      // need to translate the InputSection sh_link to the OutputSection sh_link,
>      // all InputSections in the OutputSection have the same dependency.
> -    if (auto *D = Sections.front()->getLinkOrderDep())
> +    if (auto *D = First->getLinkOrderDep())
>        Link = D->getParent()->SectionIndex;
>    }
>  
>    if (Type == SHT_GROUP) {
> -    finalizeShtGroup<ELFT>(this, Sections);
> +    finalizeShtGroup<ELFT>(this, First);
>      return;
>    }
>  
>    if (!Config->CopyRelocs || (Type != SHT_RELA && Type != SHT_REL))
>      return;
>  
> -  InputSection *First = Sections[0];
>    if (isa<SyntheticSection>(First))
>      return;
>  
>
>
> Index: ELF/OutputSections.cpp
> ===================================================================
> --- ELF/OutputSections.cpp
> +++ ELF/OutputSections.cpp
> @@ -273,47 +273,47 @@
>  
>  template <class ELFT>
>  static void finalizeShtGroup(OutputSection *OS,
> -                             ArrayRef<InputSection *> Sections) {
> -  assert(Config->Relocatable && Sections.size() == 1);
> +                             InputSection *Section) {
> +  assert(Config->Relocatable);
>  
>    // sh_link field for SHT_GROUP sections should contain the section index of
>    // the symbol table.
>    OS->Link = InX::SymTab->getParent()->SectionIndex;
>  
>    // sh_info then contain index of an entry in symbol table section which
>    // provides signature of the section group.
> -  ObjFile<ELFT> *Obj = Sections[0]->getFile<ELFT>();
> +  ObjFile<ELFT> *Obj = Section->getFile<ELFT>();
>    ArrayRef<Symbol *> Symbols = Obj->getSymbols();
> -  OS->Info = InX::SymTab->getSymbolIndex(Symbols[Sections[0]->Info]);
> +  OS->Info = InX::SymTab->getSymbolIndex(Symbols[Section->Info]);
>  }
>  
>  template <class ELFT> void OutputSection::finalize() {
> -  std::vector<InputSection *> Sections;
> -  for (BaseCommand *Base : SectionCommands) {
> +  InputSection *First = nullptr;
> +  for (BaseCommand *Base : SectionCommands)
>      if (auto *ISD = dyn_cast<InputSectionDescription>(Base)) {
> -      for (InputSection *&IS : ISD->Sections)
> -        Sections.push_back(IS);
> +      if (ISD->Sections.empty())
> +        continue;
> +      First = ISD->Sections.front();
> +      break;
>      }
> -  }
>  
>    if (Flags & SHF_LINK_ORDER) {
>      // We must preserve the link order dependency of sections with the
>      // SHF_LINK_ORDER flag. The dependency is indicated by the sh_link field. We
>      // need to translate the InputSection sh_link to the OutputSection sh_link,
>      // all InputSections in the OutputSection have the same dependency.
> -    if (auto *D = Sections.front()->getLinkOrderDep())
> +    if (auto *D = First->getLinkOrderDep())
>        Link = D->getParent()->SectionIndex;
>    }
>  
>    if (Type == SHT_GROUP) {
> -    finalizeShtGroup<ELFT>(this, Sections);
> +    finalizeShtGroup<ELFT>(this, First);
>      return;
>    }
>  
>    if (!Config->CopyRelocs || (Type != SHT_RELA && Type != SHT_REL))
>      return;
>  
> -  InputSection *First = Sections[0];
>    if (isa<SyntheticSection>(First))
>      return;
>