Add support for thinlto option to emit import files for thinlink

[rdhindsa]

Summary

Add support for thinlto option ( thinlto-emit-imports-files) to emit import files for thinlink.

[ruiu]

lld/ELF/LTO.cpp

102 ↗

(On Diff #145057)

I don't understand this part of code. I think OS will be closed at end of this if block, so I don't think you are writing anything to the file. Is this code correct?

[rdhindsa]

lld/ELF/LTO.cpp

102 ↗

(On Diff #145057)

For all the bitcode files, empty import files are generated at first. For the files selected by the linker having summary, lto::createWriteIndexesThinBackend overwrites the imports files . However, for the files which are either not selected by linker or they don't have summary, we don't write anything to .imports file since they don't import anything. These files are required for distributed ThinLTO build file staging.

[rdhindsa]

Added test to ensure that lld doesn't generate import files when thinlto-index-only is not enabled

[ruiu]

lld/ELF/LTO.cpp

105 ↗

(On Diff #145084)

failed to write -> cannot open

for consistency with other error messages.

102 ↗

(On Diff #145057)

Got it. Let's factor it out as a separate function

static void createEmptyFile(const Twine &Path);

that creates an empty file at a given path. That should make things clear.

[rdhindsa]

Added createEmptyFile function to create empty import files. Also updated the test with error description.

[rdhindsa]

I need to make similar change while creating empty index files, along with updating the description in test for it. I will add that change in next patch if that is okay.

[rdhindsa]

I will be sending another patch to address tejohnson's concerns. I think, in the meantime, we can add emit-imports option support. Or would you like me to send that patch first?

[rdhindsa]

Rebased diff

[ruiu]

I'm sorry but could you rebase again?

[rdhindsa]

Rebased the diff

[ruiu]

lld/ELF/LTO.cpp

173 ↗

(On Diff #145574)

nit: remove {}

277 ↗

(On Diff #145574)

OutputFileName seems a bit inaccurate as a variable name because it is not an output file name but a prefix. I'd just name Path.

287 ↗

(On Diff #145574)

remove {}

[ruiu]

LGTM

lld/trunk/ELF/Config.h

struct Configuration {
   bool Relocatable;
   bool SaveTemps;
   bool SingleRoRx;
   bool Shared;
   bool Static = false;
   bool SysvHash = false;
   bool Target1Rel;
   bool Trace;
+  bool ThinLTOEmitImportsFiles;
   bool ThinLTOIndexOnly;
   bool UndefinedVersion;
   bool WarnBackrefs;
   bool WarnCommon;
   bool WarnMissingEntry;
   bool WarnSymbolOrdering;
   bool WriteAddends;
   bool ZCombreloc;

lld/trunk/ELF/Driver.cpp

if (S == "disable-verify") {
       Config->LTODebugPassManager = true;
     } else if (S.startswith("sample-profile=")) {
       Config->LTOSampleProfile = S.substr(15);
     } else if (S == "thinlto-index-only") {
       Config->ThinLTOIndexOnly = true;
     } else if (S.startswith("thinlto-index-only=")) {
       Config->ThinLTOIndexOnly = true;
       Config->ThinLTOIndexOnlyObjectsFile = S.substr(19);
+    } else if (S == "thinlto-emit-imports-files") {
+      Config->ThinLTOEmitImportsFiles = true;
     } else if (S.startswith("thinlto-prefix-replace=")) {
       std::tie(Config->ThinLTOPrefixReplace.first,
                Config->ThinLTOPrefixReplace.second) = S.substr(23).split(';');
       if (Config->ThinLTOPrefixReplace.second.empty())
         error("thinlto-prefix-replace expects 'old;new' format");
     } else if (!S.startswith("/") && !S.startswith("-fresolution=") &&
                !S.startswith("-pass-through=") && !S.startswith("thinlto")) {
       parseClangOption(S, Arg->getSpelling());

lld/trunk/ELF/LTO.cpp

if (EC) {
     return nullptr;
   }
   return Ret;
 }
 
 static std::string getThinLTOOutputFile(StringRef ModulePath) {
   return lto::getThinLTOOutputFile(ModulePath,
                                    Config->ThinLTOPrefixReplace.first,
-                                   Config->ThinLTOPrefixReplace.second) +
-         ".thinlto.bc";
+                                   Config->ThinLTOPrefixReplace.second);
 }
 
 // Initializes IndexFile, Backend and LTOObj members.
 void BitcodeCompiler::init() {
   lto::Config Conf;
 
   // LLD supports the new relocations.
   Conf.Options = InitTargetOptionsFromCodeGenFlags();
void BitcodeCompiler::init() {
   lto::ThinBackend Backend;
   if (Config->ThinLTOJobs != -1u)
     Backend = lto::createInProcessThinBackend(Config->ThinLTOJobs);
 
   if (Config->ThinLTOIndexOnly) {
     IndexFile = openFile(Config->ThinLTOIndexOnlyObjectsFile);
     Backend = lto::createWriteIndexesThinBackend(
         Config->ThinLTOPrefixReplace.first, Config->ThinLTOPrefixReplace.second,
-        true, IndexFile.get(), nullptr);
+        Config->ThinLTOEmitImportsFiles, IndexFile.get(), nullptr);
   }
 
   Conf.SampleProfile = Config->LTOSampleProfile;
   Conf.UseNewPM = Config->LTONewPassManager;
   Conf.DebugPassManager = Config->LTODebugPassManager;
 
   LTOObj = llvm::make_unique<lto::LTO>(std::move(Conf), Backend,
                                        Config->LTOPartitions);
static void undefine(Symbol *S) {
   replaceSymbol<Undefined>(S, nullptr, S->getName(), STB_GLOBAL, STV_DEFAULT,
                            S->Type);
 }
 
 void BitcodeCompiler::add(BitcodeFile &F) {
   lto::InputFile &Obj = *F.Obj;
 
   // Create the empty files which, if indexed, will be overwritten later.
-  if (Config->ThinLTOIndexOnly)
-    openFile(getThinLTOOutputFile(Obj.getName()));
+  if (Config->ThinLTOIndexOnly) {
+    std::string Path = getThinLTOOutputFile(Obj.getName());
+    openFile(Path + ".thinlto.bc");
+
+    if (Config->ThinLTOEmitImportsFiles)
+      openFile(Path + ".imports");
+  }
 
   unsigned SymNum = 0;
   std::vector<Symbol *> Syms = F.getSymbols();
   std::vector<lto::SymbolResolution> Resols(Syms.size());
 
   bool IsExecutable = !Config->Shared && !Config->Relocatable;
 
   // Provide a resolution to the LTO API for each symbol.
std::vector<InputFile *> BitcodeCompiler::compile() {
   // If LazyObjFile has not been added to link, emit empty index files.
   // This is needed because this is what GNU gold plugin does and we have a
   // distributed build system that depends on that behavior.
   if (Config->ThinLTOIndexOnly) {
     for (LazyObjFile *F : LazyObjFiles) {
       if (F->AddedToLink || !isBitcode(F->MB))
         continue;
 
-      std::unique_ptr<raw_fd_ostream> OS =
-          openFile(getThinLTOOutputFile(F->getName()));
+      std::string Path = getThinLTOOutputFile(F->getName());
+      std::unique_ptr<raw_fd_ostream> OS = openFile(Path + ".thinlto.bc");
       if (!OS)
         continue;
 
       ModuleSummaryIndex M(false);
       M.setSkipModuleByDistributedBackend();
       WriteIndexToFile(M, *OS);
+
+      if (Config->ThinLTOEmitImportsFiles)
+        openFile(Path + ".imports");
     }
 
     // ThinLTO with index only option is required to generate only the index
     // files. After that, we exit from linker and ThinLTO backend runs in a
     // distributed environment.
     if (IndexFile)
       IndexFile->close();
     return {};
   }
 
   for (std::unique_ptr<MemoryBuffer> &File : Files)
     if (File)
       Ret.push_back(createObjectFile(*File));
   return Ret;
 }

lld/trunk/test/ELF/lto/thinlto_emit_imports.ll

+; REQUIRES: x86
+
+; Generate summary sections and test lld handling.
+; RUN: opt -module-summary %s -o %t.o
+; RUN: opt -module-summary %p/Inputs/thinlto.ll -o %t2.o
+
+; Include a file with an empty module summary index, to ensure that the expected
+; output files are created regardless, for a distributed build system.
+; RUN: opt -module-summary %p/Inputs/thinlto_empty.ll -o %t3.o
+
+; Ensure lld generates imports files if requested for distributed backends.
+; RUN: rm -f %t3.o.imports %t3.o.thinlto.bc
+; RUN: ld.lld --plugin-opt=thinlto-index-only --plugin-opt=thinlto-emit-imports-files -shared %t.o %t2.o %t3.o -o %t4
+
+; 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: 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
+
+; The imports file for Input/thinlto_empty.ll is empty but should exist.
+; RUN: cat %t3.o.imports | count 0
+
+; The index file should be created even for the input with an empty summary.
+; RUN: ls %t3.o.thinlto.bc
+
+; Ensure lld generates error if unable to write to imports file.
+; RUN: rm -f %t3.o.imports
+; RUN: touch %t3.o.imports
+; RUN: chmod 400 %t3.o.imports
+; RUN: not ld.lld -m elf_x86_64 --plugin-opt=thinlto-index-only --plugin-opt=thinlto-emit-imports-files -shared %t.o %t2.o %t3.o -o %t4 2>&1 | FileCheck %s --check-prefix=ERR
+; ERR: cannot open {{.*}}3.o.imports: {{P|p}}ermission denied
+
+; Ensure lld doesn't generate import files when thinlto-index-only is not enabled
+; RUN: rm -f %t.o.imports
+; RUN: rm -f %t2.o.imports
+; RUN: rm -f %t3.o.imports
+; RUN: ld.lld -m elf_x86_64 --plugin-opt=thinlto-emit-imports-files -shared %t.o %t2.o %t3.o -o %t4
+; RUN: not ls %t.o.imports
+; RUN: not ls %t2.o.imports
+; RUN: not ls %t4.o.imports
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @g(...)
+
+define void @f() {
+entry:
+  call void (...) @g()
+  ret void
+}