[lld] r301678 - Remove LinkerScript::flush.
Rafael Avila de Espindola via llvm-commits
llvm-commits at lists.llvm.org
Sat Apr 29 08:07:45 PDT 2017
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
>>
More information about the llvm-commits
mailing list