[PATCH] D21917: ThinLTO: Remove check for multiple modules before applying weak resolutions.

[Teresa Johnson via llvm-commits]

tejohnson added a comment.

In http://reviews.llvm.org/D21917#476244, @mehdi_amini wrote:

> In http://reviews.llvm.org/D21917#476048, @pcc wrote:
>
> > > I don't see this in the test cases in http://reviews.llvm.org/D21915 - one of them (alias_import) has multiple modules, and the other (weak_resolution) we are internalizing in the new case added.
> >
> >
> > To be more clear, I mean that we're already upgrading to weak in the multiple module case. As far as I'm concerned, that's the most important case here, as presumably if someone is using LTO their program has multiple modules. So I think there's no great harm in upgrading to weak in the single module case.
> >
> > Probably the most explicit instance of this is `linkoncefunc` in `weak_resolution`, which I've now added a promote+internalize test case for.
> >
> > > Maybe we need to add a flag to control this behavior, since it seems to need to be different for the different linkers.
> >
> >
> > That doesn't seem to be necessary.
>
>
> Agreed.
>
> > We just need a way to express "auto-hide + keep". There's already a way to do that which is compatible with Mach-O linkers, which is to use linkonce_odr + local_unnamed_addr + llvm.compiler.used. ELF linkers don't care about this (at least unless/until we extend ELF to have an auto-hide bit like Mach-O), so we can just do the same thing there. But that's outside of the scope of what I'm doing here.
>
>
> Agreed as well.


Ok SGTM, I didn't realized the Mach-O case was addressed.


Repository:
  rL LLVM

http://reviews.llvm.org/D21917