[llvm-dev] ThinLTO and linkonce_odr + unnamed_addr

Thu Feb 8 10:55:29 PST 2018

On Thu, Feb 8, 2018 at 10:44 AM, Steven Wu via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

>
>
> 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.
>

To confirm, for this case I think then that we could add the following
where we promote linkonce_odr to weak_odr (here:
http://llvm-cs.pcc.me.uk/lib/Transforms/IPO/FunctionImport.cpp#659):

    if (GV.hasLinkOnceODRLinkage() && GV.hasGlobalUnnamedAddr() &&
GlobalValue::isWeakODRLinkage(NewLinkage))
       GV.setVisibility(GlobalValue::HiddenVisibility);

Or am I misunderstanding?

> For linkonce_odr local_unnamed_addr, it needs help from linker to do it
> correctly.
>
> 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/20180208/eee87fc6/attachment-0001.html>