[ThinLTO] Always create linked objects file for --thinlto-index-only=

[vitalybuka]

Summary

ThinLTO indexing may decide to skip all objects. If we don't write something to
the list build system may consider this as failure or linker can reuse a file
from the previews build.

[tejohnson]

llvm/include/llvm/LTO/LTO.h

219

While here, can you add the missing documentation for this parameter?

llvm/lib/LTO/LTO.cpp

1037

We only have one backend object - why use a shared_ptr instead of a unique_ptr?

llvm/tools/gold/gold-plugin.cpp

739

s/return/returns/

742

Suggest changing order to do early return of nullptr if filename is empty.

[vitalybuka]

addressed comments

[vitalybuka]

llvm/lib/LTO/LTO.cpp

1037

No easy way to capture unique_ptr and copy that into std::function in c++11.
I can avoid transferring stream ownership. But to my taste shared_ptr version was better.

[tejohnson]

LGTM

llvm/lib/LTO/LTO.cpp

1037

I see, thanks for the explanation. I think I prefer then keeping the ownership in the client as you have done here.

llvm/tools/gold/gold-plugin.cpp

756

s/OnWrite/OnIndexWrite/

[vitalybuka]

fixed doc string

llvm/include/llvm/LTO/LTO.h

 /// To find the path to write the index to, the backend checks if the path has a
 /// prefix of OldPrefix; if so, it replaces that prefix with NewPrefix. It then
 /// 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,
-                                          IndexWriteCallback OnWrite);
+ThinBackend
+createWriteIndexesThinBackend(std::string OldPrefix, std::string NewPrefix,
+                              std::unique_ptr<raw_fd_ostream> LinkedObjectsFile,
 
 /// This class implements a resolution-based interface to LLVM's LTO
 /// functionality. It supports regular LTO, parallel LTO code generation and
 /// ThinLTO. You can use it from a linker in the following way:
 /// - Set hooks and code generation options (see lto::Config struct defined in
 ///   Config.h), and use the lto::Config object to create an lto::LTO object.
 /// - Create lto::InputFile objects using lto::InputFile::create(), then use
 ///   the symbols() function to enumerate its symbols and compute a resolution

llvm/lib/LTO/LTO.cpp

std::string lto::getThinLTOOutputFile(const std::string &Path,
   }
   return NewPath.str();
 }
 
 namespace {
 class WriteIndexesThinBackend : public ThinBackendProc {
   std::string OldPrefix, NewPrefix;
   bool ShouldEmitImportsFiles;
-
-  std::string LinkedObjectsFileName;
-  std::unique_ptr<llvm::raw_fd_ostream> LinkedObjectsFile;
-
+  std::shared_ptr<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, lto::IndexWriteCallback OnWrite)
+      std::shared_ptr<raw_fd_ostream> LinkedObjectsFile,
+      lto::IndexWriteCallback OnWrite)
       : ThinBackendProc(Conf, CombinedIndex, ModuleToDefinedGVSummaries),
         OldPrefix(OldPrefix), NewPrefix(NewPrefix),
         ShouldEmitImportsFiles(ShouldEmitImportsFiles),
-        LinkedObjectsFileName(LinkedObjectsFileName), OnWrite(OnWrite) {}
+        LinkedObjectsFile(std::move(LinkedObjectsFile)), OnWrite(OnWrite) {}
 
   Error start(
       unsigned Task, BitcodeModule BM,
       const FunctionImporter::ImportMapTy &ImportList,
       const FunctionImporter::ExportSetTy &ExportList,
       const std::map<GlobalValue::GUID, GlobalValue::LinkageTypes> &ResolvedODR,
       MapVector<StringRef, BitcodeModule> &ModuleMap) override {
     StringRef ModulePath = BM.getModuleIdentifier();
     std::string NewModulePath =
         getThinLTOOutputFile(ModulePath, OldPrefix, NewPrefix);
 
-    std::error_code EC;
-    if (!LinkedObjectsFileName.empty()) {
-      if (!LinkedObjectsFile) {
-        LinkedObjectsFile = llvm::make_unique<raw_fd_ostream>(
-            LinkedObjectsFileName, EC, sys::fs::OpenFlags::F_None);
-        if (EC)
-          return errorCodeToError(EC);
-      }
+    if (LinkedObjectsFile)
       *LinkedObjectsFile << NewModulePath << '\n';
-    }
 
     std::map<std::string, GVSummaryMapTy> ModuleToSummariesForIndex;
     gatherImportedSummariesForModule(ModulePath, ModuleToDefinedGVSummaries,
                                      ImportList, ModuleToSummariesForIndex);
 
+    std::error_code EC;
     raw_fd_ostream OS(NewModulePath + ".thinlto.bc", EC,
                       sys::fs::OpenFlags::F_None);
     if (EC)
       return errorCodeToError(EC);
     WriteIndexToFile(CombinedIndex, OS, &ModuleToSummariesForIndex);
 
     if (ShouldEmitImportsFiles) {
       EC = EmitImportsFiles(ModulePath, NewModulePath + ".imports", ImportList);
       if (EC)
         return errorCodeToError(EC);
     }
 
     if (OnWrite)
       OnWrite(ModulePath);
     return Error::success();
   }
 
   Error wait() override { return Error::success(); }
 };
 } // end anonymous namespace
 
