[ThinLTO] Add option to emit imports files for distributed backends

[tejohnson]

Summary

Add a new thinlto-emit-imports-files plugin option to enable emission of
plaintext lists of the imported files for each distributed backend compilation.
Used for distributed build file staging. This option is only valid with
thinlto-index-only (i.e. for distributed builds).

Depends on D19556.

[mehdi_amini]

It'd be nice to have this mirrored in llvm-lto.

Actually I have the impression that the gold-plugin is getting too fat: these functionality should be abstracted in LLVM an not be implemented here.

[tejohnson]

In D19636#415046, @joker.eph wrote:

It'd be nice to have this mirrored in llvm-lto.

I will do that and add a test.

Actually I have the impression that the gold-plugin is getting too fat: these functionality should be abstracted in LLVM an not be implemented here.

I could move this particular code into a helper in FunctionImport.cpp that takes in the ImportsLists and a file name as input, and do the file I/O there.
(Unrelated to this patch, but to the above comment: as noted in my response to D19556 this morning, I will pull the preceding code to compute the ModuleToSummariesForIndexMap into a helper in FunctionImport.cpp.)

[mehdi_amini]

Actually I have the impression that the gold-plugin is getting too fat: these functionality should be abstracted in LLVM an not be implemented here.

I could move this particular code into a helper in FunctionImport.cpp that takes in the ImportsLists and a file name as input, and do the file I/O there.
(Unrelated to this patch, but to the above comment: as noted in my response to D19556 this morning, I will pull the preceding code to compute the ModuleToSummariesForIndexMap into a helper in FunctionImport.cpp.)

Yes this is not about this patch in particular, this is about this plugin file that drifted over time. Its scope should be very narrowed to the interaction with gold IMO. All other logic should be refactored in some LLVM libraries.

[tejohnson]

• Address comments: refactor into a helper and add libLTO and llvm-lto support and tests

[tejohnson]

Ping

[mehdi_amini]

Oh sorry I missed the update to the patch, will look this morning (hopefully)

[tejohnson]

In D19636#421031, @joker.eph wrote:

Oh sorry I missed the update to the patch, will look this morning (hopefully)

Thanks, actually I meant to ping the one this is dependent on first (D19556 for emitting individual backend index files), but they were both updated last week (along with D19644) and are ready for review.

[mehdi_amini]

LGTM, with two remarks.

include/llvm/LTO/ThinLTOCodeGenerator.h

198 ↗

