[PATCH] D27415: [ELF] - Replace MergeOutputSection with synthetic input section MergeSection.
George Rimar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 6 07:27:25 PST 2016
grimar added inline comments.
================
Comment at: ELF/OutputSections.cpp:139
+
+ // If we add mergable input section we might want to create or
+ // use existing synthetic MergeSection for each input section group.
----------------
grimar wrote:
> ruiu wrote:
> > This is not a right place to add this code. You want to do outside of this function. Instead of hiding this logic inside `addSection`, make a new function that scans `Symtab<ELFT>::X->Sections` vector to merge mergeable input sections into synthetic mergeable sections.
> This can work for non-script case, but how would you handle linkerscript then ?
>
> This patch already relies on fact that script calls addSection() with merge input sections.
> It still creates multiple output sections for different attributes because script has own createKey(),
> but OutputSection::addSection is responsible for merging all what I give it.
>
> I tried to do the suggested change and need to fix script logic too because
> it still passes MergeInputSections.
>
> Script code for creating the list of sections is next:
> ```
> std::vector<InputSectionBase<ELFT> *> V = createInputSectionList(*Cmd);
>
> // The output section name `/DISCARD/' is special.
> // Any input section assigned to it is discarded.
> if (Cmd->Name == "/DISCARD/") {
> discard(V);
> continue;
> }
>
> // This is for ONLY_IF_RO and ONLY_IF_RW. An output section directive
> // ".foo : ONLY_IF_R[OW] { ... }" is handled only if all member input
> // sections satisfy a given constraint. If not, a directive is handled
> // as if it wasn't present from the beginning.
> //
> // Because we'll iterate over Commands many more times, the easiest
> // way to "make it as if it wasn't present" is to just remove it.
> if (!matchConstraints<ELFT>(V, Cmd->Constraint)) {
> for (InputSectionBase<ELFT> *S : V)
> S->Assigned = false;
> Opt.Commands.erase(Iter);
> --I;
> continue;
> }
> ```
>
> So looks after the last block of code I can take V, create synthetic mergeable sections for sections in V if any
> and update V and Symtab<ELFT>::X->Sections.
>
> There is no way to do that once outside like for non-scripted case. And that will complicate the logic.
> Do you think it is correct direction ?
So generally I do not see benefits from doing that outside of OutputSection.
Logic of "scan all sections, create new synthetic sections, remove regular mergable sections from array and do something more on a linker script side" looks more complicated that do everything incapsulated in OutputSection in a short method.
https://reviews.llvm.org/D27415
More information about the llvm-commits
mailing list