<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">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:0 0 0 .8ex;border-left:1px #ccc 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><div>ICF is on by default for COFF, so if something doesn't work with ICF, it doesn't work for COFF.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc 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><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><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc 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><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>