llvm dev - Feb 2018 - ThinLTO and linkonce_odr + unnamed_addr

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>

Add Rafael here. Because he basically ask me the same question in my commit.

I wonder why clang choose to do unnamed_addr instead of just do visibility
hidden. If linkonce_odr + unnamed_addr is just hidden, is the only difference
linkonce_odr + unnamed_addr not in the symbol table?

Steven
> On Feb 9, 2018, at 10:50 AM, Peter Collingbourne <peter at pcc.me.uk>
wrote:
> 
> 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 <mailto: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
<mailto: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 <mailto: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
<mailto:joker.eph at gmail.com>> wrote:
>>> 
>>> 
>>> 
>>> 2018-02-08 10:44 GMT-08:00 Steven Wu <stevenwu at apple.com
<mailto:stevenwu at apple.com>>:
>>> 
>>> 
>>>> On Feb 8, 2018, at 10:28 AM, Mehdi AMINI <joker.eph at
gmail.com <mailto:joker.eph at gmail.com>> wrote:
>>>> 
>>>> 
>>>> 
>>>> 2018-02-08 9:33 GMT-08:00 Steven Wu <stevenwu at apple.com
<mailto:stevenwu at apple.com>>:
>>>> 
>>>> 
>>>>> On Feb 7, 2018, at 4:03 PM, Mehdi AMINI <joker.eph at
gmail.com <mailto:joker.eph at gmail.com>> wrote:
>>>>> 
>>>>> 
>>>>> 
>>>>> 2018-02-07 12:45 GMT-08:00 Steven Wu <stevenwu at
apple.com <mailto:stevenwu at apple.com>>:
>>>>> 
>>>>> 
>>>>>> On Feb 7, 2018, at 12:36 PM, Mehdi AMINI <joker.eph
at gmail.com <mailto: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 <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 <mailto: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 <mailto: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 <mailto: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 <mailto: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 <mailto: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 <mailto: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 <mailto:tejohnson at google.com> |	 408-460-2413
<tel:(408)%20460-2413>
>>>>>>>> _______________________________________________
>>>>>>>> LLVM Developers mailing list
>>>>>>>> llvm-dev at lists.llvm.org <mailto:llvm-dev
at lists.llvm.org>
>>>>>>>>
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> _______________________________________________
>>>>>>> LLVM Developers mailing list
>>>>>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>>>>>>>
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>> 
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>> 
>> 
>> 
>> 
>> -- 
>> Teresa Johnson |	 Software Engineer |	 tejohnson at google.com
<mailto:tejohnson at google.com> |	 408-460-2413
<tel:(408)%20460-2413>
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<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/acd334a5/attachment.html>

Peter Collingbourne via llvm-dev <llvm-dev at lists.llvm.org> writes:
> 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.
I think we can: https://bugs.llvm.org/show_bug.cgi?id=36325

LTO would still be important for local_unnamed_addr.

Cheers,
Rafael

Steven Wu <stevenwu at apple.com> writes:
> Add Rafael here. Because he basically ask me the same question in my
commit.
>
> I wonder why clang choose to do unnamed_addr instead of just do visibility
hidden. If linkonce_odr + unnamed_addr is just hidden, is the only difference
linkonce_odr + unnamed_addr not in the symbol table?
A local symbol being in the symbol table is mostly an implementation
detail.

Not marking a linkonce_odr + *global* unnamed_addr hidden is a missing
optimization on non-MachO.

Cheers,
Rafael