[ThinLTO/gold] Write empty imports even for modules with symbols

Summary: ThinLTO may skip object for other reasons, e.g. if there is no summary.

Reviewers: pcc, eugenis

Subscribers: mehdi_amini, inglorion, eraman, llvm-commits

Differential Revision: https://reviews.llvm.org/D42514

llvm-svn: 323818

llvm/include/llvm/LTO/LTO.h

@@ -210,10 +210,14 @@ ThinBackend createInProcessThinBackend(unsigned ParallelismLevel);
 /// appends ".thinlto.bc" and writes the index to that path. If
 /// ShouldEmitImportsFiles is true it also writes a list of imported files to a
 /// similar path with ".imports" appended instead.
+/// OnWrite is callback which receives module identifier and notifies LTO user
+/// that index file for the module (and optionally imports file) was created.
+using IndexWriteCallback = std::function<void(const std::string &)>;
 ThinBackend createWriteIndexesThinBackend(std::string OldPrefix,
                                           std::string NewPrefix,
                                           bool ShouldEmitImportsFiles,
-                                          std::string LinkedObjectsFile);
+                                          std::string LinkedObjectsFile,
+                                          IndexWriteCallback OnWrite);
 
 /// This class implements a resolution-based interface to LLVM's LTO
 /// functionality. It supports regular LTO, parallel LTO code generation and

llvm/lib/LTO/LTO.cpp

@@ -1033,16 +1033,18 @@ class WriteIndexesThinBackend : public ThinBackendProc {
   std::string LinkedObjectsFileName;
   std::unique_ptr<llvm::raw_fd_ostream> LinkedObjectsFile;
 
+  lto::IndexWriteCallback OnWrite;
+
 public:
   WriteIndexesThinBackend(
       Config &Conf, ModuleSummaryIndex &CombinedIndex,
       const StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries,
       std::string OldPrefix, std::string NewPrefix, bool ShouldEmitImportsFiles,
-      std::string LinkedObjectsFileName)
+      std::string LinkedObjectsFileName, lto::IndexWriteCallback OnWrite)
       : ThinBackendProc(Conf, CombinedIndex, ModuleToDefinedGVSummaries),
         OldPrefix(OldPrefix), NewPrefix(NewPrefix),
         ShouldEmitImportsFiles(ShouldEmitImportsFiles),
-        LinkedObjectsFileName(LinkedObjectsFileName) {}
+        LinkedObjectsFileName(LinkedObjectsFileName), OnWrite(OnWrite) {}
 
   Error start(
       unsigned Task, BitcodeModule BM,
@@ -1075,9 +1077,14 @@ public:
       return errorCodeToError(EC);
     WriteIndexToFile(CombinedIndex, OS, &ModuleToSummariesForIndex);
 
-    if (ShouldEmitImportsFiles)
+    if (ShouldEmitImportsFiles) {
-      return errorCodeToError(
+      EC = EmitImportsFiles(ModulePath, NewModulePath + ".imports", ImportList);
-          EmitImportsFiles(ModulePath, NewModulePath + ".imports", ImportList));
+      if (EC)
+        return errorCodeToError(EC);
+    }
+
+    if (OnWrite)
+      OnWrite(ModulePath);
     return Error::success();
   }
 
@@ -1088,13 +1095,14 @@ public:
 ThinBackend lto::createWriteIndexesThinBackend(std::string OldPrefix,
                                                std::string NewPrefix,
                                                bool ShouldEmitImportsFiles,
-                                               std::string LinkedObjectsFile) {
+                                               std::string LinkedObjectsFile,
+                                               IndexWriteCallback OnWrite) {
   return [=](Config &Conf, ModuleSummaryIndex &CombinedIndex,
              const StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries,
              AddStreamFn AddStream, NativeObjectCache Cache) {
     return llvm::make_unique<WriteIndexesThinBackend>(
         Conf, CombinedIndex, ModuleToDefinedGVSummaries, OldPrefix, NewPrefix,
-        ShouldEmitImportsFiles, LinkedObjectsFile);
+        ShouldEmitImportsFiles, LinkedObjectsFile, OnWrite);
   };
 }
 

