IRGen: When loading the main module in the distributed ThinLTO backend, look for the module containing the summary.

[mehdi_amini]

clang/include/clang/CodeGen/BackendUtil.h

51 ↗

(On Diff #85515)

Indentation seems strange?

clang/lib/CodeGen/CodeGenAction.cpp

841 ↗

(On Diff #85515)

Can you extract this function in a separate patch? I feel the diff could be a lot more contained here.

[tejohnson]

clang/lib/CodeGen/BackendUtil.cpp

867 ↗

(On Diff #85515)

Would it be better to have this in llvm (e.g. in BitcodeReader like getBitcodeModuleList), or do you anticipate that we will never need that functionality within llvm itself?

clang/lib/CodeGen/CodeGenAction.cpp

848 ↗

(On Diff #85515)

What should happen if we have a multi-module bitcode file but no ThinLTOIndexFile? Right now I think that would error (with or without this patch), right? Do we instead want to compile the non-ThinLTO module, or compile the ThinLTO module without ThinLTO (which is what would happen if you passed a single-module bitcode file to clang without -fthinlto-index).

clang/test/CodeGen/thinlto_backend.ll

26 ↗

(On Diff #85515)

Are we testing the right thing here? How do we know we have loaded the correct module when passed an empty index file which causes us to drop to non-ThinLTO compilation with -ignore-empty-index-file? Maybe that handling is later, but it isn't clear from this test - can you instead test using an index file that enables ThinLTO compilation?

[pcc]

clang/include/clang/CodeGen/BackendUtil.h

51 ↗

(On Diff #85515)

Yes, it's what clang-format does though.

clang/lib/CodeGen/BackendUtil.cpp

867 ↗

(On Diff #85515)

Maybe -- at this point I feel that we are shoehorning far too much thinlto backend functionality into clang and that we should think about making it into a separate tool. If we ever create such a tool, we can easily move this function there.

clang/lib/CodeGen/CodeGenAction.cpp

841 ↗

(On Diff #85515)

Done for this function as well as loadModule in r292970 and r292972.

848 ↗

(On Diff #85515)

Yes, we'd error out in that case, and I think that's fine. Basically at the moment we only support passing ThinLTO output to either the linker or the thin backend, and anything else is unsupported.

clang/test/CodeGen/thinlto_backend.ll

26 ↗

(On Diff #85515)

Done, and split out into a separate test.

[pcc]

Address review comments

[tejohnson]

LGTM

cfe/trunk/lib/CodeGen/CodeGenAction.cpp

static void BitcodeInlineAsmDiagHandler(const llvm::SMDiagnostic &SM,
   Diags->Report(DiagID).AddString("cannot compile inline asm");
 }
 
 std::unique_ptr<llvm::Module> CodeGenAction::loadModule(MemoryBufferRef MBRef) {
   CompilerInstance &CI = getCompilerInstance();
   SourceManager &SM = CI.getSourceManager();
 
   // For ThinLTO backend invocations, ensure that the context
-  // merges types based on ODR identifiers.
-  if (!CI.getCodeGenOpts().ThinLTOIndexFile.empty())
+  // merges types based on ODR identifiers. We also need to read
+  // the correct module out of a multi-module bitcode file.
+  if (!CI.getCodeGenOpts().ThinLTOIndexFile.empty()) {
     VMContext->enableDebugTypeODRUniquing();
 
+    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::unique_ptr<llvm::Module>> MOrErr =
+        BMOrErr->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;
 
   // Translate from the diagnostic info to the SourceManager location if
   // available.
   // TODO: Unify this with ConvertBackendLocation()
   SourceLocation Loc;

cfe/trunk/test/CMakeLists.txt

set(CLANG_TEST_PARAMS
   )
 
 if( NOT CLANG_BUILT_STANDALONE )
   list(APPEND CLANG_TEST_DEPS
     llvm-config
     FileCheck count not
     llc
     llvm-bcanalyzer
+    llvm-cat
     llvm-dis
     llvm-nm
     llvm-objdump
     llvm-profdata
     llvm-readobj
     llvm-symbolizer
     opt
     )

cfe/trunk/test/CodeGen/thinlto-multi-module.ll

+; RUN: opt -module-summary -o %t1.o %s
+; RUN: llvm-lto -thinlto -o %t %t1.o
+
+; RUN: opt -o %t2.o %S/Inputs/thinlto_backend.ll
+; RUN: llvm-cat -b -o %t1cat.o %t1.o %t2.o
+; RUN: cp %t1cat.o %t1.o
+; 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: U f2
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @f2()
+
+define void @f1() {
+  call void @f2()
+  ret void
+}