> 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