> Honza,
>
> I am not sure that the problem is caused only by aliases and thunks.
> The large increase in AIX linker warnings about branches not followed
> by nop also worry me.
>
> Your patch was about visibility. How does the more aggressive
ipa-visibility is a pass that basically bring external symbols local whenever
it can (i.e. does privatizatoin), it is not that much about ELF visibilities.
> algorithm behave on a platform that does not support visibility? Is it
> defaulting to hidden? If the new algorithm is being too aggressive and
> incorrectly converting calls from global to local, it could cause
> serious problems for AIX because the GOT register will not be
> restored.
One of optimizations IPA visibility does is that it looks for global symbols
that
1) are believed by target to be overwritable (interposed - I will probably
update name here) by linker to a different definition (we have
decl_binds_to_current_def_p that is target tweakable), and
2) the interposition may not change semantics of the symbol (i.e. function
body must be the same or variable initializer must match)
For some symbols (such as inline functions, virtual tables, readonly
variables with no address taken) we know it won't.
3) the symbol's definition can not be optimized away by linker
(by symtab_can_be_discarded)
If all conditons match, it creates a static alias (not hidden, just local to
the .o file) and redirects users of the symbol to the alias. This should be
always win: we know that the representation of symbol will survive to final
binary (it is not discarded) and we replace expensive references through GOT by
cheap references to local symbol.
We did, for longer time, redirect calls. The troublesome patch makes us to
redirect also references in virtual tables and newly we also consider virtual
tables themselves for aliases.
This should be a win, since virtual tables tends to be startup time hogs and
it is common to have virtual tables in one DSO to refer to comdats that are
shared with other DSO, but because they must be the same, we can just ignore
the sharing.
On AIX we observed interesting series of events.
1) First the output machinery was not quite able to declare local static
alias for a symbol (this was about year ago when I introduced the first
change).
2) AIX assembler seems to issue warning when jump happens to the local
static alias confusing it with the global symbol it is aliasing.
I do not know if the warning is just bogus or we output something
incorrectly. This is the reason for NOP warning as I understand.
3) Important difference we is that in AIX all COMDAT symbols are considered
non-discardable. This makes us to produce a lot more aliases than on ELF
system.
I am not sure if this is acurate and AIX linker really has no means
of removing duplicated bodies of COMDAT functions/initializers of variables.
If it has, we need to model it in symtab_can_be_discarded.
Currently we test whether symbol is in comdat group and in the case of
AIX it isn't.
As disucssed earlier, I am thinking about making symtab_can_be_discarded
return true also for implicit sections (I have WIP patch for this to
commit today). Earlier version of the patch however did not solve
the warnings.
I also tested libstdc++.so sizes with current mainline, with the
local aliases disabled and with this change and current mainline
wins. Suggesting that perhaps there is really no way to discard
duplicated comdats or libstdc++ doesn't really have them.
Insight here would be welcome - I am sure it is easy to test if
including and using inline __noinline__ function in two units
leads to two copies of that beast or not. It would be nice to know
how native toolchain handles it and if GCC does the same trick.
4) Before 4.9 we hit bug in inliner dealing with aliases that gave me
a headache. It reproduced on AIX only because of 3)
5) We hit problem with aliases to anchored sections, hopefully solved now
6) We hit the problem that AIX assembler silently accepts but miscompile
when alias is declared before its target. THis is also hopefully
fixed now. We hit it twice - one for normal symbols about year ago
and now again for thunks.
7) Given number of issues I ended up writting a verifier that checks
sanity of sections, aliases and comdat groups. I check
a) all symbols in comdat group have the same section
b) alias and its target have same comdat group & section characteristics
(obivously one can not place alias out of comdat)
c) I check sanity of IMPLICIT_SECTION flag.
It turns out that C++ FE makes complete mess of those breaking all three
rules due to complex interactions in between comdat code, same body aliases
and the way thunks are produced. I think this may confuse AIX output
macros since they are just given a contradictionary information.
So it may turn out that the AIX assembler warnings are correct.
Have patch in testing and it seems that warnings are gone, but I am not
at the end of bootstrap, yet, now resolving issues with libfortran.
I plan to add check that we do not have two comdat groups of the same
name, but I am not there, yet. As usual, verifier improvements show
issues that are a lot of fun to resolve.
Still, i would not think of the original optimization as aggressive and risky
one.
We just found that the way we deal with aliases is terribly broken at some
targets.
Pretty much all the bugs above are reproducable by a testcases using alias
attribute.
Yes, i understand it is frustrating (to me too) and that I should be faster
dealing
with the issue (teaching in Canada is time consuming).
I am now again testing similar changes on AIX before comitting. I considered
the original TLC patch as simple change given that we already debugged the
local aliases for 4.9. I am sorry for that. I will try to be sure that I get
libstdc++
testsuite into a shape either by comitting the patch for 7) or reverting the
relevant
part of the TLC patch.
Honza
