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

[Rafael Espíndola]

> 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.

Correct.

> 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.

Or that the language can guarantee that the addresses are not
compared, like c++ constructors. That is when the FE adds an
unnamed_addr marker.

>
> 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.

:-(

That is better done in the linker plugin (in the case of LTO) and in
codegen no? We should probably not have modified the IL for it. Would
you mind putting that comment on pr13175?

>> 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.

This part is identical on the ELF side.

> 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.

Similar idea to what I was trying to implement, but a bit stronger. In
the case of the linker plugin api in gold, gold only differentiates
two cases:

* I want this symbol because I will put it in a symbol table.
* I want this symbol because some native ELF file uses it.

In the first case the plugin has the option of hiding the symbol, and
gold will omit it from the table. In other words, the optimization
only works when all uses are from IL files.

>> 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.

Not strip, but make them hidden so that we get smaller symbol tables
in the DSOs.

Thanks,
Rafael

P.S.: The part of my patch that I ported to globalopt found an bug in
clang (or maybe in the itanium abi) and had to be reverted, so I am
waiting for that to be resolved before trying this again.