(On Diff #55528)

for *each* module? I'd say for \p ModulePath instead.

201 ↗

(On Diff #55528)

Same remark as the previous path: this does not really use anything specific to the ThinLTOCodeGenerator.

[tejohnson]

thanks

include/llvm/LTO/ThinLTOCodeGenerator.h

198 ↗

(On Diff #55528)

Yes the old comment wasn't really correct, will fix

201 ↗

(On Diff #55528)

Will change this to a static method too.

[vitalybuka]

This patch is the reason for
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/12540

[tejohnson]

In D19636#422547, @vitalybuka wrote:

This patch is the reason for
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/12540

This patch is not yet committed. However, another patch I committed this morning (D19556) is the cause of this. I'm working on tracking it down (unfortunately, it isn't obvious since we shouldn't even execute the routine in question and don't with a non-msan compiler. Trying to build an msan compiler right now to track down further). Should I revert in the meantime?

[vitalybuka]

My mistake, it is D19556. I searched by sub-string in the title.
It's better to revert if a fix is not obvious.

test/tools/gold/X86/thinlto_emit_imports.ll

+; Generate summary sections and test gold handling.
+; RUN: opt -module-summary %s -o %t.o
+; RUN: opt -module-summary %p/Inputs/thinlto.ll -o %t2.o
+
+; Ensure gold generates imports files if requested for distributed backends.
+; RUN: %gold -plugin %llvmshlibdir/LLVMgold.so \
+; RUN:    --plugin-opt=thinlto \
+; RUN:    --plugin-opt=thinlto-index-only \
+; RUN:    --plugin-opt=thinlto-emit-imports-files \
+; RUN:    -shared %t.o %t2.o -o %t3
+
+; The imports file for this module contains the bitcode file for
+; Inputs/thinlto.ll
+; RUN: cat %t.o.imports | count 1
+; RUN: cat %t.o.imports | FileCheck %s --check-prefix=IMPORTS1
+; IMPORTS1: test/tools/gold/X86/Output/thinlto_emit_imports.ll.tmp2.o
+
+; The imports file for Input/thinlto.ll is empty as it does not import anything.
+; RUN: cat %t2.o.imports | count 0
+
+declare void @g(...)
+
+define void @f() {
+entry:
+  call void (...) @g()
+  ret void
+}

tools/gold/gold-plugin.cpp

#endif
   // global index for the ThinLTO backends.
   static bool thinlto = false;
   // If false, all ThinLTO backend compilations through code gen are performed
   // using multiple threads in the gold-plugin, before handing control back to
   // gold. If true, write individual backend index files which reflect
   // the import decisions, and exit afterwards. The assumption is
   // that the build system will launch the backend processes.
   static bool thinlto_index_only = false;
+  // If true, when generating individual index files for distributed backends,
+  // also generate a "${bitcodefile}.imports" file at the same location for each
+  // bitcode file, listing the files it imports from in plain text. This is to
+  // support distributed build file staging.
+  static bool thinlto_emit_imports_files = false;
   // Additional options to pass into the code generator.
   // Note: This array will contain all plugin options which are not claimed
   // as plugin exclusive to pass to the code generator.
   // For example, "generate-api-file" and "as"options are for the plugin
   // use only and will not be passed.
   static std::vector<const char *> extra;
 
   static void process_plugin_option(const char *opt_)
static void process_plugin_option(const char *opt_)
     } else if (opt == "save-temps") {
       TheOutputType = OT_SAVE_TEMPS;
     } else if (opt == "disable-output") {
       TheOutputType = OT_DISABLE;
     } else if (opt == "thinlto") {
       thinlto = true;
     } else if (opt == "thinlto-index-only") {
       thinlto_index_only = true;
+    } else if (opt == "thinlto-emit-imports-files") {
+      thinlto_emit_imports_files = true;
     } else if (opt.size() == 2 && opt[0] == 'O') {
       if (opt[1] < '0' || opt[1] > '3')
         message(LDPL_FATAL, "Optimization level must be between 0 and 3");
       OptLevel = opt[1] - '0';
     } else if (opt.startswith("jobs=")) {
       if (StringRef(opt_ + 5).getAsInteger(10, Parallelism))
         message(LDPL_FATAL, "Invalid parallelism level: %s", opt_ + 5);
     } else if (opt == "disable-verify") {
for (claimed_file &F : Modules) {
     std::unique_ptr<ModuleSummaryIndex> Index =
         getModuleSummaryIndexForFile(F, InputFile.file());
 
     // Skip files without a module summary.
     if (Index)
       CombinedIndex.mergeFrom(std::move(Index), ++NextModuleId);
   }
 
+  if (options::thinlto_emit_imports_files && !options::thinlto_index_only)
+    message(LDPL_WARNING,
+            "thinlto-emit-imports-files ignored unless thinlto-index-only");
+
   if (options::thinlto_index_only) {
     // Collect for each module the list of function it defines (GUID ->
     // Summary).
     StringMap<std::map<GlobalValue::GUID, GlobalValueSummary *>>
         ModuleToDefinedGVSummaries(NextModuleId);
     CombinedIndex.collectDefinedGVSummariesPerModule(
         ModuleToDefinedGVSummaries);
 
for (claimed_file &F : Modules) {
         for (auto &ILI : ModuleImports->second) {
           auto &SummariesForIndex = ModuleToSummariesForIndex[ILI.first()];
           auto &DefinedGVSummaries = ModuleToDefinedGVSummaries[ILI.first()];
           for (auto &GI : ILI.second) {
             SummariesForIndex[GI.first] = DefinedGVSummaries[GI.first];
           }
         }
       }
+      if (options::thinlto_emit_imports_files) {
+        raw_fd_ostream ImportsOS(
+            (Twine(InputFile.file().name) + ".imports").str(), EC,
+            sys::fs::OpenFlags::F_None);
+        if (EC)
+          message(LDPL_FATAL, "Unable to open %s.imports",
+                  InputFile.file().name, EC.message().c_str());
+        if (ModuleImports != ImportLists.end())
+          for (auto &ILI : ModuleImports->second)
+            ImportsOS << ILI.first() << "\n";
+      }
       WriteIndexToFile(CombinedIndex, OS, &ModuleToSummariesForIndex);
     }
 
     cleanup_hook();
     exit(0);
   }
 
   // Create OS in nested scope so that it will be closed on destruction.