[PATCH] D22356: [ThinLTO] Perform conservative weak/linkonce resolution in distributed backend case

[Teresa Johnson via llvm-commits]

tejohnson added a comment.

In https://reviews.llvm.org/D22356#487812, @mehdi_amini wrote:

> In https://reviews.llvm.org/D22356#487800, @tejohnson wrote:
>
> > In https://reviews.llvm.org/D22356#487784, @mehdi_amini wrote:
> >
> > > > That's a good question and an idea I thought about briefly but discarded for a couple reasons. I was concerned about requiring communication between the ThinLink and final link to build the link line (it would be more difficult to support in a build system, and also seems conceptually more complicated).
> > >
> > >
> > > How is the final link invocation computed right now?
> >
> >
> > The link line is essentially the same, but with native .o instead of the bitcode .o. (See the new test case for an example)
>
>
> The question is: in the presence of static archives, how do you generates --start-lib/--end-lib? This seems to already require some build-system integration?


We use --start-lib/--end-lib in all of our links - not regular archive libraries. Yes, a distributed build on regular archive libraries will require build system integration to extract the individual object files first.

> 

> 

> > > 

> 

> > 

> 

> > > 

> 

> > 

> 

> > > > Also I'm not 100% convinced that removing the --start-lib/--end-lib, even if we only include those object files the linker decided to select symbols from, would result in the same linking behavior.

> 

> > 

> 

> > > 

> 

> > 

> 

> > > 

> 

> > 

> 

> > > Your observations about the linker picking different symbols seem to indicate that the --start-lib/--end-lib model is already broken.

> 

> > 

> 

> > 

> 

> > When you say "is already broken" do you mean even in non-ThinLTO mode? I'm not sure why - it is just like having an archive of the objects between each start/end pair.

> 

> 

> I'm only talking about ThinLTO and the two-stage linking, i.e. the second invocation of the linker does not end-up with the same prevailing resolution as the first invocation. Your current patches are working around this deficiency.

> 

> > 

> 

> > 

> 

> > > If a list of `.o` on the command line is not enough for relinking, there's gonna be a need for a "linker resolution map" file that drives the linker.

> 

> > 

> 

> > 

> 

> > In ThinLTO it is because of the change (between the ThinLink and native object link) in which strong references exist between objects/libraries due to importing and inlining. But I believe with this patch and the follow-on https://reviews.llvm.org/D22467 the importing and symbol resolution is made suitably conservative.

> 

> 

> I don't see any justification for --start-lib/--end-lib right now.


We use --start-lib/--end-lib internally instead of regular objects. So it is not a matter of justification, it is a matter of keeping that working.


https://reviews.llvm.org/D22356