[llvm-commits] [patch][pr11866] Drop linkonce_odr symbols from the symbol table when safe

[Nick Kledzik]

On Jun 22, 2012, at 9:20 AM, Rafael Espíndola wrote:
> On 21 June 2012 22:40, Nick Kledzik <kledzik at apple.com> wrote:
>> Rafael,
>> 
>> I think LTO_SYMBOL_IS_LINKONCE_ODR_UNUSED_ADDR may actually be the same as the existing LTO_SYMBOL_SCOPE_DEFAULT_CAN_BE_HIDDEN flag.  That flag is only set on definitions of which the front end did not see the address taken.  If the linker does not see the address taken either, then the linker can change the visibility to hidden in the final linked image.  It might just be that the linker_private_weak_def_auto linkage type was only wired up for Darwin.
> 
> Is it? It looks stronger. It is set for linker_private_weak_def_auto
> symbols. The IR documentation says:
> 
>> Similar to "linker_private_weak", but it's known that the address of the object is not taken.

My larger point is that I think darwin and linux are trying to solve the same problem, but have independently come up with similar but different solutions.   The problem is that global symbols that are exposed to the runtime linker have a performance cost.  If you can determine at build time that a symbol does not need to be global, making it hidden will improve runtime performance.  

The obvious candidate for this is templates and inline functions defined in headers.  Since these can and often are inlined into other functions, it is ok if different linkage units each have there own (hidden) copy of the function.  The one catch is that if two linkage units have code that takes the address of the inline function/template, both instances should arrive at the same value for function's address.  Therefore, those inline functions that have their address taken cannot be automatically made hidden.


That said, I just reread the llvm LinkageTypes doc, and the LinkerPrivateWeakDefAutoLinkage is not what I had requested last year.  I think the term "private" is overloaded and caused confusion.  I wanted a flag that would show up in mach-o object files on weak symbols if nothing in that translation unit "took the address" of that weak symbol.  This has nothing to do with LinkerPrivate (which means the symbol *name* will be stripped by the linker).  It should have been named something like LinkOnceODRAutoHideLinkage.  



> 
> Which I assume means "not taken anywhere". Given that, the linker
> plugin would be allowed to just tell the linker the symbol is hidden
> and the linker will not try to put it in the symbol table. No need for
> the new gold API.
> 
> From the above, I think  LTO_SYMBOL_SCOPE_DEFAULT_CAN_BE_HIDDEN can
> also be used to handle unnamed_addr functions and constants.  I have
> attached a patch that implements this bit, but now that I think of it,
> this could very well be just an IL optimization that marks as hidden
> linkonce_odr functions and constants that are also unnamed_addr. I
> think mach-o needs the special linkages that have to reach the linker
> because of things like objc metadata.
I don't know how the gold plugin works.  But for darwin, there are two cases: without LTO and with LTO.  Without LTO means clang is producing mach-o object files.  In that case, the compiler (IL optimizer) can only see that one translation unit, so it can't know if some other translation unit will take the address of the weak (linkonce_odr) function. That is were there new auto-hide bit comes in.  The compiler can tell that linker (via that bit) that this translation unit does not take the address of an inline function.  In the LTO case, the darwin linker supports mixing mach-o and bitcode object files.  But since the LTO optimizer cannot see the mach-o object files, it does not know if any of them take the address of some inline function.  Again limiting what the optimizer can do.  But just like the single file compile case, by having a new auto-hide bit, the LTO engine can tell the linker that nothing in the LTO merged code takes the address of an inline function.  When the linker coalesces two definitions, one with the auto-hide bit and one without, it always picks the one without the bit.  Then after all coalescing is done, any definitions remaining with auto-hide bit set are changed to visibility hidden.


> 
> What it cannot be used for are regular linkonce_odr functions. The
> semantics for those are that they can be made hidden/dropped
The "dropped" part here should be clarified.  The "link once"  documentation says "Unreferenced link once globals are allowed to be discarded".  I always read that as a compiler optimization - not a linker optimization.  That is, the AST or bitcode may have hundreds of linkonce(odr) function definitions (from inline header definitions), but the code generated need only produce code for inline functions actually used in that translation unit.    I don't see a point of the linker trying to dead-stripping away these linkonce functions, because the linker will never see any unused linkonce functions.


> if they
> don't have their address taken anywhere in the DSO, but we will have
> to wait for the linker to tell us if that is the case. Gold is a bit
> more conservative, it doesn't tell us if the address was taken, only
> if the symbol was used for anything else other than putting it in the
> symbol table, but that is probably good enough.
In Darwin, the linker does not determine if the address was taken.  It only makes a global weak symbol hidden, if every copy of it from every translation unit have their auto-hide bit set.


> 
> With the attached patch the combined IL for clang goes from 1870
> linkonce_odr symbols to 1598. Let me know if you like this patch of if
> you agree that it should be just an IL optimization. In any case we
> can handle the linkonce_odr functions case in a new patch.
> 
>> -Nick
> 
> Cheers,
> Rafael
> <t.patch>