[llvm-dev] ThinLTO and linkonce_odr + unnamed_addr

Peter Collingbourne via llvm-dev llvm-dev at lists.llvm.org
Fri Feb 9 10:50:56 PST 2018 

I wonder whether we could change the frontend to always give hidden
visibility to globals which are linkonce_odr unnamed_addr. That way, the
mechanism would just work in ELF as well and would not require LTO.

Peter

On Fri, Feb 9, 2018 at 10:46 AM, Steven Wu via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> The one that doesn’t require global analysis is commit in r324757.
>
> The global analysis will require adding a field into GlobalSummary. Let me
> do some research. The implementation should not be hard but I don’t want to
> break bitcode compatibility if that applies to the summary.
>
> Steven
>
>
> On Feb 9, 2018, at 9:33 AM, Teresa Johnson <tejohnson at google.com> wrote:
>
> SGTM. I would do as 2 separate patches, the local per-module case, then
> the case that requires linker/global analysis.
> Thanks,
> Teresa
>
> On Fri, Feb 9, 2018 at 9:20 AM, Steven Wu via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> Are we agreeing on this is what we should do? If so, I will prepare a
>> patch.
>>
>> Steven
>>
>>
>> On Feb 8, 2018, at 11:07 AM, Mehdi AMINI <joker.eph at gmail.com> wrote:
>>
>>
>>
>> 2018-02-08 10:44 GMT-08:00 Steven Wu <stevenwu at apple.com>:
>>
>>>
>>>
>>> On Feb 8, 2018, at 10:28 AM, Mehdi AMINI <joker.eph at gmail.com> wrote:
>>>
>>>
>>>
>>> 2018-02-08 9:33 GMT-08:00 Steven Wu <stevenwu at apple.com>:
>>>
>>>>
>>>>
>>>> On Feb 7, 2018, at 4:03 PM, Mehdi AMINI <joker.eph at gmail.com> wrote:
>>>>
>>>>
>>>>
>>>> 2018-02-07 12:45 GMT-08:00 Steven Wu <stevenwu at apple.com>:
>>>>
>>>>>
>>>>>
>>>>> On Feb 7, 2018, at 12:36 PM, Mehdi AMINI <joker.eph at gmail.com> wrote:
>>>>>
>>>>> > But it is interesting in general because according to the
>>>>> definition for local_unnamed_addr, the symbol has to be linkonce_odr to be
>>>>> auto hide as well. ThinLTO promotion can break that as well.
>>>>>
>>>>> What do you mean that the promotion can break that?
>>>>>
>>>>> The original description I found here: https://reviews.llvm.org/D20348
>>>>>  says that it is possible to exclude a global from the symbol table
>>>>> if three things are true:
>>>>>
>>>>>    - This attribute is present on every instance of the global (which
>>>>>    means that the normal rule that the global must have a unique address can
>>>>>    be broken without being observable by the program by performing comparisons
>>>>>    against the global's address)
>>>>>    - The global has linkonce_odr linkage (which means that each
>>>>>    linkage unit must have its own copy of the global if it requires one, and
>>>>>    the copy in each linkage unit must be the same)
>>>>>    - It is a constant or a function (which means that the program
>>>>>    cannot observe that the unique-address rule has been broken by writing to
>>>>>    the global)
>>>>>
>>>>>
>>>>> When promoting from a linkonce_odr, it seems safe to me to *keep* the local_unnamed_addr
>>>>> on the weak_odr since we know the symbol was linkonce_odr in the first
>>>>> place.
>>>>>
>>>>>
>>>>> I don't think there is such guarantees. The description doesn't
>>>>> prevent local_unnamed_addr on other linkage types. If your assumption is
>>>>> correct, then the description can simply state "symbols with
>>>>> local_unnamed_addr can be hidden from the symbol table, regardless of the
>>>>> linkage type). I guess I will leave pcc to interpret this.
>>>>>
>>>>> If my interpretation is correct, then a constant with linkonce_odr +
>>>>> local_unnamed_addr can be hidden from symbol table before promotion. After
>>>>> promotion, it no longer satisfies rule 2, so it has be in the symbol table.
>>>>>
>>>>
>>>> I don't see what would justify this, but I can miss some subtleties
>>>> here.
>>>> If we can't do this with promotion, then it would be very unfortunate:
>>>> the whole point of these was to allow to "auto-hide".
>>>>
>>>>
>>>> I think thinLTO should handle unnamed_addr and generate auto hide if
>>>> needed. We can put (local_)unnamed_addr into GlobalSummary and teach
>>>> thinLTO to add visibility hidden for symbols that satisfies the condition.
>>>> I don't think this is very hard to do.
>>>>
>>>> I don't know if we have a definition for unnamed_addr. Do we treat it
>>>> local_unnamed_addr that automatically satisfy condition #1? Then promote
>>>> linkonce_odr + unnamed_addr into weak_odr + unnamed_addr + hidden is a
>>>> correct transform then.
>>>>
>>>
>>> > promote linkonce_odr + unnamed_addr into weak_odr + unnamed_addr +
>>> hidden is a correct transform then.
>>>
>>> I believe it is only possible to hide if the symbol isn't required to be
>>> preserved by the linker.
>>> But in this case we should always be able to hide regardless of the
>>> linkage don't we?
>>>
>>>
>>> I might not understand your question completely.
>>>
>>> This is the current linker semantics for auto hiding:
>>> * linkonce_odr + unnamed_addr = auto hide (not in symbol table)
>>> * linkonce_odr + local_unnamed_addr in all modules + constant/function =
>>> auto hide (not in symbol table)
>>> * hidden visibility (internal to DSOs but in symbol table)
>>> These results of these 3 conditions should be the same during runtime.
>>> So it seems to me that if we want to promote linkonce_odr to other linkage
>>> type, we should at least add hidden if they were able to be auto hide. For
>>> linkonce_odr unnamed_addr, it can be done locally within the module when
>>> getting promoted. For linkonce_odr local_unnamed_addr, it needs help from
>>> linker to do it correctly.
>>>
>>>
>> I think you're right.
>>
>> --
>> Mehdi
>>
>>
>>
>>
>>> Steven
>>>
>>>
>>> --
>>> Mehdi
>>>
>>>
>>>
>>>
>>>>
>>>> Steven
>>>>
>>>>
>>>> --
>>>> Mehdi
>>>>
>>>>
>>>>
>>>>>
>>>>> --
>>>>> Mehdi
>>>>>
>>>>> 2018-02-07 12:12 GMT-08:00 Steven Wu <stevenwu at apple.com>:
>>>>>
>>>>>> We didn't drop unnamed_addr. I just don't think weakodr_addr +
>>>>>> unnamed_addr is safe to hide in all cases.
>>>>>>
>>>>>> I don't know if I interpreted local_unnamed_addr correctly but I
>>>>>> think it is mostly the same in thinLTO for ld64. The code generator only
>>>>>> looks at the individual module and ld64 will be in charge of merging all
>>>>>> the symbols with autohide. It doesn't really help in this case. But it is
>>>>>> interesting in general because according to the definition for
>>>>>> local_unnamed_addr, the symbol has to be linkonce_odr to be auto hide as
>>>>>> well. ThinLTO promotion can break that as well.
>>>>>>
>>>>>> Steven
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Feb 7, 2018, at 11:44 AM, Mehdi AMINI <joker.eph at gmail.com> wrote:
>>>>>>
>>>>>> Something I haven't seen mentioned: why are we dropping the
>>>>>> unnamed_addr? Can't we preserve it with the weak symbol? Would it be OK to
>>>>>> add auto-hide in this case (maybe it would already happen)?
>>>>>> Can we use the new local_unnamed_addr that was added (by pcc or
>>>>>> Rafael I don't remember)? I think this attribute matches exactly the
>>>>>> `auto-hide` semantic. Wasn't the idea that this could be added any time by
>>>>>> a module-level `infer_attribute` pass?
>>>>>>
>>>>>> --
>>>>>> Mehdi
>>>>>>
>>>>>>
>>>>>> 2018-02-07 11:36 GMT-08:00 Steven Wu via llvm-dev <
>>>>>> llvm-dev at lists.llvm.org>:
>>>>>>
>>>>>>> I didn't realize that that WeakDefCanBeHiddenDirective is only
>>>>>>> available on Darwin. So if we are doing it for #2, it should be a Darwin
>>>>>>> only fix as well.
>>>>>>>
>>>>>>> Steven
>>>>>>>
>>>>>>>
>>>>>>> On Feb 7, 2018, at 11:29 AM, Reid Kleckner <rnk at google.com> wrote:
>>>>>>>
>>>>>>> I agree with Teresa, we should probably do #2 to preserve behavior
>>>>>>> for now.
>>>>>>>
>>>>>>> On Wed, Feb 7, 2018 at 9:34 AM, Teresa Johnson via llvm-dev <
>>>>>>> llvm-dev at lists.llvm.org> wrote:
>>>>>>>
>>>>>>>> Hi Steven,
>>>>>>>>
>>>>>>>> I'd prefer not to inhibit importing. I am also concerned about
>>>>>>>> putting these symbols in the llvm.compiler_used (I don't recall earlier
>>>>>>>> discussion around this, but it seems like it could have effects on
>>>>>>>> optimization as you mention).
>>>>>>>>
>>>>>>>> What are the downsides of #2 (adding visibility hidden)? We already
>>>>>>>> do this when promoting internal linkage to external due to importing. I'm
>>>>>>>> not an expert on how this would affect link semantics.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Teresa
>>>>>>>>
>>>>>>>> On Tue, Feb 6, 2018 at 5:35 PM, Steven Wu <stevenwu at apple.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> I recently found that thinLTO doesn't deal with globals that has
>>>>>>>>> linkonce_odr and unnamed_addr (for macho at least) because it prohibits the
>>>>>>>>> autohide optimization during link time.
>>>>>>>>>
>>>>>>>>> In LLVM, we tagged a global linkonce_odr and unnamed_addr to
>>>>>>>>> indicate to the linker can hide them from symbol table if they were picked
>>>>>>>>> (aka, linkonce_odr_auto_hide linkage). It is very commonly used for some
>>>>>>>>> type of Tables for c++ code in clang for example.
>>>>>>>>> However, thinLTO is promoting these symbols to weak_odr +
>>>>>>>>> unnamed_addr, which lose the property. As a result, it introduces
>>>>>>>>> unnecessary weak external symbols and weak external are not good for
>>>>>>>>> performance on darwin platforms.
>>>>>>>>>
>>>>>>>>> I have few proposed solutions for this issue but I don't know
>>>>>>>>> which one works the best for none macho platforms and other LTO clients
>>>>>>>>> like lld.
>>>>>>>>>
>>>>>>>>> 1. Use llvm.compiler_used.
>>>>>>>>> As far as I know, the linkage promote are just there to keep the
>>>>>>>>> symbol through internalize and codegen so adding them to compiler used
>>>>>>>>> should solve this issue. I was told that there was some objections to do
>>>>>>>>> that in the first place. Is it because the globals added to compiler used
>>>>>>>>> is ignored by the optimizer so they cannot be internalized and they cannot
>>>>>>>>> be optimized away? This works well for the case I am looking at because c++
>>>>>>>>> VTable can't really be optimized and for darwin platforms because we can
>>>>>>>>> rely on ld64 to do dead_stripping if needed.
>>>>>>>>>
>>>>>>>>> 2. Add visibility hidden when promote linkonce_odr + unnamed_addr.
>>>>>>>>> Well,this doesn't really preserve the link semantics, but neither
>>>>>>>>> does promoting linkonce_odr to weak_odr. The global will still end up in
>>>>>>>>> the symbol table but at least it isn't external so it doesn't come with a
>>>>>>>>> performance cost.
>>>>>>>>>
>>>>>>>>> 3. We can teach function importer that it cannot just reference to
>>>>>>>>> linkonce_odr + unnamed_addr symbols without importing them. I have some
>>>>>>>>> thoughts about how to do this so I can propose something if people are
>>>>>>>>> interested going down this route. I am expecting at least add an entry in
>>>>>>>>> the global summery and change the cost of importing symbols that references
>>>>>>>>> to linkonce_odr + unnamed_addr symbols.
>>>>>>>>>
>>>>>>>>> 4. As a temporary fix, just targeting at the VTables for c++. We
>>>>>>>>> can put a special case for global constants that uses this linkage so they
>>>>>>>>> are never promoted and their parents are never imported into other modules.
>>>>>>>>> The benefit for inlining global constants is very minimal and I don't think
>>>>>>>>> we are doing it currently.
>>>>>>>>>
>>>>>>>>> Let me know if any of those solutions work for other LTO client.
>>>>>>>>>
>>>>>>>>> Thanks
>>>>>>>>>
>>>>>>>>> Steven
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Teresa Johnson |  Software Engineer |  tejohnson at google.com |
>>>>>>>> 408-460-2413 <(408)%20460-2413>
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> LLVM Developers mailing list
>>>>>>>> llvm-dev at lists.llvm.org
>>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> LLVM Developers mailing list
>>>>>>> llvm-dev at lists.llvm.org
>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>>>
>>>>>>
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>>
>
>
> --
> Teresa Johnson |  Software Engineer |  tejohnson at google.com |
> 408-460-2413 <(408)%20460-2413>
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>


-- 
-- 
Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180209/89c8578d/attachment-0001.html>

More information about the llvm-dev mailing list