llvm/test/tools/gold/X86/v1.12/thinlto_emit_linked_objects.ll

@@ -1,37 +1,40 @@
+; RUN: rm -f %t*.o.thinlto.bc
+; RUN: rm -f %t*.o.imports
+
 ; First generate bitcode with a module summary index for each file
-; RUN: opt -module-summary %s -o %t.o
+; RUN: opt -module-summary %s -o %t1.o
 ; RUN: opt -module-summary %p/Inputs/thinlto_emit_linked_objects.ll -o %t2.o
+; RUN: opt %s -o %t3.o
 
 ; Next do the ThinLink step, specifying thinlto-index-only so that the gold
 ; plugin exits after generating individual indexes. The objects the linker
 ; decided to include in the link should be emitted into the file specified
-; after 'thinlto-index-only='. In this version of the test, only %t.o will
+; after 'thinlto-index-only='. In this version of the test, only %t1.o will
 ; be included in the link, and not %t2.o since it is within
 ; a library (--start-lib/--end-lib pair) and not strongly referenced.
 ; Note that the support for detecting this is in gold v1.12.
-; RUN: rm -f %t.o.thinlto.bc
-; RUN: rm -f %t2.o.thinlto.bc
-; RUN: rm -f %t.o.imports
-; RUN: rm -f %t2.o.imports
 ; RUN: %gold -plugin %llvmshlibdir/LLVMgold%shlibext \
 ; RUN:    --plugin-opt=thinlto \
-; RUN:    --plugin-opt=thinlto-index-only=%t3 \
+; RUN:    --plugin-opt=thinlto-index-only=%t.index \
 ; RUN:    --plugin-opt=thinlto-emit-imports-files \
 ; RUN:    -m elf_x86_64 \
 ; RUN:    -o %t4 \
-; RUN:    %t.o \
+; RUN:    %t1.o %t3.o \
 ; RUN:    --start-lib %t2.o --end-lib
 
 ; Ensure that the expected output files are created, even for the file
 ; the linker decided not to include in the link.
-; RUN: ls %t.o.thinlto.bc
+; RUN: ls %t1.o.thinlto.bc
 ; RUN: ls %t2.o.thinlto.bc
-; RUN: ls %t.o.imports
+; RUN: ls %t3.o.thinlto.bc
+; RUN: ls %t1.o.imports
 ; RUN: ls %t2.o.imports
+; RUN: ls %t3.o.imports
 
-; RUN: cat %t3 | FileCheck %s
+; RUN: cat %t.index | FileCheck %s
-; CHECK: thinlto_emit_linked_objects.ll.tmp.o
+; CHECK: thinlto_emit_linked_objects.ll.tmp1.o
 ; CHECK-NOT: thinlto_emit_linked_objects.ll.tmp2.o
+; CHECK-NOT: thinlto_emit_linked_objects.ll.tmp3.o
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"

llvm/tools/gold/gold-plugin.cpp

@@ -750,7 +750,7 @@ static void getThinLTOOldAndNewPrefix(std::string &OldPrefix,
   std::tie(OldPrefix, NewPrefix) = PrefixReplace.split(';');
 }
 
