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

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added inline comments.


================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:867
 
+Expected<BitcodeModule> clang::FindThinLTOModule(MemoryBufferRef MBRef) {
+  Expected<std::vector<BitcodeModule>> BMsOrErr = getBitcodeModuleList(MBRef);
----------------
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?


================
Comment at: clang/lib/CodeGen/CodeGenAction.cpp:848
+  // the correct module out of a multi-module bitcode file.
+  if (!CI.getCodeGenOpts().ThinLTOIndexFile.empty()) {
+    VMContext->enableDebugTypeODRUniquing();
----------------
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).


================
Comment at: clang/test/CodeGen/thinlto_backend.ll:26
+; RUN: llvm-cat -b -o %t5.o %t1.o %t2merge.o
+; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t6.o -x ir %t5.o -c -fthinlto-index=%t.empty.thinlto.bc -mllvm -ignore-empty-index-file
+; RUN: llvm-nm %t5.o | FileCheck --check-prefix=CHECK-OBJ-IGNORE-EMPTY %s
----------------
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?


https://reviews.llvm.org/D29067