[llvm-dev] ThinLTO and linkonce_odr + unnamed_addr

Fri Feb 9 09:33:47 PST 2018

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180209/7c5827bc/attachment.html>