[PATCH] D23680: [ThinLTO] Emit files for distributed builds for all modules

[Mehdi AMINI via llvm-commits]

mehdi_amini added inline comments.

================
Comment at: tools/gold/gold-plugin.cpp:755
@@ -754,1 +754,3 @@
 
+// Write empty files that may be expected by a distributed build
+// system when invoked with thinlto_index_only. This is invoked when
----------------
pcc wrote:
> tejohnson wrote:
> > mehdi_amini wrote:
> > > tejohnson wrote:
> > > > pcc wrote:
> > > > > I am not sure if this is needed, but even if it is, can't this be done inside lib/LTO?
> > > > This is needed because the distributed build system (in this case Bazel) wants to check that its expected outputs were generated. It doesn't know what the linker decided to include in the final link or not. This was a regression from the earlier behavior before we moved to the new LTO API.
> > > > 
> > > > My earlier version of the patch had this fix in lib/LTO, but Mehdi expressed a preference (in an offline discussion) to move this into the client.
> > > Right: it seems "client specific" as it is a constraint that comes from Bazel.
> > Although note that Bazel is independent of gold. It just happens that we build with using the gold linker and the Bazel distributed build system.
> When you say "It doesn't know what the linker decided to include" are you referring to which archive members the linker chose?
> 
> So the regression is that the distributed build is missing non-ThinLTO objects from archives, and the fix is to create empty .thinlto.bc files to cause the build system to include the non-ThinLTO objects in the final link?
> 
> Or am I misunderstanding?
> Although note that Bazel is independent of gold. It just happens that we build with using the gold linker and the Bazel distributed build system.

Right, I'm not convinced it belongs here entirely :)

My understanding is that it is very targeted: this operates under the assumptions that:

1) The build system has a list of all the input to the linker (including archive members) 
2) The build system expects the linker to produces 1-1 output for each input.

So it  is about both archive members not chosen by the linker, and input files that are not ThinLTO.
I don't think the latter case is handled though, it seems to be relying on every input being built with `-flto=thin` (or I missed a piece somewhere, how would an object file be handled?)


https://reviews.llvm.org/D23680