<div dir="ltr">Rafael,<div><br></div><div>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?</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Apr 28, 2017 at 1:22 PM, Rafael Espindola via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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/LinkerScript.cpp?rev=301678&r1=301677&r2=301678&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/lld/trunk/ELF/<wbr>LinkerScript.cpp?rev=301678&<wbr>r1=301677&r2=301678&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<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-><wbr>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 !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 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 changes would<br>
+// be done directly in the "AST".<br>
+//<br>
+// This can only handle new sections being added and sections being 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>
+ 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 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/LinkerScript.h?rev=301678&r1=301677&r2=301678&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/lld/trunk/ELF/<wbr>LinkerScript.h?rev=301678&r1=<wbr>301677&r2=301678&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<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.cpp?rev=301678&r1=301677&r2=301678&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/lld/trunk/ELF/Writer.<wbr>cpp?rev=301678&r1=301677&r2=<wbr>301678&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<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 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 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>
</blockquote></div><br></div>