llvm dev - Feb 2018 - ThinLTO and linkonce_odr + unnamed_addr

> On Feb 14, 2018, at 09:55, Steven Wu via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> I did a bit more digging for the auto hide problem. Here is my finding that
prevent us from doing this by default in GlobalOpts
> 
> 1. When a symbol is linkonce_odr hidden unnamed_addr, it emits both
'.private_extern' and '.weak_def_can_be_hidden' asm directives
on macho platform. There result of that is .private_extern will win so this is
essentially linkonce_odr hidden.
> 2. ld64 does treat these two type of symbols differently. For example, ld64
will deduplicate all the can_be_hidden symbols to reduce code size. This
can't be achieved when the symbols is private external.
For others' context, this deduplication in ld64 will effectively merge
(e.g.) std::vector<int>::push_back with
std::vector<unsigned>::push_back based on function content hashes.

It would be a shame to block this optimization because of #1 – it seems like
there should be a way to work around that.  Perhaps there's something we can
change in ld64.
> Steven
> 
>> On Feb 9, 2018, at 2:00 PM, Steven Wu via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
>> 
>> Interesting. Now I understand the full picture. For MachO, linker is
picking based on atoms so it picks the atom with more visibility which doesn’t
have the constraint you mentioned. Let me do some more digging. Worst case I
will add a different rule for MachO but it might be fine.
>> 
>> Steven
>> 
>>> On Feb 9, 2018, at 1:40 PM, Rafael Avila de Espindola
<rafael.espindola at gmail.com> wrote:
>>> 
>>> Steven Wu <stevenwu at apple.com> writes:
>>> 
>>>> Rafael, another question for you. IRLinker currently takes min
visibility for the symbol (lib/Linker/LinkModules.cpp:120). Should it take the
max visibility? At least that is what ld64 is doing and is somewhat related to
this change because I want to make sure the behavior is consistent if we mark
more stuff as hidden.
>>> 
>>> No, it should take the min visibility to be consistent with the ELF
>>> spec. The spec says
>>> 
>>> If different visibility attributes are specified for distinct
>>> references to or definitions of a symbol, the most constraining
>>> visibility attribute must be propagated to the resolving symbol in
the
>>> linked object.
>>> 
>>> The reason the ELF spec mandates the minimum is that a .o having a
>>> hidden symbol foo means that that file can access foo in a way that
is
>>> only valid under the assumption it is local. It not might change
the pic
>>> register before doing a call for example.
>>> 
>>> Cheers,
>>> Rafael
>> 
>> _______________________________________________
>> 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

On Wed, Feb 14, 2018 at 2:26 PM, Duncan P. N. Exon Smith via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
>
>
> > On Feb 14, 2018, at 09:55, Steven Wu via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
> >
> > I did a bit more digging for the auto hide problem. Here is my finding
> that prevent us from doing this by default in GlobalOpts
> >
> > 1. When a symbol is linkonce_odr hidden unnamed_addr, it emits both
> '.private_extern' and '.weak_def_can_be_hidden' asm
directives on macho
> platform. There result of that is .private_extern will win so this is
> essentially linkonce_odr hidden.
> > 2. ld64 does treat these two type of symbols differently. For example,
> ld64 will deduplicate all the can_be_hidden symbols to reduce code size.
> This can't be achieved when the symbols is private external.
>
> For others' context, this deduplication in ld64 will effectively merge
> (e.g.) std::vector<int>::push_back with
std::vector<unsigned>::push_back
> based on function content hashes.
>
> It would be a shame to block this optimization because of #1 – it seems
> like there should be a way to work around that.  Perhaps there's
something
> we can change in ld64.
>
As far as I can tell, the "auto hide" property is independent of
whether
the symbol is actually hidden (the former is N_WEAK_DEF|N_WEAK_REF in
n_desc, and the latter is N_PEXT in n_type). So perhaps what you can do is
change ld64 to recognize N_WEAK_DEF|N_WEAK_REF together with N_PEXT as
meaning "hidden and mergeable".

Peter
>
> > Steven
> >
> >> On Feb 9, 2018, at 2:00 PM, Steven Wu via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
> >>
> >> Interesting. Now I understand the full picture. For MachO, linker
is
> picking based on atoms so it picks the atom with more visibility which
> doesn’t have the constraint you mentioned. Let me do some more digging.
> Worst case I will add a different rule for MachO but it might be fine.
> >>
> >> Steven
> >>
> >>> On Feb 9, 2018, at 1:40 PM, Rafael Avila de Espindola <
> rafael.espindola at gmail.com> wrote:
> >>>
> >>> Steven Wu <stevenwu at apple.com> writes:
> >>>
> >>>> Rafael, another question for you. IRLinker currently takes
min
> visibility for the symbol (lib/Linker/LinkModules.cpp:120). Should it
> take the max visibility? At least that is what ld64 is doing and is
> somewhat related to this change because I want to make sure the behavior is
> consistent if we mark more stuff as hidden.
> >>>
> >>> No, it should take the min visibility to be consistent with
the ELF
> >>> spec. The spec says
> >>>
> >>> If different visibility attributes are specified for distinct
> >>> references to or definitions of a symbol, the most
constraining
> >>> visibility attribute must be propagated to the resolving
symbol in the
> >>> linked object.
> >>>
> >>> The reason the ELF spec mandates the minimum is that a .o
having a
> >>> hidden symbol foo means that that file can access foo in a way
that is
> >>> only valid under the assumption it is local. It not might
change the
> pic
> >>> register before doing a call for example.
> >>>
> >>> Cheers,
> >>> Rafael
> >>
> >> _______________________________________________
> >> 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
>


