<div dir="ltr">Yes, it was a debug version of clang.</div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Apr 29, 2017 at 8:07 AM, Rafael Avila de Espindola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Will do.<br>
<br>
What exactly is the testcase? Given the time I assume a debug version of<br>
clang?<br>
<br>
Cheers,<br>
Rafael<br>
<div class="HOEnZb"><div class="h5"><br>
Rui Ueyama <<a href="mailto:ruiu@google.com">ruiu@google.com</a>> writes:<br>
<br>
> Rafael,<br>
><br>
> This patch has significantly slowed down the linker. It used to be able to<br>
> link clang in 8 seconds, but after this it takes more than 40 seconds to do<br>
> the same thing. I'll revert it for now as the performance degradation is<br>
> large. Can you investigate?<br>
><br>
> On Fri, Apr 28, 2017 at 1:22 PM, Rafael Espindola via llvm-commits <<br>
> <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
><br>
>> Author: rafael<br>
>> Date: Fri Apr 28 15:22:47 2017<br>
>> New Revision: 301678<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=301678&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=301678&view=rev</a><br>
>> Log:<br>
>> Remove LinkerScript::flush.<br>
>><br>
>> This patch replaces flush with a last ditch attempt at synchronizing<br>
>> the section list with the linker script "AST".<br>
>><br>
>> The synchronization is a bit of a hack and should in time be avoided<br>
>> by creating the AST earlier so that modifications can be made directly<br>
>> to it instead of modifying the section list and synchronizing it back.<br>
>><br>
>> This is the main step for fixing<br>
>> <a href="https://bugs.llvm.org/show_bug.cgi?id=32816" rel="noreferrer" target="_blank">https://bugs.llvm.org/show_<wbr>bug.cgi?id=32816</a>. With this in place I<br>
>> think the only missing thing would be to have processCommands assign<br>
>> section indexes as dummy offsets so that the sort in<br>
>> OutputSection::finalize works.<br>
>><br>
>> With this LinkerScript::assignAddresses becomes much simpler, which<br>
>> should help with the thunk work.<br>
>><br>
>> Modified:<br>
>> lld/trunk/ELF/LinkerScript.cpp<br>
>> lld/trunk/ELF/LinkerScript.h<br>
>> lld/trunk/ELF/Writer.cpp<br>
>><br>
>> Modified: lld/trunk/ELF/LinkerScript.cpp<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/lld/trunk/ELF/</a><br>
>> LinkerScript.cpp?rev=301678&<wbr>r1=301677&r2=301678&view=diff<br>
>> ==============================<wbr>==============================<br>
>> ==================<br>
>> --- lld/trunk/ELF/LinkerScript.cpp (original)<br>
>> +++ lld/trunk/ELF/LinkerScript.cpp Fri Apr 28 15:22:47 2017<br>
>> @@ -475,10 +475,15 @@ void LinkerScript::<wbr>addOrphanSections(Out<br>
>> return Cmd->Name == Name;<br>
>> return false;<br>
>> });<br>
>> - if (I == Opt.Commands.end())<br>
>> + if (I == Opt.Commands.end()) {<br>
>> Factory.addInputSec(S, Name);<br>
>> - else<br>
>> - Factory.addInputSec(S, Name, cast<OutputSectionCommand>(*I)<wbr>->Sec);<br>
>> + } else {<br>
>> + auto *Cmd = cast<OutputSectionCommand>(*I)<wbr>;<br>
>> + Factory.addInputSec(S, Name, Cmd->Sec);<br>
>> + auto *ISD = make<InputSectionDescription>(<wbr>"");<br>
>> + ISD->Sections.push_back(S);<br>
>> + Cmd->Commands.push_back(ISD);<br>
>> + }<br>
>> }<br>
>> }<br>
>><br>
>> @@ -487,8 +492,6 @@ static bool isTbss(OutputSection *Sec) {<br>
>> }<br>
>><br>
>> void LinkerScript::output(<wbr>InputSection *S) {<br>
>> - if (!AlreadyOutputIS.insert(S).<wbr>second)<br>
>> - return;<br>
>> bool IsTbss = isTbss(CurOutSec);<br>
>><br>
>> uint64_t Pos = IsTbss ? Dot + ThreadBssOffset : Dot;<br>
>> @@ -520,19 +523,9 @@ void LinkerScript::output(<wbr>InputSection *<br>
>> Dot = Pos;<br>
>> }<br>
>><br>
>> -void LinkerScript::flush() {<br>
>> - assert(CurOutSec);<br>
>> - if (!AlreadyOutputOS.insert(<wbr>CurOutSec).second)<br>
>> - return;<br>
>> - for (InputSection *I : CurOutSec->Sections)<br>
>> - output(I);<br>
>> -}<br>
>> -<br>
>> void LinkerScript::switchTo(<wbr>OutputSection *Sec) {<br>
>> if (CurOutSec == Sec)<br>
>> return;<br>
>> - if (AlreadyOutputOS.count(Sec))<br>
>> - return;<br>
>><br>
>> CurOutSec = Sec;<br>
>><br>
>> @@ -583,7 +576,7 @@ void LinkerScript::process(<wbr>BaseCommand &<br>
>><br>
>> if (!Sec->Live)<br>
>> continue;<br>
>> - assert(CurOutSec == Sec->OutSec || AlreadyOutputOS.count(Sec-><br>
>> OutSec));<br>
>> + assert(CurOutSec == Sec->OutSec);<br>
>> output(cast<InputSection>(Sec)<wbr>);<br>
>> }<br>
>> }<br>
>> @@ -642,19 +635,8 @@ void LinkerScript::assignOffsets(<wbr>OutputS<br>
>> Dot = CurMemRegion->Offset;<br>
>> switchTo(Sec);<br>
>><br>
>> - // flush() may add orphan sections, so the order of flush() and<br>
>> - // symbol assignments is important. We want to call flush() first so<br>
>> - // that symbols pointing the end of the current section points to<br>
>> - // the location after orphan sections.<br>
>> - auto Mid =<br>
>> - std::find_if(Cmd->Commands.<wbr>rbegin(), Cmd->Commands.rend(),<br>
>> - [](BaseCommand *Cmd) { return<br>
>> !isa<SymbolAssignment>(Cmd); })<br>
>> - .base();<br>
>> - for (auto I = Cmd->Commands.begin(); I != Mid; ++I)<br>
>> - process(**I);<br>
>> - flush();<br>
>> - for (auto I = Mid, E = Cmd->Commands.end(); I != E; ++I)<br>
>> - process(**I);<br>
>> + for (BaseCommand *Cmd : Cmd->Commands)<br>
>> + process(*Cmd);<br>
>> }<br>
>><br>
>> void LinkerScript::<wbr>removeEmptyCommands() {<br>
>> @@ -824,15 +806,24 @@ void LinkerScript::<wbr>placeOrphanSections()<br>
>> ++CmdIndex;<br>
>> }<br>
>><br>
>> + // If there is no command corresponding to this output section,<br>
>> + // create one and put a InputSectionDescription in it so that both<br>
>> + // representations agree on which input sections to use.<br>
>> auto Pos = std::find_if(CmdIter, E, [&](BaseCommand *Base) {<br>
>> auto *Cmd = dyn_cast<OutputSectionCommand><wbr>(Base);<br>
>> return Cmd && Cmd->Name == Name;<br>
>> });<br>
>> if (Pos == E) {<br>
>> auto *Cmd = make<OutputSectionCommand>(<wbr>Name);<br>
>> - Cmd->Sec = Sec;<br>
>> Opt.Commands.insert(CmdIter, Cmd);<br>
>> ++CmdIndex;<br>
>> +<br>
>> + Cmd->Sec = Sec;<br>
>> + auto *ISD = make<InputSectionDescription>(<wbr>"");<br>
>> + for (InputSection *IS : Sec->Sections)<br>
>> + ISD->Sections.push_back(IS);<br>
>> + Cmd->Commands.push_back(ISD);<br>
>> +<br>
>> continue;<br>
>> }<br>
>><br>
>> @@ -850,6 +841,49 @@ void LinkerScript::<wbr>processNonSectionComm<br>
>> }<br>
>> }<br>
>><br>
>> +// Do a last effort at synchronizing the linker script "AST" and the<br>
>> section<br>
>> +// list. This is needed to account for last minute changes, like adding a<br>
>> +// .ARM.exidx terminator and sorting SHF_LINK_ORDER sections.<br>
>> +//<br>
>> +// FIXME: We should instead create the "AST" earlier and the above<br>
>> changes would<br>
>> +// be done directly in the "AST".<br>
>> +//<br>
>> +// This can only handle new sections being added and sections being<br>
>> reordered.<br>
>> +void LinkerScript::synchronize() {<br>
>> + for (BaseCommand *Base : Opt.Commands) {<br>
>> + auto *Cmd = dyn_cast<OutputSectionCommand><wbr>(Base);<br>
>> + if (!Cmd)<br>
>> + continue;<br>
>> + ArrayRef<InputSection *> Sections = Cmd->Sec->Sections;<br>
>> + std::vector<InputSectionBase **> ScriptSections;<br>
>> + for (BaseCommand *Base : Cmd->Commands) {<br>
>> + auto *ISD = dyn_cast<<wbr>InputSectionDescription>(Base)<wbr>;<br>
>> + if (!ISD)<br>
>> + continue;<br>
>> + for (InputSectionBase *&IS : ISD->Sections)<br>
>> + if (IS->Live)<br>
>> + ScriptSections.push_back(&IS);<br>
>> + }<br>
>> + std::vector<InputSectionBase *> Missing;<br>
>> + for (InputSection *IS : Sections)<br>
>> + if (std::find_if(ScriptSections.<wbr>begin(), ScriptSections.end(),<br>
>> + [=](InputSectionBase **Base) { return *Base == IS;<br>
>> }) ==<br>
>> + ScriptSections.end())<br>
>> + Missing.push_back(IS);<br>
>> + if (!Missing.empty()) {<br>
>> + auto ISD = make<InputSectionDescription>(<wbr>"");<br>
>> + ISD->Sections = Missing;<br>
>> + Cmd->Commands.push_back(ISD);<br>
>> + for (InputSectionBase *&IS : ISD->Sections)<br>
>> + if (IS->Live)<br>
>> + ScriptSections.push_back(&IS);<br>
>> + }<br>
>> + assert(ScriptSections.size() == Sections.size());<br>
>> + for (int I = 0, N = Sections.size(); I < N; ++I)<br>
>> + *ScriptSections[I] = Sections[I];<br>
>> + }<br>
>> +}<br>
>> +<br>
>> void LinkerScript::assignAddresses(<wbr>std::vector<PhdrEntry> &Phdrs) {<br>
>> // Assign addresses as instructed by linker script SECTIONS<br>
>> sub-commands.<br>
>> Dot = 0;<br>
>><br>
>> Modified: lld/trunk/ELF/LinkerScript.h<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/lld/trunk/ELF/</a><br>
>> LinkerScript.h?rev=301678&r1=<wbr>301677&r2=301678&view=diff<br>
>> ==============================<wbr>==============================<br>
>> ==================<br>
>> --- lld/trunk/ELF/LinkerScript.h (original)<br>
>> +++ lld/trunk/ELF/LinkerScript.h Fri Apr 28 15:22:47 2017<br>
>> @@ -228,7 +228,6 @@ protected:<br>
>> MemoryRegion *findMemoryRegion(<wbr>OutputSectionCommand *Cmd);<br>
>><br>
>> void switchTo(OutputSection *Sec);<br>
>> - void flush();<br>
>> void output(InputSection *Sec);<br>
>> void process(BaseCommand &Base);<br>
>><br>
>> @@ -242,9 +241,6 @@ protected:<br>
>> OutputSection *CurOutSec = nullptr;<br>
>> MemoryRegion *CurMemRegion = nullptr;<br>
>><br>
>> - llvm::DenseSet<OutputSection *> AlreadyOutputOS;<br>
>> - llvm::DenseSet<<wbr>InputSectionBase *> AlreadyOutputIS;<br>
>> -<br>
>> public:<br>
>> bool hasPhdrsCommands() { return !Opt.PhdrsCommands.empty(); }<br>
>> uint64_t getDot() { return Dot; }<br>
>> @@ -271,6 +267,7 @@ public:<br>
>> void assignOffsets(<wbr>OutputSectionCommand *Cmd);<br>
>> void placeOrphanSections();<br>
>> void processNonSectionCommands();<br>
>> + void synchronize();<br>
>> void assignAddresses(std::vector<<wbr>PhdrEntry> &Phdrs);<br>
>> int getSectionIndex(StringRef Name);<br>
>><br>
>><br>
>> Modified: lld/trunk/ELF/Writer.cpp<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Writer" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/lld/trunk/ELF/Writer</a>.<br>
>> cpp?rev=301678&r1=301677&r2=<wbr>301678&view=diff<br>
>> ==============================<wbr>==============================<br>
>> ==================<br>
>> --- lld/trunk/ELF/Writer.cpp (original)<br>
>> +++ lld/trunk/ELF/Writer.cpp Fri Apr 28 15:22:47 2017<br>
>> @@ -254,6 +254,7 @@ template <class ELFT> void Writer<ELFT>:<br>
>> fixSectionAlignments();<br>
>> Script-><wbr>fabricateDefaultCommands(<wbr>Config->MaxPageSize);<br>
>> }<br>
>> + Script->synchronize();<br>
>> Script->assignAddresses(Phdrs)<wbr>;<br>
>><br>
>> // Remove empty PT_LOAD to avoid causing the dynamic linker to try to<br>
>> mmap a<br>
>> @@ -1080,6 +1081,7 @@ static void removeUnusedSyntheticSection<br>
>><br>
>> SS->OutSec->Sections.erase(<wbr>std::find(SS->OutSec-><wbr>Sections.begin(),<br>
>> SS->OutSec->Sections.end(), SS));<br>
>> + SS->Live = false;<br>
>> // If there are no other sections in the output section, remove it<br>
>> from the<br>
>> // output.<br>
>> if (SS->OutSec->Sections.empty())<br>
>><br>
>><br>
>> ______________________________<wbr>_________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
>><br>
</div></div></blockquote></div><br></div>