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

[Teresa Johnson via llvm-commits]

tejohnson added inline comments.

================
Comment at: lib/Bitcode/Reader/BitcodeReader.cpp:6111
@@ -6107,1 +6110,3 @@
+          SeenValueSymbolTable = true;
+        }
         SeenGlobalValSummary = true;
----------------
mehdi_amini wrote:
> tejohnson wrote:
> > mehdi_amini wrote:
> > > Can you commit this as a separate patch? This is mostly unrelated to the distributed build.
> > It is related to the change to always emit a summary when compiling for ThinLTO. Maybe I should split that portion (along with the corresponding fixes like this one) out into a separate patch. Is that what you meant? It fixes part of the distributed build issue.
> It also change the behavior outside of distributed build right?
> It will make files that we didn't include in ThinLTO before, part of the ThinLTO partitions now.
Yes. Ok, will split that whole part out into a separate patch. This patch will end up just being the changes to gold-plugin.cc, test/tools/gold/X86/v1.12/thinlto_emit_linked_objects.ll, and the refactoring of getThinLTOOutputFile in LTO.

================
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
----------------
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.


https://reviews.llvm.org/D23680