[ThinLTO] Ignore object files with no ThinLTO modules if -fthinlto-index= is set

[vitalybuka]

Summary

ThinLTO compilation may decide not to split module and keep at as regular LTO.
In this can this module already processed during indexing and already a part of
merged object file. So here we can just skip it.

[tejohnson]

LGTM (minor comment fix in test needed)

clang/test/CodeGen/thinlto_backend.ll

23 ↗

(On Diff #131936)

Comment line length too long, and sentence isn't complete.

[pcc]

This doesn't seem right to me. In a mixed full/thin LTO link the full LTO module would be compiled during the indexing phase. We don't want to compile it again in the backend as it could lead at best to duplicate symbol errors and at worst to miscompiles of llvm.type.* intrinsic calls.

[tejohnson]

In D42680#991938, @pcc wrote:

This doesn't seem right to me. In a mixed full/thin LTO link the full LTO module would be compiled during the indexing phase. We don't want to compile it again in the backend as it could lead at best to duplicate symbol errors and at worst to miscompiles of llvm.type.* intrinsic calls.

Good point. The build system will presumably expect an output file to be generated, so probably just compile an "empty" Module in this case?

[vitalybuka]

skip full LTO objects

[vitalybuka]

comment

[vitalybuka]

PTAL

[tejohnson]

This looks ok to me, assuming it produces the empty output file as I expect it should. Please wait for pcc to take a look as well.

clang/lib/CodeGen/CodeGenAction.cpp

959 ↗

(On Diff #132702)

It looks like this should result in an empty .o file being produced, but please confirm in the test (otherwise the build system may complain that expected output not produced).

clang/test/CodeGen/thinlto_backend.ll

24 ↗

(On Diff #132702)

comment isn't complete (ends in "which").

26 ↗

(On Diff #132702)

Check that empty $t4.o is produced.

[vitalybuka]

empty file test

[vitalybuka]

Create valid empty object file

[vitalybuka]

set triple

[vitalybuka]

ping

[vitalybuka]

fix rebase issues introduced in the last patch

[vitalybuka]

@pcc Could you please take a look?

[pcc]

LGTM

clang/test/CodeGen/thinlto_backend.ll

27 ↗

(On Diff #133966)

llvm-nm %t4.o | count 0

[vitalybuka]

use count 0

cfe/trunk/include/clang/CodeGen/BackendUtil.h

void EmitBackendOutput(DiagnosticsEngine &Diags, const HeaderSearchOptions &,
                          BackendAction Action,
                          std::unique_ptr<raw_pwrite_stream> OS);
 
   void EmbedBitcode(llvm::Module *M, const CodeGenOptions &CGOpts,
                     llvm::MemoryBufferRef Buf);
 
   llvm::Expected<llvm::BitcodeModule>
   FindThinLTOModule(llvm::MemoryBufferRef MBRef);
+  llvm::BitcodeModule *
+  FindThinLTOModule(llvm::MutableArrayRef<llvm::BitcodeModule> BMs);
 }
 
 #endif

cfe/trunk/lib/CodeGen/BackendUtil.cpp

 
 Expected<BitcodeModule> clang::FindThinLTOModule(MemoryBufferRef MBRef) {
   Expected<std::vector<BitcodeModule>> BMsOrErr = getBitcodeModuleList(MBRef);
   if (!BMsOrErr)
     return BMsOrErr.takeError();
 
   // The bitcode file may contain multiple modules, we want the one that is
   // marked as being the ThinLTO module.
-  for (BitcodeModule &BM : *BMsOrErr) {
-    Expected<BitcodeLTOInfo> LTOInfo = BM.getLTOInfo();
-    if (LTOInfo && LTOInfo->IsThinLTO)
-      return BM;
-  }
+  if (const BitcodeModule *Bm = FindThinLTOModule(*BMsOrErr))
+    return *Bm;
 
   return make_error<StringError>("Could not find module summary",
                                  inconvertibleErrorCode());
 }
 
+BitcodeModule *clang::FindThinLTOModule(MutableArrayRef<BitcodeModule> BMs) {
+  for (BitcodeModule &BM : BMs) {
+    Expected<BitcodeLTOInfo> LTOInfo = BM.getLTOInfo();
+    if (LTOInfo && LTOInfo->IsThinLTO)
+      return &BM;
+  }
+  return nullptr;
+}
+
 static void runThinLTOBackend(ModuleSummaryIndex *CombinedIndex, Module *M,
                               const HeaderSearchOptions &HeaderOpts,
                               const CodeGenOptions &CGOpts,
                               const clang::TargetOptions &TOpts,
                               const LangOptions &LOpts,
                               std::unique_ptr<raw_pwrite_stream> OS,
                               std::string SampleProfile,
                               BackendAction Action) {

cfe/trunk/lib/CodeGen/CodeGenAction.cpp

auto DiagErrors = [&](Error E) -> std::unique_ptr<llvm::Module> {
       unsigned DiagID =
           CI.getDiagnostics().getCustomDiagID(DiagnosticsEngine::Error, "%0");
       handleAllErrors(std::move(E), [&](ErrorInfoBase &EIB) {
         CI.getDiagnostics().Report(DiagID) << EIB.message();
       });
       return {};
     };
 
-    Expected<llvm::BitcodeModule> BMOrErr = FindThinLTOModule(MBRef);
-    if (!BMOrErr)
-      return DiagErrors(BMOrErr.takeError());
-
+    Expected<std::vector<BitcodeModule>> BMsOrErr = getBitcodeModuleList(MBRef);
+    if (!BMsOrErr)
+      return DiagErrors(BMsOrErr.takeError());
+    BitcodeModule *Bm = FindThinLTOModule(*BMsOrErr);
+    // We have nothing to do if the file contains no ThinLTO module. This is
+    // possible if ThinLTO compilation was not able to split module. Content of
+    // the file was already processed by indexing and will be passed to the
+    // linker using merged object file.
+    if (!Bm) {
+      auto M = llvm::make_unique<llvm::Module>("empty", *VMContext);
+      M->setTargetTriple(CI.getTargetOpts().Triple);
+      return M;
+    }
     Expected<std::unique_ptr<llvm::Module>> MOrErr =
-        BMOrErr->parseModule(*VMContext);
+        Bm->parseModule(*VMContext);
     if (!MOrErr)
       return DiagErrors(MOrErr.takeError());
     return std::move(*MOrErr);
   }
 
   llvm::SMDiagnostic Err;
   if (std::unique_ptr<llvm::Module> M = parseIR(MBRef, Err, *VMContext))
     return M;

cfe/trunk/test/CodeGen/thinlto_backend.ll

 ; Ensure we ignore empty index file, and run non-ThinLTO compilation which
 ; would not import f2
 ; RUN: touch %t4.thinlto.bc
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t4.o -x ir %t1.o -c -fthinlto-index=%t4.thinlto.bc
 ; RUN: llvm-nm %t4.o | FileCheck --check-prefix=CHECK-OBJ-IGNORE-EMPTY %s
 ; CHECK-OBJ-IGNORE-EMPTY: T f1
 ; CHECK-OBJ-IGNORE-EMPTY: U f2
 
+; Ensure we don't fail with index and non-ThinLTO object file, and output must
+; be empty file.
+; RUN: opt -o %t5.o %s
+; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t4.o -x ir %t5.o -c -fthinlto-index=%t.thinlto.bc
+; RUN: llvm-nm %t4.o | count 0
+
 ; Ensure f2 was imported
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc
 ; RUN: llvm-nm %t3.o | FileCheck --check-prefix=CHECK-OBJ %s
 ; CHECK-OBJ: T f1
 ; CHECK-OBJ-NOT: U f2
 
 ; Ensure we get expected error for input files without summaries
 ; RUN: opt -o %t2.o %s