-ThinBackend lto::createWriteIndexesThinBackend(std::string OldPrefix,
-                                               std::string NewPrefix,
-                                               bool ShouldEmitImportsFiles,
-                                               std::string LinkedObjectsFile,
-                                               IndexWriteCallback OnWrite) {
+ThinBackend lto::createWriteIndexesThinBackend(
+    std::string OldPrefix, std::string NewPrefix, bool ShouldEmitImportsFiles,
+    std::unique_ptr<raw_fd_ostream> LinkedObjectsFile,
+  std::shared_ptr<raw_fd_ostream> LinkedObjectsFileShared =
+      std::move(LinkedObjectsFile);
   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, OnWrite);
+        ShouldEmitImportsFiles, LinkedObjectsFileShared, OnWrite);
   };
 }
 
 Error LTO::runThinLTO(AddStreamFn AddStream, NativeObjectCache Cache) {
   if (ThinLTO.ModuleMap.empty())
     return Error::success();
 
   if (Conf.CombinedIndexHook && !Conf.CombinedIndexHook(ThinLTO.CombinedIndex))

llvm/test/tools/gold/X86/thinlto_no_objects.ll

+; Check that thinlto-index-only= always creates linked objects file, even
+; if nothing to add there.
+
+; Non-ThinLTO file should not get into list of linked objects.
+; RUN: opt %s -o %t.o
+
+; RUN: rm -f %t3
+; RUN: %gold -plugin %llvmshlibdir/LLVMgold%shlibext \
+; RUN:    --plugin-opt=thinlto \
+; RUN:    --plugin-opt=thinlto-index-only=%t3 \
+; RUN:    -o %t5 \
+; RUN:    %t.o
+
+; RUN: cat %t3 | count 0
+
+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

 /// \p NewPrefix strings, if it was specified.
 static void getThinLTOOldAndNewPrefix(std::string &OldPrefix,
                                       std::string &NewPrefix) {
   StringRef PrefixReplace = options::thinlto_prefix_replace;
   assert(PrefixReplace.empty() || PrefixReplace.find(";") != StringRef::npos);
   std::tie(OldPrefix, NewPrefix) = PrefixReplace.split(';');
 }
 
+// Creates and return output stream with a list of object files for final
+// linking of distributed ThinLTO.
+static std::unique_ptr<raw_fd_ostream> CreateLinkedObjectsFile() {
+  if (!options::thinlto_linked_objects_file.empty()) {
+    std::error_code EC;
+    auto LinkedObjectsFile = llvm::make_unique<raw_fd_ostream>(
+        options::thinlto_linked_objects_file, EC, sys::fs::OpenFlags::F_None);
+    if (EC)
+      message(LDPL_FATAL, "Failed to create '%s': %s",
+              options::thinlto_linked_objects_file.c_str(),
+              EC.message().c_str());
+    return LinkedObjectsFile;
+  }
+  return nullptr;
+}
+
 static std::unique_ptr<LTO> createLTO(IndexWriteCallback OnIndexWrite) {
   Config Conf;
   ThinBackend Backend;
 
   Conf.CPU = options::mcpu;
   Conf.Options = InitTargetOptionsFromCodeGenFlags();
 
   // Disable the new X86 relax relocations since gold might not support them.
   // FIXME: Check the gold version or add a new option to enable them.
   Conf.Options.RelaxELFRelocations = false;
 
   // Enable function/data sections by default.
   Conf.Options.FunctionSections = true;
   Conf.Options.DataSections = true;
 
   Conf.MAttrs = MAttrs;
   Conf.RelocModel = RelocationModel;
   Conf.DisableVerify = options::DisableVerify;
   Conf.OptLevel = options::OptLevel;
   if (options::Parallelism)
     Backend = createInProcessThinBackend(options::Parallelism);
   if (options::thinlto_index_only) {
     std::string OldPrefix, NewPrefix;
     getThinLTOOldAndNewPrefix(OldPrefix, NewPrefix);
     Backend = createWriteIndexesThinBackend(
         OldPrefix, NewPrefix, options::thinlto_emit_imports_files,
-        options::thinlto_linked_objects_file, OnIndexWrite);
+        CreateLinkedObjectsFile(), OnIndexWrite);
   }
 
   Conf.OverrideTriple = options::triple;
   Conf.DefaultTriple = sys::getDefaultTargetTriple();
 
   Conf.DiagHandler = diagnosticHandler;
 
   switch (options::TheOutputType) {

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

static int run(int argc, char **argv) {
   if (FileType.getNumOccurrences())
     Conf.CGFileType = FileType;
 
   Conf.OverrideTriple = OverrideTriple;
   Conf.DefaultTriple = DefaultTriple;
 
   ThinBackend Backend;
   if (ThinLTODistributedIndexes)
-    Backend = createWriteIndexesThinBackend("", "", true, "", {});
+    Backend = createWriteIndexesThinBackend(/* OldPrefix */ "",
+                                            /* NewPrefix */ "",
+                                            /* ShouldEmitImportsFiles */ true,
+                                            /* LinkedObjectsFile */ nullptr,
+                                            /* OnWrite */ {});
   else
     Backend = createInProcessThinBackend(Threads);
   LTO Lto(std::move(Conf), std::move(Backend));
 
   bool HasErrors = false;
   for (std::string F : InputFilenames) {
     std::unique_ptr<MemoryBuffer> MB = check(MemoryBuffer::getFile(F), F);
     std::unique_ptr<InputFile> Input =