[PATCH] D18346: ThinLTO: special handling for LinkOnce functions

[Mehdi AMINI via llvm-commits]

joker.eph added inline comments.

================
Comment at: lib/LTO/ThinLTOCodeGenerator.cpp:182
@@ +181,3 @@
+    if (!IsFirstDefinitionForLinker(*GVInfo, Index, ModulePath))
+      GV.setLinkage(GlobalValue::AvailableExternallyLinkage);
+    break;
----------------
joker.eph wrote:
> tejohnson wrote:
> > Thinking more about this - I don't think it is correct for WeakAny or LinkOnceAny to be converted to AvailableExternally. This could change the program result. The reason is that the selected copy is allowed to have different behavior than the unselected version. This is why WeakAny and LinkOnceAny return true from GlobalValue::mayBeOverridden(), which prevents optimizations such as inlining and therefore ensures that the linker-selected copy is executed. Changing their linkage to AvailableExternally enables the unselected versions to be inlined, and therefore the selected body is not executed at runtime, which can result in different program behavior.
> Yeah that's a very good catch.
> 
> It lead me to a more interesting point: if we know which copy the linker will select, we should turn it into a strong definition and *eliminate* all the other. This way the strong one will be imported and could be inlined where it usually can't. I imagine gold is already doing that in LTO.
Maybe we can't turn a weak into a strong, because the dynamic loader can override it with another symbol in another library?


http://reviews.llvm.org/D18346