<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Feb 22, 2016 at 5:39 PM, Rui Ueyama <span dir="ltr"><<a href="mailto:ruiu@google.com" target="_blank">ruiu@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="">On Mon, Feb 22, 2016 at 5:32 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">silvas added a subscriber: silvas.<br>
silvas added a comment.<br>
<span><br>
> The lacking feature is "safe" version of ICF. This merges all<br>
<br>
>  identical sections. That is not compatible with C/C++ language<br>
<br>
>  requirement that two distinct functions must not have the address.<br>
<br>
>  I think violation this rule breaks many C++ ABIs.<br>
<br>
<br>
</span>How does the COFF ICF avoid this problem?<br></blockquote><div><br></div></span><div>ICF is on by default for COFF, so if something doesn't work with ICF, it doesn't work for COFF.</div></div></div></div></blockquote><div><br></div><div>This hasn't been an issue in practice? Why do you think it will be an issue with ELF?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class=""><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<span><br>
> We have a different idea<br>
<br>
>  which David Majnemer suggested, which is to add NOPs at beginning<br>
<br>
>  of merged functions so that two or more pointers can have distinct<br>
<br>
>  values.<br>
<br>
<br>
</span>If we do this, we need to be careful to not misalign the function as emitted by the compiler. E.g. even if we only need to insert a single nop before the current section contents, we still want to grow the section by the section's alignment.<br>
<br>
<br>
================<br>
Comment at: ELF/ICF.cpp:16<br>
@@ +15,3 @@<br>
+//<br>
+// ICF is theoretically a problem of reducing graphs by merging as many<br>
+// identical subgraphs as possible if we consider sections as vertices and<br>
----------------<br>
Don't you use a partition-refinement approach instead of a graph approach? Why mention graphs?<br></blockquote><div><br></div></span><div>Well, this algorithm is a partition-refinement approach and at the same time works on a graph. They are just different two views of the same algorithm. Is it still confusing after you read through all this comments?</div></div></div></div></blockquote><div><br></div><div>I thought the main insight of the partition refinement approach was that describing the problem as "reducing graphs by merging as many identical subgraphs as possible if we consider sections as vertices and relocations as edges" is fundamentally wrong and fails to merge certain things that can be merged? The partition refinement approach fundamentally does not operate in terms of merging identical subgraphs of the original graph. It is sort of confusing to talk about it in terms of graphs.</div><div><br></div><div>I would describe it as:</div><div>"We begin by optimistically putting all sections into a single equivalence class. Then we apply a series of checks that split this initial equivalence class into more and more refined equivalence classes based on the properties by which a section can be distinguished. We begin by checking that the section contents and flags are the same. This only needs to be done once since these properties don't depend on the current equivalence class assignment. Then we split the equivalence classes based on checking that their relocations are the same, where relocation targets are compared by their equivalence class, not the concrete section. This may need to be done multiple times because as the equivalence classes are refined, two sections that had a relocation target in the same equivalence class may now target different equivalence classes, and hence these two sections must be put in different equivalence classes (whereas in the previous iteration they were not since the relocation target was the same)."</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class=""><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
================<br>
Comment at: ELF/InputSection.h:182<br>
@@ -170,1 +181,3 @@<br>
+  // Used by ICF.<br>
+  uint64_t GroupId = 0;<br>
 };<br>
----------------<br>
I know we aren't currently optimizing the sizeof of InputSection, but we should be aware of it since we potentially allocate many.<br>
Out of curiosity, have you measured if the extra memory pressure of this GroupId causes any performance change?<br>
It it has an adverse effect, we can always merge it with OutSecOff, since I don't think we need both simultaneously. (or we can make sure that Repl always points to a canonical group member).<br></blockquote><div><br></div></span><div>I haven't measured. If you are interested, I can measure, but I think it wouldn't make measurable difference. I measured the performance penalty of a change that increases SymbolBody's size by 8 bytes, and I didn't find no measurable difference. (And # of symbols > # of sections.)</div></div></div></div></blockquote><div><br></div><div>Were you compiling with -ffunction-sections -fdata-sections? I think it is worth measuring for the -ffunction-sections -fdata-sections case.</div><div><br></div><div>-- Sean Silva</div><div> </div></div><br></div></div>