-static std::unique_ptr<LTO> createLTO() {
+static std::unique_ptr<LTO> createLTO(IndexWriteCallback OnIndexWrite) {
   Config Conf;
   ThinBackend Backend;
 
@@ -777,7 +777,7 @@ static std::unique_ptr<LTO> createLTO() {
     getThinLTOOldAndNewPrefix(OldPrefix, NewPrefix);
     Backend = createWriteIndexesThinBackend(
         OldPrefix, NewPrefix, options::thinlto_emit_imports_files,
-        options::thinlto_linked_objects_file);
+        options::thinlto_linked_objects_file, OnIndexWrite);
   }
 
   Conf.OverrideTriple = options::triple;
@@ -826,9 +826,9 @@ static std::unique_ptr<LTO> createLTO() {
 // final link. Frequently the distributed build system will want to
 // confirm that all expected outputs are created based on all of the
 // modules provided to the linker.
-static void writeEmptyDistributedBuildOutputs(std::string &ModulePath,
+static void writeEmptyDistributedBuildOutputs(const std::string &ModulePath,
-                                              std::string &OldPrefix,
+                                              const std::string &OldPrefix,
-                                              std::string &NewPrefix) {
+                                              const std::string &NewPrefix) {
   std::string NewModulePath =
       getThinLTOOutputFile(ModulePath, OldPrefix, NewPrefix);
   std::error_code EC;
@@ -865,7 +865,13 @@ static ld_plugin_status allSymbolsReadHook() {
   // through Lto->run().
   DenseMap<void *, std::unique_ptr<PluginInputFile>> HandleToInputFile;
 
-  std::unique_ptr<LTO> Lto = createLTO();
+  // Owns string objects and tells if index file was already created.
+  StringMap<bool> ObjectToIndexFileState;
+
+  std::unique_ptr<LTO> Lto =
+      createLTO([&ObjectToIndexFileState](const std::string &Identifier) {
+        ObjectToIndexFileState[Identifier] = true;
+      });
 
   std::string OldPrefix, NewPrefix;
   if (options::thinlto_index_only)
@@ -873,29 +879,20 @@ static ld_plugin_status allSymbolsReadHook() {
 
   std::string OldSuffix, NewSuffix;
   getThinLTOOldAndNewSuffix(OldSuffix, NewSuffix);
-  // Set for owning string objects used as buffer identifiers.
-  StringSet<> ObjectFilenames;
 
   for (claimed_file &F : Modules) {
     if (options::thinlto && !HandleToInputFile.count(F.leader_handle))
       HandleToInputFile.insert(std::make_pair(
           F.leader_handle, llvm::make_unique<PluginInputFile>(F.handle)));
-    const void *View = getSymbolsAndView(F);
     // In case we are thin linking with a minimized bitcode file, ensure
     // the module paths encoded in the index reflect where the backends
     // will locate the full bitcode files for compiling/importing.
     std::string Identifier =
         getThinLTOObjectFileName(F.name, OldSuffix, NewSuffix);
-    auto ObjFilename = ObjectFilenames.insert(Identifier);
+    auto ObjFilename = ObjectToIndexFileState.insert({Identifier, false});
     assert(ObjFilename.second);
-    if (!View) {
+    if (const void *View = getSymbolsAndView(F))
-      if (options::thinlto_index_only)
+      addModule(*Lto, F, View, ObjFilename.first->first());
-        // Write empty output files that may be expected by the distributed
-        // build system.
-        writeEmptyDistributedBuildOutputs(Identifier, OldPrefix, NewPrefix);
-      continue;
-    }
-    addModule(*Lto, F, View, ObjFilename.first->first());
   }
 
   SmallString<128> Filename;
@@ -932,6 +929,14 @@ static ld_plugin_status allSymbolsReadHook() {
 
   check(Lto->run(AddStream, Cache));
 
+  // Write empty output files that may be expected by the distributed build
+  // system.
+  if (options::thinlto_index_only)
+    for (auto &Identifier : ObjectToIndexFileState)
+      if (!Identifier.getValue())
+        writeEmptyDistributedBuildOutputs(Identifier.getKey(), OldPrefix,
+                                          NewPrefix);
+
   if (options::TheOutputType == options::OT_DISABLE ||
       options::TheOutputType == options::OT_BC_ONLY)
     return LDPS_OK;

llvm/tools/llvm-lto2/llvm-lto2.cpp

@@ -243,7 +243,7 @@ static int run(int argc, char **argv) {
 
   ThinBackend Backend;
   if (ThinLTODistributedIndexes)
-    Backend = createWriteIndexesThinBackend("", "", true, "");
+    Backend = createWriteIndexesThinBackend("", "", true, "", {});
   else
     Backend = createInProcessThinBackend(Threads);
   LTO Lto(std::move(Conf), std::move(Backend));