[lld] r301678 - Remove LinkerScript::flush.

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 28 15:42:17 PDT 2017


Rafael,

This patch has significantly slowed down the linker. It used to be able to
link clang in 8 seconds, but after this it takes more than 40 seconds to do
the same thing. I'll revert it for now as the performance degradation is
large. Can you investigate?

On Fri, Apr 28, 2017 at 1:22 PM, Rafael Espindola via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: rafael
> Date: Fri Apr 28 15:22:47 2017
> New Revision: 301678
>
> URL: http://llvm.org/viewvc/llvm-project?rev=301678&view=rev
> Log:
> Remove LinkerScript::flush.
>
> This patch replaces flush with a last ditch attempt at synchronizing
> the section list with the linker script "AST".
>
> The synchronization is a bit of a hack and should in time be avoided
> by creating the AST earlier so that modifications can be made directly
> to it instead of modifying the section list and synchronizing it back.
>
> This is the main step for fixing
> https://bugs.llvm.org/show_bug.cgi?id=32816. With this in place I
> think the only missing thing would be to have processCommands assign
> section indexes as dummy offsets so that the sort in
> OutputSection::finalize works.
>
> With this LinkerScript::assignAddresses becomes much simpler, which
> should help with the thunk work.
>
> Modified:
>     lld/trunk/ELF/LinkerScript.cpp
>     lld/trunk/ELF/LinkerScript.h
>     lld/trunk/ELF/Writer.cpp
>
> Modified: lld/trunk/ELF/LinkerScript.cpp
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/
> LinkerScript.cpp?rev=301678&r1=301677&r2=301678&view=diff
> ============================================================
> ==================
> --- lld/trunk/ELF/LinkerScript.cpp (original)
> +++ lld/trunk/ELF/LinkerScript.cpp Fri Apr 28 15:22:47 2017
> @@ -475,10 +475,15 @@ void LinkerScript::addOrphanSections(Out
>              return Cmd->Name == Name;
>            return false;
>          });
> -    if (I == Opt.Commands.end())
> +    if (I == Opt.Commands.end()) {
>        Factory.addInputSec(S, Name);
> -    else
> -      Factory.addInputSec(S, Name, cast<OutputSectionCommand>(*I)->Sec);
> +    } else {
> +      auto *Cmd = cast<OutputSectionCommand>(*I);
> +      Factory.addInputSec(S, Name, Cmd->Sec);
> +      auto *ISD = make<InputSectionDescription>("");
> +      ISD->Sections.push_back(S);
> +      Cmd->Commands.push_back(ISD);
> +    }
>    }
>  }
>
> @@ -487,8 +492,6 @@ static bool isTbss(OutputSection *Sec) {
>  }
>
>  void LinkerScript::output(InputSection *S) {
> -  if (!AlreadyOutputIS.insert(S).second)
> -    return;
>    bool IsTbss = isTbss(CurOutSec);
>
>    uint64_t Pos = IsTbss ? Dot + ThreadBssOffset : Dot;
> @@ -520,19 +523,9 @@ void LinkerScript::output(InputSection *
>      Dot = Pos;
>  }
>
> -void LinkerScript::flush() {
> -  assert(CurOutSec);
> -  if (!AlreadyOutputOS.insert(CurOutSec).second)
> -    return;
> -  for (InputSection *I : CurOutSec->Sections)
> -    output(I);
> -}
> -
>  void LinkerScript::switchTo(OutputSection *Sec) {
>    if (CurOutSec == Sec)
>      return;
> -  if (AlreadyOutputOS.count(Sec))
> -    return;
>
>    CurOutSec = Sec;
>
> @@ -583,7 +576,7 @@ void LinkerScript::process(BaseCommand &
>
>      if (!Sec->Live)
>        continue;
> -    assert(CurOutSec == Sec->OutSec || AlreadyOutputOS.count(Sec->
> OutSec));
> +    assert(CurOutSec == Sec->OutSec);
>      output(cast<InputSection>(Sec));
>    }
>  }
> @@ -642,19 +635,8 @@ void LinkerScript::assignOffsets(OutputS
>      Dot = CurMemRegion->Offset;
>    switchTo(Sec);
>
> -  // flush() may add orphan sections, so the order of flush() and
> -  // symbol assignments is important. We want to call flush() first so
> -  // that symbols pointing the end of the current section points to
> -  // the location after orphan sections.
> -  auto Mid =
> -      std::find_if(Cmd->Commands.rbegin(), Cmd->Commands.rend(),
> -                   [](BaseCommand *Cmd) { return
> !isa<SymbolAssignment>(Cmd); })
> -          .base();
> -  for (auto I = Cmd->Commands.begin(); I != Mid; ++I)
> -    process(**I);
> -  flush();
> -  for (auto I = Mid, E = Cmd->Commands.end(); I != E; ++I)
> -    process(**I);
> +  for (BaseCommand *Cmd : Cmd->Commands)
> +    process(*Cmd);
>  }
>
>  void LinkerScript::removeEmptyCommands() {
> @@ -824,15 +806,24 @@ void LinkerScript::placeOrphanSections()
>        ++CmdIndex;
>      }
>
> +    // If there is no command corresponding to this output section,
> +    // create one and put a InputSectionDescription in it so that both
> +    // representations agree on which input sections to use.
>      auto Pos = std::find_if(CmdIter, E, [&](BaseCommand *Base) {
>        auto *Cmd = dyn_cast<OutputSectionCommand>(Base);
>        return Cmd && Cmd->Name == Name;
>      });
>      if (Pos == E) {
>        auto *Cmd = make<OutputSectionCommand>(Name);
> -      Cmd->Sec = Sec;
>        Opt.Commands.insert(CmdIter, Cmd);
>        ++CmdIndex;
> +
> +      Cmd->Sec = Sec;
> +      auto *ISD = make<InputSectionDescription>("");
> +      for (InputSection *IS : Sec->Sections)
> +        ISD->Sections.push_back(IS);
> +      Cmd->Commands.push_back(ISD);
> +
>        continue;
>      }
>
> @@ -850,6 +841,49 @@ void LinkerScript::processNonSectionComm
>    }
>  }
>
> +// Do a last effort at synchronizing the linker script "AST" and the
> section
> +// list. This is needed to account for last minute changes, like adding a
> +// .ARM.exidx terminator and sorting SHF_LINK_ORDER sections.
> +//
> +// FIXME: We should instead create the "AST" earlier and the above
> changes would
> +// be done directly in the "AST".
> +//
> +// This can only handle new sections being added and sections being
> reordered.
> +void LinkerScript::synchronize() {
> +  for (BaseCommand *Base : Opt.Commands) {
> +    auto *Cmd = dyn_cast<OutputSectionCommand>(Base);
> +    if (!Cmd)
> +      continue;
> +    ArrayRef<InputSection *> Sections = Cmd->Sec->Sections;
> +    std::vector<InputSectionBase **> ScriptSections;
> +    for (BaseCommand *Base : Cmd->Commands) {
> +      auto *ISD = dyn_cast<InputSectionDescription>(Base);
> +      if (!ISD)
> +        continue;
> +      for (InputSectionBase *&IS : ISD->Sections)
> +        if (IS->Live)
> +          ScriptSections.push_back(&IS);
> +    }
> +    std::vector<InputSectionBase *> Missing;
> +    for (InputSection *IS : Sections)
> +      if (std::find_if(ScriptSections.begin(), ScriptSections.end(),
> +                       [=](InputSectionBase **Base) { return *Base == IS;
> }) ==
> +          ScriptSections.end())
> +        Missing.push_back(IS);
> +    if (!Missing.empty()) {
> +      auto ISD = make<InputSectionDescription>("");
> +      ISD->Sections = Missing;
> +      Cmd->Commands.push_back(ISD);
> +      for (InputSectionBase *&IS : ISD->Sections)
> +        if (IS->Live)
> +          ScriptSections.push_back(&IS);
> +    }
> +    assert(ScriptSections.size() == Sections.size());
> +    for (int I = 0, N = Sections.size(); I < N; ++I)
> +      *ScriptSections[I] = Sections[I];
> +  }
> +}
> +
>  void LinkerScript::assignAddresses(std::vector<PhdrEntry> &Phdrs) {
>    // Assign addresses as instructed by linker script SECTIONS
> sub-commands.
>    Dot = 0;
>
> Modified: lld/trunk/ELF/LinkerScript.h
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/
> LinkerScript.h?rev=301678&r1=301677&r2=301678&view=diff
> ============================================================
> ==================
> --- lld/trunk/ELF/LinkerScript.h (original)
> +++ lld/trunk/ELF/LinkerScript.h Fri Apr 28 15:22:47 2017
> @@ -228,7 +228,6 @@ protected:
>    MemoryRegion *findMemoryRegion(OutputSectionCommand *Cmd);
>
>    void switchTo(OutputSection *Sec);
> -  void flush();
>    void output(InputSection *Sec);
>    void process(BaseCommand &Base);
>
> @@ -242,9 +241,6 @@ protected:
>    OutputSection *CurOutSec = nullptr;
>    MemoryRegion *CurMemRegion = nullptr;
>
> -  llvm::DenseSet<OutputSection *> AlreadyOutputOS;
> -  llvm::DenseSet<InputSectionBase *> AlreadyOutputIS;
> -
>  public:
>    bool hasPhdrsCommands() { return !Opt.PhdrsCommands.empty(); }
>    uint64_t getDot() { return Dot; }
> @@ -271,6 +267,7 @@ public:
>    void assignOffsets(OutputSectionCommand *Cmd);
>    void placeOrphanSections();
>    void processNonSectionCommands();
> +  void synchronize();
>    void assignAddresses(std::vector<PhdrEntry> &Phdrs);
>    int getSectionIndex(StringRef Name);
>
>
> Modified: lld/trunk/ELF/Writer.cpp
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Writer.
> cpp?rev=301678&r1=301677&r2=301678&view=diff
> ============================================================
> ==================
> --- lld/trunk/ELF/Writer.cpp (original)
> +++ lld/trunk/ELF/Writer.cpp Fri Apr 28 15:22:47 2017
> @@ -254,6 +254,7 @@ template <class ELFT> void Writer<ELFT>:
>        fixSectionAlignments();
>        Script->fabricateDefaultCommands(Config->MaxPageSize);
>      }
> +    Script->synchronize();
>      Script->assignAddresses(Phdrs);
>
>      // Remove empty PT_LOAD to avoid causing the dynamic linker to try to
> mmap a
> @@ -1080,6 +1081,7 @@ static void removeUnusedSyntheticSection
>
>      SS->OutSec->Sections.erase(std::find(SS->OutSec->Sections.begin(),
>                                           SS->OutSec->Sections.end(), SS));
> +    SS->Live = false;
>      // If there are no other sections in the output section, remove it
> from the
>      // output.
>      if (SS->OutSec->Sections.empty())
>
>
> _______________________________________________
> 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/20170428/83f2546d/attachment.html>


More information about the llvm-commits mailing list