-- 
-- 
Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180215/82e8eb9b/attachment.html>

> On Feb 15, 2018, at 4:19 PM, Peter Collingbourne <peter at pcc.me.uk>
wrote:
> 
> On Wed, Feb 14, 2018 at 2:26 PM, Duncan P. N. Exon Smith via llvm-dev
<llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>>
wrote:
> 
> 
> > On Feb 14, 2018, at 09:55, Steven Wu via llvm-dev <llvm-dev at
lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
> >
> > I did a bit more digging for the auto hide problem. Here is my finding
that prevent us from doing this by default in GlobalOpts
> >
> > 1. When a symbol is linkonce_odr hidden unnamed_addr, it emits both
'.private_extern' and '.weak_def_can_be_hidden' asm directives
on macho platform. There result of that is .private_extern will win so this is
essentially linkonce_odr hidden.
> > 2. ld64 does treat these two type of symbols differently. For example,
ld64 will deduplicate all the can_be_hidden symbols to reduce code size. This
can't be achieved when the symbols is private external.
> 
> For others' context, this deduplication in ld64 will effectively merge
(e.g.) std::vector<int>::push_back with
std::vector<unsigned>::push_back based on function content hashes.
> 
> It would be a shame to block this optimization because of #1 – it seems
like there should be a way to work around that.  Perhaps there's something
we can change in ld64.
> 
> As far as I can tell, the "auto hide" property is independent of
whether the symbol is actually hidden (the former is N_WEAK_DEF|N_WEAK_REF in
n_desc, and the latter is N_PEXT in n_type). So perhaps what you can do is
change ld64 to recognize N_WEAK_DEF|N_WEAK_REF together with N_PEXT as meaning
"hidden and mergeable".
Yes, they are independent and llvm does generate both in the object file.
However, all the tools ignores "auto hide" when the symbol is already
hidden. Teach ld64 (and other tools like nm) to not ignore the field can
potentially fix the problem. I still need to check if there are other instances
in ld64 that might rely on auto hide.

And also deduplicate auto hide symbols is basically a choice of implementation
for ld64. The fact that the symbol doesn't have autohide does not imply it
cannot be merged or deduplicated. It used "auto hide" because it adds
too much overhead to run on all the private symbols so it picks a subset that
which is the ones that gets auto hide. I don't think this is a blocker for
whichever fix we come up with.

Steven
> 
> Peter
> 
> > Steven
> >
> >> On Feb 9, 2018, at 2:00 PM, Steven Wu via llvm-dev <llvm-dev at
lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
> >>
> >> Interesting. Now I understand the full picture. For MachO, linker
is picking based on atoms so it picks the atom with more visibility which
doesn’t have the constraint you mentioned. Let me do some more digging. Worst
case I will add a different rule for MachO but it might be fine.
> >>
> >> Steven
> >>
> >>> On Feb 9, 2018, at 1:40 PM, Rafael Avila de Espindola
<rafael.espindola at gmail.com <mailto:rafael.espindola at
gmail.com>> wrote:
> >>>
> >>> Steven Wu <stevenwu at apple.com <mailto:stevenwu at
apple.com>> writes:
> >>>
> >>>> Rafael, another question for you. IRLinker currently takes
min visibility for the symbol (lib/Linker/LinkModules.cpp:120). Should it take
the max visibility? At least that is what ld64 is doing and is somewhat related
to this change because I want to make sure the behavior is consistent if we mark
more stuff as hidden.
> >>>
> >>> No, it should take the min visibility to be consistent with
the ELF
> >>> spec. The spec says
> >>>
> >>> If different visibility attributes are specified for distinct
> >>> references to or definitions of a symbol, the most
constraining
> >>> visibility attribute must be propagated to the resolving
symbol in the
> >>> linked object.
> >>>
> >>> The reason the ELF spec mandates the minimum is that a .o
having a
> >>> hidden symbol foo means that that file can access foo in a way
that is
> >>> only valid under the assumption it is local. It not might
change the pic
> >>> register before doing a call for example.
> >>>
> >>> Cheers,
> >>> Rafael
> >>
> >> _______________________________________________
> >> 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>
> 
> 
> 
> -- 
> -- 
> Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180215/ebcaea7c/attachment.html>