[llvm-dev] [RFC] Annotating global functions and variables to prevent ICF during linking
Zequan Wu via llvm-dev
llvm-dev at lists.llvm.org
Thu Apr 8 21:19:57 PDT 2021
Hi James,
Thanks for the feedback.
1. There exists some hacking in chromium code base to make functions
distinct. That's why chromium developers have requested for this feature in
compiler to avoid this sort of hacking.
2. I found that there are cases unwanted symbols show up in the
addrsig table. I'm looking at this problem.
3. This sounds like a better approach to me. We can do it like using a new
attribute to annotate functions that should show up in addrsig table and
use safe icf mode.
I'll try investigating unwanted symbols in the addrsig table before the
last approach.
On Wed, Apr 7, 2021 at 12:26 PM James Y Knight <jyknight at google.com> wrote:
> Three things:
>
> 1. if chrome already has a mechanism/hack to make functions distinct in
> the face of icf=all, do we really need to add a compiler feature?
>
> 2. Are we sure functions aren't ending up in the addrsig table spuriously
> (and that the addrsig table isn't getting lost)? IOW: are the additional
> functions that get emitted with icf=safe really needed (as far as
> reasonable compiler analysis can determine), or should/could they have been
> removed?
>
> 3. If we _do_ need to implement something here (which I'm skeptical of) it
> would be better to add a compiler flag which adjusts how the contents of
> the addrsig table are computed, vs adding a new linker mechanism.
>
> You'd continue to link with icf=safe, but the compiler option would allow
> more functions to be merged, by putting fewer into addrsig.
>
> This would also allow the "unsafe" semantics to be per-TU instead of
> global for the entire link, allowing you not to break other libraries that
> may be linked into the same binary.
>
>
> On Wed, Apr 7, 2021, 1:32 PM Reid Kleckner <rnk at google.com> wrote:
>
>> Thanks for gathering the data, Zequan. For more context, I'll add that
>> chrome.dll is around 163MB, so saving 6MB is around 3.5% of binary size. I
>> feel like this is a compelling use case for aggressive ICF.
>>
>> Chrome has historically used MSVC ICF, which is aggressive, and it can
>> make debugging challenging. We're looking for a way that we can selectively
>> power down ICF for functions that developers are interested in. I think it
>> would be reasonable to have a compiler flag that emits an extra section
>> with the same format as llvm_addrsig, with the semantics that it blocks ICF
>> even in the aggressive mode.
>>
>> For the user interface, we can either invent a new attribute, (noicf,
>> blockicf, disable_icf), document that it only works with linkers that
>> support the new section (LLD), and go that way. Alternatively we can
>> overload an existing attribute like __attribute__((used)). I think I favor
>> the new attribute. It makes it easier to search and find the relevant
>> documentation.
>>
>> Does that seem reasonable?
>>
>> On Wed, Apr 7, 2021 at 10:06 AM Zequan Wu <zequanwu at google.com> wrote:
>>
>>> I got some data for the size increase from safe icf to all icf. The 14
>>> MB I initially found was due to .addrsig not emitted on COFF thinlto.
>>> After enabling it, the size of chrome.dll is increased by 6 MB when
>>> switching from all icf to safe icf on windows and 5 MB increase on linux.
>>>
>>> Here is a breakdown of chrome.dll section size increase on Windows.
>>> 5574656 bytes increase in .text
>>> 643072 bytes increase in .rdata
>>> 573440 bytes increase in .pdata
>>> 57344 bytes increase in .reloc
>>> 407152 bytes increase in rvatable
>>>
>>> I dumped the sum of section size removed and selected/removed sections
>>> count by icf in ELF and COFF as below.
>>> all-icf.win.txt:
>>> total size removed: 10763454 selected: 19712 removed: 153544
>>> safe-icf.win.txt:
>>> total size removed: 6122783 selected: 8653 removed: 48999
>>> all-icf.linux.txt:
>>> total size removed: 10423699 selected: 22254 removed: 158806
>>> safe-icf.linux.txt:
>>> total size removed: 6165470 selected: 9313 removed: 53981
>>>
>>> There is not much difference. So, safe icf is performing the same on
>>> linux and windows. I think we have enough motivation to add a new section
>>> for disabling icf.
>>>
>>> On Thu, Mar 25, 2021 at 10:57 AM Reid Kleckner <rnk at google.com> wrote:
>>>
>>>> I should add, though, that I agree with the final direction here: if
>>>> the cost of safe ICF is low, it is much better to just use it instead of
>>>> adding a whole new feature requiring documentation, etc. We need new
>>>> measurements to show if the feature is justified.
>>>>
>>>> On Thu, Mar 25, 2021 at 10:55 AM Reid Kleckner <rnk at google.com> wrote:
>>>>
>>>>> 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.
>>>>>
>>>>> On Tue, Mar 23, 2021 at 2:55 PM James Y Knight via llvm-dev <
>>>>> llvm-dev at lists.llvm.org> wrote:
>>>>>
>>>>>> 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.
>>>>>>
>>>>>> 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 *why* icf=safe is apparently less-effective on
>>>>>> the windows binary vs the linux one.
>>>>>>
>>>>>>
>>>>>> On Tue, Mar 23, 2021 at 2:50 PM Fangrui Song via llvm-dev <
>>>>>> llvm-dev at lists.llvm.org> wrote:
>>>>>>
>>>>>>>
>>>>>>> On 2021-03-23, Zequan Wu wrote:
>>>>>>> >The size increase of chrome on Linux by switching from all ICF to
>>>>>>> safe ICF
>>>>>>> >is small.
>>>>>>> >All ICF:
>>>>>>> > text data bss dec hex filename
>>>>>>> >169314343 8472660 2368965 180155968 abcf640 chrome
>>>>>>> >Safe ICF:
>>>>>>> > text data bss dec hex filename
>>>>>>> >174521550 8497604 2368965 185388119 b0ccc57 chrome
>>>>>>> >
>>>>>>> >On Windows, chrome.dll increases size by around 14 MB (12MB
>>>>>>> increases in
>>>>>>> >.text section).
>>>>>>> >All ICF:
>>>>>>> >Size of out\Default\chrome.dll is 170.715648 MB
>>>>>>> > name: mem size , disk size
>>>>>>> > .text: 141.701417 MB
>>>>>>> > .rdata: 22.458476 MB
>>>>>>> > .data: 3.093948 MB, 0.523264 MB
>>>>>>> > .pdata: 4.412364 MB
>>>>>>> > .00cfg: 0.000040 MB
>>>>>>> > .gehcont: 0.000132 MB
>>>>>>> > .retplne: 0.000108 MB
>>>>>>> > .rodata: 0.004544 MB
>>>>>>> > .tls: 0.000561 MB
>>>>>>> > CPADinfo: 0.000056 MB
>>>>>>> > _RDATA: 0.000244 MB
>>>>>>> > .rsrc: 0.285232 MB
>>>>>>> > .reloc: 1.324196 MB
>>>>>>> >Safe ICF:
>>>>>>> >Size of out\icf-safe\chrome.dll is 184.499712 MB
>>>>>>> > name: mem size , disk size
>>>>>>> > .text: 153.809529 MB
>>>>>>> > .rdata: 23.123628 MB
>>>>>>> > .data: 3.093948 MB, 0.523264 MB
>>>>>>> > .pdata: 5.367396 MB
>>>>>>> > .00cfg: 0.000040 MB
>>>>>>> > .gehcont: 0.000132 MB
>>>>>>> > .retplne: 0.000108 MB
>>>>>>> > .rodata: 0.004544 MB
>>>>>>> > .tls: 0.000561 MB
>>>>>>> > CPADinfo: 0.000056 MB
>>>>>>> > _RDATA: 0.000244 MB
>>>>>>> > .rsrc: 0.285232 MB
>>>>>>> > .reloc: 1.379364 MB
>>>>>>> >
>>>>>>> >If an attribute is used and it affects unnamed_addr of a symbol, it
>>>>>>> >determines whether the symbols should show up in the .addrsig table.
>>>>>>> >All-ICF mode in ld.lld and lld-link ignore symbols in the .addrsig
>>>>>>> table,
>>>>>>> >if they belong to code sections. So, it won't have an effect on
>>>>>>> disabling
>>>>>>> >ICF.
>>>>>>>
>>>>>>> Is there something which can be improved on lld-link? Note also that
>>>>>>> the size difference between gold --icf={safe,all} is smaller than the
>>>>>>> difference between ld.lld --icf={safe,all}. Some may be due to gold
>>>>>>> performing "unsafe" safe ICF, some may suggest places where clang
>>>>>>> fails
>>>>>>> to add {,local_}unnamed_addr.
>>>>>>>
>>>>>>>
>>>>>>> >On Mon, Mar 22, 2021 at 10:19 PM Fangrui Song <maskray at google.com>
>>>>>>> wrote:
>>>>>>> >
>>>>>>> >>
>>>>>>> >> On 2021-03-22, David Blaikie via llvm-dev wrote:
>>>>>>> >> >ICF: Identical Code Folding
>>>>>>> >> >
>>>>>>> >> >Linker deduplicates functions by collapsing any identical
>>>>>>> functions
>>>>>>> >> >together - with icf=safe, the linker looks at a .addressing
>>>>>>> section in the
>>>>>>> >> >object file and any functions listed in that section are not
>>>>>>> treated as
>>>>>>> >> >collapsible (eg: because they need to meet C++'s "distinct
>>>>>>> functions have
>>>>>>> >> >distinct addresses" guarantee)
>>>>>>> >>
>>>>>>> >> The name originated from MSVC link.exe where icf stands for
>>>>>>> "identical
>>>>>>> >> COMDAT folding".
>>>>>>> >> gold named it "identical code folding" - which makes some sense
>>>>>>> because
>>>>>>> >> gold does not fold readonly data.
>>>>>>> >>
>>>>>>> >> In LLD, the name is not accurate for two reasons: (1) the feature
>>>>>>> can
>>>>>>> >> apply to readonly data as well; (2) the folding is by section,
>>>>>>> not by
>>>>>>> >> function.
>>>>>>> >>
>>>>>>> >> We define identical sections as they have identical content and
>>>>>>> their
>>>>>>> >> outgoing relocation sets cannot be distinguished: they need to
>>>>>>> have the
>>>>>>> >> same number of relocations, with the same relative locations,
>>>>>>> with the
>>>>>>> >> referenced symbols indistinguishable.
>>>>>>> >>
>>>>>>> >> Then, ld.lld --icf={safe,all} works like this:
>>>>>>> >>
>>>>>>> >> For a set of identical sections, the linker picks one
>>>>>>> representative and
>>>>>>> >> drops the rest, then redirects references to the representative.
>>>>>>> >>
>>>>>>> >> Note: this can confuse debuggers/symbolizers/profilers easily.
>>>>>>> >>
>>>>>>> >> lld-link /opt:icf is different from ld.lld --icf but I haven't
>>>>>>> looked
>>>>>>> >> into it closely.
>>>>>>> >>
>>>>>>> >>
>>>>>>> >> I find that the feature's saving is small given its downside
>>>>>>> >> (also increaded link time: the current LLD's implementation is
>>>>>>> inferior:
>>>>>>> >> it performs a quadratic number of comparisons among an equality
>>>>>>> class):
>>>>>>> >>
>>>>>>> >> This is the size differences for the 'lld' executable:
>>>>>>> >>
>>>>>>> >> % size lld.{none,safe,all}
>>>>>>> >> text data bss dec hex filename
>>>>>>> >> 96821040 7210504 550810 104582354 63bccd2 lld.none
>>>>>>> >> 95217624 7167656 550810 102936090 622ae1a lld.safe
>>>>>>> >> 94038808 7167144 550810 101756762 610af5a lld.all
>>>>>>> >> % size gold.{none,safe,all}
>>>>>>> >> text data bss dec hex filename
>>>>>>> >> 96857302 7174792 550825 104582919 63bcf07 gold.none
>>>>>>> >> 94469390 7174792 550825 102195007 6175f3f gold.safe
>>>>>>> >> 94184430 7174792 550825 101910047 613061f gold.all
>>>>>>> >>
>>>>>>> >> Note that the --icf=all result caps the potential saving of the
>>>>>>> proposed
>>>>>>> >> annotation.
>>>>>>> >>
>>>>>>> >> Actually with some large internal targets I get even smaller
>>>>>>> savings.
>>>>>>> >>
>>>>>>> >>
>>>>>>> >> ld.lld --icf=safe is safer than gold --icf=safe but probably
>>>>>>> misses some
>>>>>>> >> opportunities.
>>>>>>> >> It can be that clang codegen/optimizer fail to mark some cases as
>>>>>>> >> {,local_}unnamed_addr.
>>>>>>> >>
>>>>>>> >> I know Chromium and the Windows world can be different:) But I'd
>>>>>>> still
>>>>>>> >> want to
>>>>>>> >> get some numbers first.
>>>>>>> >>
>>>>>>> >>
>>>>>>> >> Last, I have seen that Chromium has some code like
>>>>>>> >>
>>>>>>> >>
>>>>>>> https://source.chromium.org/chromium/chromium/src/+/master:skia/ext/SkMemory_new_handler.cpp
>>>>>>> >>
>>>>>>> >> void sk_abort_no_print() {
>>>>>>> >> // Linker's ICF feature may merge this function with other
>>>>>>> >> functions with
>>>>>>> >> // the same definition (e.g. any function whose sole job
>>>>>>> is to call
>>>>>>> >> abort())
>>>>>>> >> // and it may confuse the crash report processing system.
>>>>>>> >> // http://crbug.com/860850
>>>>>>> >> static int static_variable_to_make_this_function_unique =
>>>>>>> 0x736b;
>>>>>>> >> // "sk"
>>>>>>> >>
>>>>>>> base::debug::Alias(&static_variable_to_make_this_function_unique);
>>>>>>> >>
>>>>>>> >> abort();
>>>>>>> >> }
>>>>>>> >>
>>>>>>> >> If we want an approach to work with link.exe, I don't know what
>>>>>>> we can
>>>>>>> >> do...
>>>>>>> >> If no desire for link.exe compatibility, I can see that having a
>>>>>>> proper
>>>>>>> >> way marking the function
>>>>>>> >> can be useful... but in any case if an attribute is used, it
>>>>>>> probably
>>>>>>> >> should affect
>>>>>>> >> unnamed_addr directly instead of being called *icf*.
>>>>>>> >>
>>>>>>> >>
>>>>>>> >>
>>>>>>> >> >On Mon, Mar 22, 2021 at 6:16 PM Philip Reames via llvm-dev <
>>>>>>> >> >llvm-dev at lists.llvm.org> wrote:
>>>>>>> >> >
>>>>>>> >> >> Can you define ICF please? And give a bit of context?
>>>>>>> >> >>
>>>>>>> >> >> Philip
>>>>>>> >> >> On 3/22/21 5:27 PM, Zequan Wu via llvm-dev wrote:
>>>>>>> >> >>
>>>>>>> >> >> Hi all,
>>>>>>> >> >>
>>>>>>> >> >> Background:
>>>>>>> >> >> It's been a longstanding difficulty of debugging with ICF.
>>>>>>> Programmers
>>>>>>> >> >> don't have control over which sections should be folded by
>>>>>>> ICF, which
>>>>>>> >> >> sections shouldn't. The existing address significant table
>>>>>>> won't have
>>>>>>> >> >> effect for code sections during all ICF mode in both ld.lld and
>>>>>>> >> lld-link.
>>>>>>> >> >> By switching to safe ICF could mark code sections as unique,
>>>>>>> but at a
>>>>>>> >> cost
>>>>>>> >> >> of increasing binary size out of control. So, it would be good
>>>>>>> if
>>>>>>> >> >> programmers could selectively disable ICF in source code by
>>>>>>> annotating
>>>>>>> >> >> global functions/variables with an attribute to improve
>>>>>>> debugging
>>>>>>> >> >> experience and have the control on the binary size increase.
>>>>>>> >> >>
>>>>>>> >> >> My plan is to add a new section table(`.no_icf`) to object
>>>>>>> files.
>>>>>>> >> Sections
>>>>>>> >> >> of all symbols inside the table should not be folded by all
>>>>>>> ICF mode.
>>>>>>> >> And
>>>>>>> >> >> symbols can only be added into the table by annotating global
>>>>>>> >> >> functions/variables with a new attribute(`no_icf`) in source
>>>>>>> code.
>>>>>>> >> >>
>>>>>>> >> >> What do you think about this approach?
>>>>>>> >> >>
>>>>>>> >> >> Thanks,
>>>>>>> >> >> Zequan
>>>>>>> >> >>
>>>>>>> >> >>
>>>>>>> >> >> _______________________________________________
>>>>>>> >> >> LLVM Developers mailing listllvm-dev at lists.llvm.orghttps://
>>>>>>> >> lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>>>> >> >>
>>>>>>> >> >> _______________________________________________
>>>>>>> >> >> LLVM Developers mailing list
>>>>>>> >> >> llvm-dev at lists.llvm.org
>>>>>>> >> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>>>> >> >>
>>>>>>> >>
>>>>>>> >> >_______________________________________________
>>>>>>> >> >LLVM Developers mailing list
>>>>>>> >> >llvm-dev at lists.llvm.org
>>>>>>> >> >https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>>>> >>
>>>>>>> >>
>>>>>>> _______________________________________________
>>>>>>> LLVM Developers mailing list
>>>>>>> llvm-dev at lists.llvm.org
>>>>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>>>>
>>>>>> _______________________________________________
>>>>>> LLVM Developers mailing list
>>>>>> llvm-dev at lists.llvm.org
>>>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>>>
>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210408/ec28bcf2/attachment-0001.html>
More information about the llvm-dev
mailing list