<div dir="ltr">This is a bit of a hot take, but I'd rather make aggressive ICF (fold any identical code) the norm and scrap the C++ language guarantee about function pointer identity. I don't participate in C++ standardization, so my opinion doesn't carry much weight, but I lean towards giving the optimizer a free hand here. There are a number of other ways in which function pointer identity breaks down. Consider -fvisibility-inlines=hidden, which if we are being real, is the only way to compile big C++ projects, even those with explicit visibility annotations. Function pointer identity is just fragile and easy to break, and I don't encourage people to rely on it. If you need some kind of globally unique identifier, it's pretty straightforward to use data instead of code.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Mar 23, 2021 at 2:55 PM James Y Knight via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">In general, I don't like the "icf=all" mode. It's not nice to have the linker break semantics of the program, and I think we should not encourage its use. (And, in particular, adding new features just for it seems like a bad idea.) Instead, we should make sure that icf=safe works as well as it possibly can, and encourage its use.<div><br></div><div>It's not possible to conclude anything based only on the size results, but it does indicate that some further investigation may be a good idea, to look into <i>why</i> icf=safe is apparently less-effective on the windows binary vs the linux one.</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Mar 23, 2021 at 2:50 PM Fangrui Song via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
On 2021-03-23, Zequan Wu wrote:<br>
>The size increase of chrome on Linux by switching from all ICF to safe ICF<br>
>is small.<br>
>All ICF:<br>
> text data bss dec hex filename<br>
>169314343 8472660 2368965 180155968 abcf640 chrome<br>
>Safe ICF:<br>
> text data bss dec hex filename<br>
>174521550 8497604 2368965 185388119 b0ccc57 chrome<br>
><br>
>On Windows, chrome.dll increases size by around 14 MB (12MB increases in<br>
>.text section).<br>
>All ICF:<br>
>Size of out\Default\chrome.dll is 170.715648 MB<br>
> name: mem size , disk size<br>
> .text: 141.701417 MB<br>
> .rdata: 22.458476 MB<br>
> .data: 3.093948 MB, 0.523264 MB<br>
> .pdata: 4.412364 MB<br>
> .00cfg: 0.000040 MB<br>
> .gehcont: 0.000132 MB<br>
> .retplne: 0.000108 MB<br>
> .rodata: 0.004544 MB<br>
> .tls: 0.000561 MB<br>
> CPADinfo: 0.000056 MB<br>
> _RDATA: 0.000244 MB<br>
> .rsrc: 0.285232 MB<br>
> .reloc: 1.324196 MB<br>
>Safe ICF:<br>
>Size of out\icf-safe\chrome.dll is 184.499712 MB<br>
> name: mem size , disk size<br>
> .text: 153.809529 MB<br>
> .rdata: 23.123628 MB<br>
> .data: 3.093948 MB, 0.523264 MB<br>
> .pdata: 5.367396 MB<br>
> .00cfg: 0.000040 MB<br>
> .gehcont: 0.000132 MB<br>
> .retplne: 0.000108 MB<br>
> .rodata: 0.004544 MB<br>
> .tls: 0.000561 MB<br>
> CPADinfo: 0.000056 MB<br>
> _RDATA: 0.000244 MB<br>
> .rsrc: 0.285232 MB<br>
> .reloc: 1.379364 MB<br>
><br>
>If an attribute is used and it affects unnamed_addr of a symbol, it<br>
>determines whether the symbols should show up in the .addrsig table.<br>
>All-ICF mode in ld.lld and lld-link ignore symbols in the .addrsig table,<br>
>if they belong to code sections. So, it won't have an effect on disabling<br>
>ICF.<br>
<br>
Is there something which can be improved on lld-link? Note also that<br>
the size difference between gold --icf={safe,all} is smaller than the<br>
difference between ld.lld --icf={safe,all}. Some may be due to gold<br>
performing "unsafe" safe ICF, some may suggest places where clang fails<br>
to add {,local_}unnamed_addr.<br>
<br>
<br>
>On Mon, Mar 22, 2021 at 10:19 PM Fangrui Song <<a href="mailto:maskray@google.com" target="_blank">maskray@google.com</a>> wrote:<br>
><br>
>><br>
>> On 2021-03-22, David Blaikie via llvm-dev wrote:<br>
>> >ICF: Identical Code Folding<br>
>> ><br>
>> >Linker deduplicates functions by collapsing any identical functions<br>
>> >together - with icf=safe, the linker looks at a .addressing section in the<br>
>> >object file and any functions listed in that section are not treated as<br>
>> >collapsible (eg: because they need to meet C++'s "distinct functions have<br>
>> >distinct addresses" guarantee)<br>
>><br>
>> The name originated from MSVC link.exe where icf stands for "identical<br>
>> COMDAT folding".<br>
>> gold named it "identical code folding" - which makes some sense because<br>
>> gold does not fold readonly data.<br>
>><br>
>> In LLD, the name is not accurate for two reasons: (1) the feature can<br>
>> apply to readonly data as well; (2) the folding is by section, not by<br>
>> function.<br>
>><br>
>> We define identical sections as they have identical content and their<br>
>> outgoing relocation sets cannot be distinguished: they need to have the<br>
>> same number of relocations, with the same relative locations, with the<br>
>> referenced symbols indistinguishable.<br>
>><br>
>> Then, ld.lld --icf={safe,all} works like this:<br>
>><br>
>> For a set of identical sections, the linker picks one representative and<br>
>> drops the rest, then redirects references to the representative.<br>
>><br>
>> Note: this can confuse debuggers/symbolizers/profilers easily.<br>
>><br>
>> lld-link /opt:icf is different from ld.lld --icf but I haven't looked<br>
>> into it closely.<br>
>><br>
>><br>
>> I find that the feature's saving is small given its downside<br>
>> (also increaded link time: the current LLD's implementation is inferior:<br>
>> it performs a quadratic number of comparisons among an equality class):<br>
>><br>
>> This is the size differences for the 'lld' executable:<br>
>><br>
>> % size lld.{none,safe,all}<br>
>> text data bss dec hex filename<br>
>> 96821040 7210504 550810 104582354 63bccd2 lld.none<br>
>> 95217624 7167656 550810 102936090 622ae1a lld.safe<br>
>> 94038808 7167144 550810 101756762 610af5a lld.all<br>
>> % size gold.{none,safe,all}<br>
>> text data bss dec hex filename<br>
>> 96857302 7174792 550825 104582919 63bcf07 gold.none<br>
>> 94469390 7174792 550825 102195007 6175f3f gold.safe<br>
>> 94184430 7174792 550825 101910047 613061f gold.all<br>
>><br>
>> Note that the --icf=all result caps the potential saving of the proposed<br>
>> annotation.<br>
>><br>
>> Actually with some large internal targets I get even smaller savings.<br>
>><br>
>><br>
>> ld.lld --icf=safe is safer than gold --icf=safe but probably misses some<br>
>> opportunities.<br>
>> It can be that clang codegen/optimizer fail to mark some cases as<br>
>> {,local_}unnamed_addr.<br>
>><br>
>> I know Chromium and the Windows world can be different:) But I'd still<br>
>> want to<br>
>> get some numbers first.<br>
>><br>
>><br>
>> Last, I have seen that Chromium has some code like<br>
>><br>
>> <a href="https://source.chromium.org/chromium/chromium/src/+/master:skia/ext/SkMemory_new_handler.cpp" rel="noreferrer" target="_blank">https://source.chromium.org/chromium/chromium/src/+/master:skia/ext/SkMemory_new_handler.cpp</a><br>
>><br>
>> void sk_abort_no_print() {<br>
>> // Linker's ICF feature may merge this function with other<br>
>> functions with<br>
>> // the same definition (e.g. any function whose sole job is to call<br>
>> abort())<br>
>> // and it may confuse the crash report processing system.<br>
>> // <a href="http://crbug.com/860850" rel="noreferrer" target="_blank">http://crbug.com/860850</a><br>
>> static int static_variable_to_make_this_function_unique = 0x736b;<br>
>> // "sk"<br>
>> base::debug::Alias(&static_variable_to_make_this_function_unique);<br>
>><br>
>> abort();<br>
>> }<br>
>><br>
>> If we want an approach to work with link.exe, I don't know what we can<br>
>> do...<br>
>> If no desire for link.exe compatibility, I can see that having a proper<br>
>> way marking the function<br>
>> can be useful... but in any case if an attribute is used, it probably<br>
>> should affect<br>
>> unnamed_addr directly instead of being called *icf*.<br>
>><br>
>><br>
>><br>
>> >On Mon, Mar 22, 2021 at 6:16 PM Philip Reames via llvm-dev <<br>
>> ><a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
>> ><br>
>> >> Can you define ICF please? And give a bit of context?<br>
>> >><br>
>> >> Philip<br>
>> >> On 3/22/21 5:27 PM, Zequan Wu via llvm-dev wrote:<br>
>> >><br>
>> >> Hi all,<br>
>> >><br>
>> >> Background:<br>
>> >> It's been a longstanding difficulty of debugging with ICF. Programmers<br>
>> >> don't have control over which sections should be folded by ICF, which<br>
>> >> sections shouldn't. The existing address significant table won't have<br>
>> >> effect for code sections during all ICF mode in both ld.lld and<br>
>> lld-link.<br>
>> >> By switching to safe ICF could mark code sections as unique, but at a<br>
>> cost<br>
>> >> of increasing binary size out of control. So, it would be good if<br>
>> >> programmers could selectively disable ICF in source code by annotating<br>
>> >> global functions/variables with an attribute to improve debugging<br>
>> >> experience and have the control on the binary size increase.<br>
>> >><br>
>> >> My plan is to add a new section table(`.no_icf`) to object files.<br>
>> Sections<br>
>> >> of all symbols inside the table should not be folded by all ICF mode.<br>
>> And<br>
>> >> symbols can only be added into the table by annotating global<br>
>> >> functions/variables with a new attribute(`no_icf`) in source code.<br>
>> >><br>
>> >> What do you think about this approach?<br>
>> >><br>
>> >> Thanks,<br>
>> >> Zequan<br>
>> >><br>
>> >><br>
>> >> _______________________________________________<br>
>> >> LLVM Developers mailing listllvm-dev@lists.llvm.orghttps://<br>
>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
>> >><br>
>> >> _______________________________________________<br>
>> >> LLVM Developers mailing list<br>
>> >> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
>> >> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
>> >><br>
>><br>
>> >_______________________________________________<br>
>> >LLVM Developers mailing list<br>
>> ><a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
>> ><a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
>><br>
>><br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div>