[lld] r301678 - Remove LinkerScript::flush.

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Sat Apr 29 10:43:41 PDT 2017


Yes, it was a debug version of clang.

On Sat, Apr 29, 2017 at 8:07 AM, Rafael Avila de Espindola <
rafael.espindola at gmail.com> wrote:

> Will do.
>
> What exactly is the testcase? Given the time I assume a debug version of
> clang?
>
> Cheers,
> Rafael
>
> Rui Ueyama <ruiu at google.com> writes:
>
> > 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/20170429/c8563fc9/attachment.html>


More information about the llvm-commits mailing list