[PATCH] D19308: ThinLTO: Resolve linkonce_odr aliases just like functions

[Teresa Johnson via llvm-commits]

tejohnson added inline comments.

================
Comment at: test/ThinLTO/X86/alias_resolution.ll:39
@@ +38,3 @@
+; PROMOTE-DAG: @linkonceODRfuncWeakAlias = weak alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
+; PROMOTE-DAG: @linkonceODRfuncLinkonceAlias = linkonce alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
+; PROMOTE-DAG: @linkonceODRfuncWeakODRAlias = weak_odr alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
----------------
joker.eph wrote:
> tejohnson wrote:
> > joker.eph wrote:
> > > There is a bug I think, this one should be promoted right?
> > You mean resolved? The alias itself isn't ODR so I think it stays Linkonce(Any).
> I think the current situation is murky. Let see if I can summarize it right:
> 
> - linkonce (ODR or not) can be discarded: if the symbol is referenced from another module, it needs to be "resolved" for *correctness*.
> - weak (ODR or not) can't be discarded: we don't need to resolve them for *correctness*.
> - linkonce/weak (the *non* ODR variant) can't be inlined, there is probably no interest in importing them, we should disable that in the importing decision.
> - For all of them: we want to resolve when there is more than one definition for compile-time optimization purpose.
Yes, I think that is a correct summary. So the bug as you point out is in fact that we should be resolving linkonce(any) to weak(any) since it is discardable.

Otherwise if under some situation the call was removed in the original exporting module (not through inlining since that doesn't happen for non-ODR linkonce, but some other transformation), any imported reference that didn't pull in the def can get a linker undef.

So seems we need to do the resolving of linkonce(any) to weak(any) if it is exported. And could do it when there is more than one definition as a compile time optimization.

I remember that in an older version of the patch you were doing this resolution for the non-odr types as well, but I don't remember why you stopped doing that for the WeakAny and LinkOnceAny. Was there an issue that I am not remembering?


http://reviews.llvm.org/D19308