LTO: Add support for multi-module bitcode files.

[pcc]

Summary

Depends on D26951

[mehdi_amini]

clang/lib/CodeGen/BackendUtil.cpp

771 ↗

(On Diff #79959)

The error message could be a bit more specific here.

775 ↗

(On Diff #79959)

We load the first BitcodeModule, should we assert that this is the only one? How do we know this is the right one?

llvm/include/llvm/LTO/LTO.h

224 ↗

(On Diff #79959)

Since we have the two, it seems worth documenting these.

421 ↗

(On Diff #79959)

This is a pretty annoying API with ResI as an InOut parameter: at least document it.

llvm/lib/LTO/LTO.cpp

187 ↗

(On Diff #79959)

Document.

190 ↗

(On Diff #79959)

Add a comment to explain why it is out-of-line.

200 ↗

(On Diff #79959)

Some high level one or two sentences commenting what is going to happen below in the loop would be welcome I think.

212 ↗

(On Diff #79959)

Is there an expectation that no symbol can be duplicated in the various modules?

250 ↗

(On Diff #79959)

You can create an InputFile where we wouldn't find any module, if you don't want to support that we should detect it in the create() and return an error there. Otherwise this is UB to be called on a valid created InputFile

276 ↗

(On Diff #79959)

Add a comment to explain why it is out-of-line.

456 ↗

(On Diff #79959)

We'll arrive here from for (InputFile::InputModule &IM : Input->Mods) in LTO::add(), how are we distinguishing BM.getModuleIdentifier() when there are multiple modules per file?
I assume we don't expect multiple modules with a summary in the same input file, but in this case we should check here that the module has been inserted and error otherwise.

[pcc]

clang/lib/CodeGen/BackendUtil.cpp

775 ↗

(On Diff #79959)

I added some code that makes sure that we load the right module.

llvm/lib/LTO/LTO.cpp

212 ↗

(On Diff #79959)

If you mean that the modules may not both contain a definition of the symbol, then yes. The entity creating these bitcode files is expected to adhere to this policy. (The same applies to definitions of comdats, so the code on lines 215-221 below remains the same.)

It's possible to have a defined symbol in one module and a duplicate undefined symbol with the same name in another module; the situation is similar to module inline asm right now (i.e. PR30396). Whatever solution we come up with for that problem should also apply to this one.

250 ↗

(On Diff #79959)

Added a check to create().

456 ↗

(On Diff #79959)

BM.getModuleIdentifier() will be the same for each BitcodeModule in the same input file by construction, so we can just test the return value of insert here.

(I feel obliged to point out that this is yet another scenario where (path, byte slice) pairs would work better as a representation of ThinLTO modules.)

[pcc]

• Address review comments

[mehdi_amini]

clang/lib/CodeGen/BackendUtil.cpp

783 ↗

(On Diff #81322)

What if I.first was already present in the map? It seems the same case as below for ThinLTO where we should check that the returned value of insert().second .

llvm/lib/LTO/LTO.cpp

456 ↗

(On Diff #79959)

What is the other scenario? I don't remember what was specific to ThinLTO for this?

Also a pair path/byte slice doesn't fit the API in general, does it? We don't necessarily have a "file".
That said, instead of path the identifier could be a <BufferID, offset>, with BufferID intended to be a unique identifier provided by the client of the API per-buffer. I'm not sure how deep we'd have to thread this though. I think that as long as when we create a llvm::Module the identifier is consistent with whatever is used here, everything should be OK.

[mehdi_amini]

LGTM otherwise.

[pcc]

clang/lib/CodeGen/BackendUtil.cpp

783 ↗

(On Diff #81322)

That doesn't seem like it can happen. The range for loop starting on line 758 enumerates elements of a map, so each I.first() will be unique. We also break out of this loop as soon as we see a module with a summary.

llvm/lib/LTO/LTO.cpp

456 ↗

(On Diff #79959)

What is the other scenario? I don't remember what was specific to ThinLTO for this?

The main other scenario is (non-thin) archive files, which is already a problem as we don't handle that correctly in the distributed case, and we have workarounds in both the gold plugin and lld to give -save-temps temporaries appropriate names. This is ThinLTO-specific because we don't create temporaries for each input file under regular LTO.

Also a pair path/byte slice doesn't fit the API in general, does it? We don't necessarily have a "file".

True, but it's also the case that we don't necessarily have a "file" now and we're already passing paths around. I imagine that clients which don't use files could just make something up for the offsets as they would make something up for file paths.

That said, instead of path the identifier could be a <BufferID, offset>, with BufferID intended to be a unique identifier provided by the client of the API per-buffer.

I think that could work as well.

[mehdi_amini]

llvm/lib/LTO/LTO.cpp

456 ↗

(On Diff #79959)

In the C API (and thus in ld64), the expectation is that every input buffer has to be provided with a unique id. It happens that this ID is usually the path on file (with libFoo.a(member.o) when a static archive is involved).
Because the linker is supplying one member at a time, the unique ID in the API is enough.

Having multiple modules per file is new though and would break this "unique ID" provided by the linker, we could suffix it when loading each individual bitcode/module though.

The --save-temps is always using integer right now I believe (files are 1.thin.o, 2.thin.o, etc.).

True, but it's also the case that we don't necessarily have a "file" now and we're already passing paths around.

That's just us being inconsistent, I tried to express "ModuleID" instead of path as much as possible.

[pcc]

llvm/lib/LTO/LTO.cpp

456 ↗

(On Diff #79959)

Because the linker is supplying one member at a time, the unique ID in the API is enough.

I think @davide had a case where an archive had two members with the same name, and they were only distinguishable by offset.

The --save-temps is always using integer right now I believe (files are 1.thin.o, 2.thin.o, etc.).

Not always: http://llvm-cs.pcc.me.uk/lib/LTO/LTOBackend.cpp#77
gold and lld both pass UseInputModulePath == true here.

We probably want to overhaul how the filenames look for save-temps, but I think in the end they should contain the module ID in some form.

[davide]

Yes, thin-archivecollision.ll in lld/test/lto is an example, but it could be even worse because the two members can be part of the same archive. I find it weird that's possible to create an archive with two distinct members and the same name, but apparently ar consider this a legitimate operation.

[mehdi_amini]

llvm/lib/LTO/LTO.cpp

456 ↗

(On Diff #79959)

I think @davide had a case where an archive had two members with the same name, and they were only distinguishable by offset

I remember this, I wouldn't be too much concerned *here* with this and push to the client the responsibility to provide unique IDs. (Nothing prevent the linker to build an id that always include the offset for instance)

[pcc]

llvm/lib/LTO/LTO.cpp

456 ↗

(On Diff #79959)

Well, in the distributed case some component needs to understand the module IDs in order to distribute the work properly. Part of that may involve translating the module IDs into "file paths" (which may be actual file paths or conceptual ones). If we can arrange to use the same ("file path", offset) scheme in all linkers, that component can be shared between linkers.

But to a certain extent this is all hypothetical, I think I'd want to prototype before being sure that this is the right design.

cfe/trunk/lib/CodeGen/BackendUtil.cpp

static void runThinLTOBackend(const CodeGenOptions &CGOpts, Module *M,
 
   // FIXME: We could simply import the modules mentioned in the combined index
   // here.
   FunctionImporter::ImportMapTy ImportList;
   ComputeCrossModuleImportForModule(M->getModuleIdentifier(), *CombinedIndex,
                                     ImportList);
 
   std::vector<std::unique_ptr<llvm::MemoryBuffer>> OwnedImports;
-  MapVector<llvm::StringRef, llvm::MemoryBufferRef> ModuleMap;
+  MapVector<llvm::StringRef, llvm::BitcodeModule> ModuleMap;
 
   for (auto &I : ImportList) {
     ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> MBOrErr =
         llvm::MemoryBuffer::getFile(I.first());
     if (!MBOrErr) {
       errs() << "Error loading imported file '" << I.first()
              << "': " << MBOrErr.getError().message() << "\n";
       return;
     }
-    ModuleMap[I.first()] = (*MBOrErr)->getMemBufferRef();
+
+    Expected<std::vector<BitcodeModule>> BMsOrErr =
+        getBitcodeModuleList(**MBOrErr);
+    if (!BMsOrErr) {
+      handleAllErrors(BMsOrErr.takeError(), [&](ErrorInfoBase &EIB) {
+        errs() << "Error loading imported file '" << I.first()
+               << "': " << EIB.message() << '\n';
+      });
+      return;
+    }
+
+    // The bitcode file may contain multiple modules, we want the one with a
+    // summary.
+    bool FoundModule = false;
+    for (BitcodeModule &BM : *BMsOrErr) {
+      Expected<bool> HasSummary = BM.hasSummary();
+      if (HasSummary && *HasSummary) {
+        ModuleMap.insert({I.first(), BM});
+        FoundModule = true;
+        break;
+      }
+    }
+    if (!FoundModule) {
+      errs() << "Error loading imported file '" << I.first()
+             << "': Could not find module summary\n";
+      return;
+    }
+
     OwnedImports.push_back(std::move(*MBOrErr));
   }
   auto AddStream = [&](size_t Task) {
     return llvm::make_unique<lto::NativeObjectStream>(std::move(OS));
   };
   lto::Config Conf;
   if (Error E = thinBackend(
           Conf, 0, AddStream, *M, *CombinedIndex, ImportList,

cfe/trunk/test/CodeGen/thinlto_backend.ll

 ; REQUIRES: x86-registered-target
 
 ; RUN: opt -module-summary -o %t1.o %s
 ; RUN: opt -module-summary -o %t2.o %S/Inputs/thinlto_backend.ll
 ; RUN: llvm-lto -thinlto -o %t %t1.o %t2.o
 
 ; Ensure clang -cc1 give expected error for incorrect input type
 ; RUN: not %clang_cc1 -O2 -o %t1.o -x c %s -c -fthinlto-index=%t.thinlto.bc 2>&1 | FileCheck %s -check-prefix=CHECK-WARNING
 ; CHECK-WARNING: error: invalid argument '-fthinlto-index={{.*}}' only allowed with '-x ir'
 
 ; Ensure we get expected error for missing index file
-; RUN: %clang -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=bad.thinlto.bc 2>&1 | FileCheck %s -check-prefix=CHECK-ERROR
-; CHECK-ERROR: Error loading index file 'bad.thinlto.bc'
+; RUN: %clang -O2 -o %t4.o -x ir %t1.o -c -fthinlto-index=bad.thinlto.bc 2>&1 | FileCheck %s -check-prefix=CHECK-ERROR1
+; CHECK-ERROR1: Error loading index file 'bad.thinlto.bc'
 
 ; 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
+; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc
+; CHECK-ERROR2: Error loading imported file '{{.*}}': Could not find module summary
+
 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
 }

llvm/trunk/include/llvm/Bitcode/BitcodeReader.h

class BitcodeModule {
     getModuleImpl(LLVMContext &Context, bool MaterializeAll,
                   bool ShouldLazyLoadMetadata);
 
   public:
     StringRef getBuffer() const {
       return StringRef((const char *)Buffer.begin(), Buffer.size());
     }
 
+    StringRef getModuleIdentifier() const { return ModuleIdentifier; }
+
     /// Read the bitcode module and prepare for lazy deserialization of function
     /// bodies. If ShouldLazyLoadMetadata is true, lazily load metadata as well.
     Expected<std::unique_ptr<Module>>
     getLazyModule(LLVMContext &Context, bool ShouldLazyLoadMetadata);
 
     /// Read the entire bitcode module and return it.
     Expected<std::unique_ptr<Module>> parseModule(LLVMContext &Context);
 

llvm/trunk/include/llvm/LTO/LTO.h

 #include "llvm/Linker/IRMover.h"
 #include "llvm/Object/IRObjectFile.h"
 #include "llvm/Support/thread.h"
 #include "llvm/Target/TargetOptions.h"
 #include "llvm/Transforms/IPO/FunctionImport.h"
 
 namespace llvm {
 
+class BitcodeModule;
 class Error;
 class LLVMContext;
 class MemoryBufferRef;
 class Module;
 class Target;
 class raw_pwrite_stream;
 
 /// Resolve Weak and LinkOnce values in the \p Index. Linkage changes recorded

 /// information that an LTO client should need in order to do symbol resolution.
 class InputFile {
   // FIXME: Remove LTO class friendship once we have bitcode symbol tables.
   friend LTO;
   InputFile() = default;
 
   // FIXME: Remove the LLVMContext once we have bitcode symbol tables.
   LLVMContext Ctx;
+  struct InputModule;
+  std::vector<InputModule> Mods;
   ModuleSymbolTable SymTab;
-  std::unique_ptr<Module> Mod;
-  MemoryBufferRef MBRef;
 
   std::vector<StringRef> Comdats;
   DenseMap<const Comdat *, unsigned> ComdatMap;
 
 public:
+  ~InputFile();
+
   /// Create an InputFile.
   static Expected<std::unique_ptr<InputFile>> create(MemoryBufferRef Object);
 
   class symbol_iterator;
 
   /// This is a wrapper for ArrayRef<ModuleSymbolTable::Symbol>::iterator that
   /// exposes only the information that an LTO client should need in order to do
   /// symbol resolution.
public:
 
   /// A range over the symbols in this InputFile.
   iterator_range<symbol_iterator> symbols() {
     return llvm::make_range(
         symbol_iterator(SymTab.symbols().begin(), SymTab, this),
         symbol_iterator(SymTab.symbols().end(), SymTab, this));
   }
 
-  StringRef getSourceFileName() const { return Mod->getSourceFileName(); }
-  MemoryBufferRef getMemoryBufferRef() const { return MBRef; }
+  /// Returns the path to the InputFile.
+  StringRef getName() const;
+
+  /// Returns the source file path specified at compile time.
+  StringRef getSourceFileName() const;
 
   // Returns a table with all the comdats used by this file.
   ArrayRef<StringRef> getComdatTable() const { return Comdats; }
+
+private:
+  iterator_range<symbol_iterator> module_symbols(InputModule &IM);
 };
 
 /// This class wraps an output stream for a native object. Most clients should
 /// just be able to return an instance of this base class from the stream
 /// callback, but if a client needs to perform some action after the stream is
 /// written to, that can be done by deriving from this class and overriding the
 /// destructor.
 class NativeObjectStream {

 public:
   /// Create an LTO object. A default constructed LTO object has a reasonable
   /// production configuration, but you can customize it by passing arguments to
   /// this constructor.
   /// FIXME: We do currently require the DiagHandler field to be set in Conf.
   /// Until that is fixed, a Config argument is required.
   LTO(Config Conf, ThinBackend Backend = nullptr,
       unsigned ParallelCodeGenParallelismLevel = 1);
+  ~LTO();
 
   /// Add an input file to the LTO link, using the provided symbol resolutions.
   /// The symbol resolutions must appear in the enumeration order given by
   /// InputFile::symbols().
   Error add(std::unique_ptr<InputFile> Obj, ArrayRef<SymbolResolution> Res);
 
   /// Returns an upper bound on the number of tasks that the client may expect.
   /// This may only be called after all IR object files have been added. For a
struct RegularLTOState {
     std::unique_ptr<IRMover> Mover;
   } RegularLTO;
 
   struct ThinLTOState {
     ThinLTOState(ThinBackend Backend);
 
     ThinBackend Backend;
     ModuleSummaryIndex CombinedIndex;
-    MapVector<StringRef, MemoryBufferRef> ModuleMap;
+    MapVector<StringRef, BitcodeModule> ModuleMap;
     DenseMap<GlobalValue::GUID, StringRef> PrevailingModuleForGUID;
   } ThinLTO;
 
   // The global resolution for a particular (mangled) symbol name. This is in
   // particular necessary to track whether each symbol can be internalized.
   // Because any input file may introduce a new cross-partition reference, we
   // cannot make any final internalization decisions until all input files have
   // been added and the client has called run(). During run() we apply
private:
 
   // Global mapping from mangled symbol names to resolutions.
   StringMap<GlobalResolution> GlobalResolutions;
 
   void addSymbolToGlobalRes(SmallPtrSet<GlobalValue *, 8> &Used,
                             const InputFile::Symbol &Sym, SymbolResolution Res,
                             unsigned Partition);
 
-  Error addRegularLTO(std::unique_ptr<InputFile> Input,
-                      ArrayRef<SymbolResolution> Res);
-  Error addThinLTO(std::unique_ptr<InputFile> Input,
-                   ArrayRef<SymbolResolution> Res);
+  // These functions take a range of symbol resolutions [ResI, ResE) and consume
+  // the resolutions used by a single input module by incrementing ResI. After
+  // these functions return, [ResI, ResE) will refer to the resolution range for
+  // the remaining modules in the InputFile.
+  Error addModule(InputFile &Input, InputFile::InputModule &IM,
+                  const SymbolResolution *&ResI, const SymbolResolution *ResE);
+  Error addRegularLTO(BitcodeModule BM, const SymbolResolution *&ResI,
+                      const SymbolResolution *ResE);
+  Error addThinLTO(BitcodeModule BM, Module &M,
+                   iterator_range<InputFile::symbol_iterator> Syms,
+                   const SymbolResolution *&ResI, const SymbolResolution *ResE);
 
   Error runRegularLTO(AddStreamFn AddStream);
   Error runThinLTO(AddStreamFn AddStream, NativeObjectCache Cache,
                    bool HasRegularLTO);
 
   mutable bool CalledGetMaxTasks = false;
 };
 

llvm/trunk/include/llvm/LTO/LTOBackend.h

 #include "llvm/IR/ModuleSummaryIndex.h"
 #include "llvm/LTO/LTO.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Target/TargetOptions.h"
 #include "llvm/Transforms/IPO/FunctionImport.h"
 
 namespace llvm {
 
+class BitcodeModule;
 class Error;
 class Module;
 class Target;
 
 namespace lto {
 
 /// Runs a regular LTO backend.
 Error backend(Config &C, AddStreamFn AddStream,
               unsigned ParallelCodeGenParallelismLevel,
               std::unique_ptr<Module> M);
 
 /// Runs a ThinLTO backend.
 Error thinBackend(Config &C, unsigned Task, AddStreamFn AddStream, Module &M,
                   ModuleSummaryIndex &CombinedIndex,
                   const FunctionImporter::ImportMapTy &ImportList,
                   const GVSummaryMapTy &DefinedGlobals,
-                  MapVector<StringRef, MemoryBufferRef> &ModuleMap);
+                  MapVector<StringRef, BitcodeModule> &ModuleMap);
 }
 }
 
 #endif

llvm/trunk/lib/LTO/LTO.cpp

 // as external and non-exported values as internal.
 void llvm::thinLTOInternalizeAndPromoteInIndex(
     ModuleSummaryIndex &Index,
     function_ref<bool(StringRef, GlobalValue::GUID)> isExported) {
   for (auto &I : Index)
     thinLTOInternalizeAndPromoteGUID(I.second, I.first, isExported);
 }
 
+struct InputFile::InputModule {
+  BitcodeModule BM;
+  std::unique_ptr<Module> Mod;
+
+  // The range of ModuleSymbolTable entries for this input module.
+  size_t SymBegin, SymEnd;
+};
+
+// Requires a destructor for std::vector<InputModule>.
+InputFile::~InputFile() = default;
+
 Expected<std::unique_ptr<InputFile>> InputFile::create(MemoryBufferRef Object) {
   std::unique_ptr<InputFile> File(new InputFile);
 
   ErrorOr<MemoryBufferRef> BCOrErr =
       IRObjectFile::findBitcodeInMemBuffer(Object);
   if (!BCOrErr)
     return errorCodeToError(BCOrErr.getError());
-  File->MBRef = *BCOrErr;
 
+  Expected<std::vector<BitcodeModule>> BMsOrErr =
+      getBitcodeModuleList(*BCOrErr);
+  if (!BMsOrErr)
+    return BMsOrErr.takeError();
+
+  if (BMsOrErr->empty())
+    return make_error<StringError>("Bitcode file does not contain any modules",
+                                   inconvertibleErrorCode());
+
+  // Create an InputModule for each module in the InputFile, and add it to the
+  // ModuleSymbolTable.
+  for (auto BM : *BMsOrErr) {
     Expected<std::unique_ptr<Module>> MOrErr =
-      getLazyBitcodeModule(*BCOrErr, File->Ctx,
-                           /*ShouldLazyLoadMetadata*/ true);
+        BM.getLazyModule(File->Ctx, /*ShouldLazyLoadMetadata*/ true);
     if (!MOrErr)
       return MOrErr.takeError();
 
-  File->Mod = std::move(*MOrErr);
-  File->SymTab.addModule(File->Mod.get());
-
-  for (const auto &C : File->Mod->getComdatSymbolTable()) {
-    auto P =
-        File->ComdatMap.insert(std::make_pair(&C.second, File->Comdats.size()));
+    size_t SymBegin = File->SymTab.symbols().size();
+    File->SymTab.addModule(MOrErr->get());
+    size_t SymEnd = File->SymTab.symbols().size();
+
+    for (const auto &C : (*MOrErr)->getComdatSymbolTable()) {
+      auto P = File->ComdatMap.insert(
+          std::make_pair(&C.second, File->Comdats.size()));
       assert(P.second);
       (void)P;
       File->Comdats.push_back(C.first());
     }
 
+    File->Mods.push_back({BM, std::move(*MOrErr), SymBegin, SymEnd});
+  }
+
   return std::move(File);
 }
 
 Expected<int> InputFile::Symbol::getComdatIndex() const {
   if (!isGV())
     return -1;
   const GlobalObject *GO = getGV()->getBaseObject();
   if (!GO)
     return make_error<StringError>("Unable to determine comdat of alias!",
                                    inconvertibleErrorCode());
   if (const Comdat *C = GO->getComdat()) {
     auto I = File->ComdatMap.find(C);
     assert(I != File->ComdatMap.end());
     return I->second;
   }
   return -1;
 }
 
+StringRef InputFile::getName() const {
+  return Mods[0].BM.getModuleIdentifier();
+}
+
+StringRef InputFile::getSourceFileName() const {
+  return Mods[0].Mod->getSourceFileName();
+}
+
+iterator_range<InputFile::symbol_iterator>
+InputFile::module_symbols(InputModule &IM) {
+  return llvm::make_range(
+      symbol_iterator(SymTab.symbols().data() + IM.SymBegin, SymTab, this),
+      symbol_iterator(SymTab.symbols().data() + IM.SymEnd, SymTab, this));
+}
+
 LTO::RegularLTOState::RegularLTOState(unsigned ParallelCodeGenParallelismLevel,
                                       Config &Conf)
     : ParallelCodeGenParallelismLevel(ParallelCodeGenParallelismLevel),
       Ctx(Conf) {}
 
 LTO::ThinLTOState::ThinLTOState(ThinBackend Backend) : Backend(Backend) {
   if (!Backend)
     this->Backend =
         createInProcessThinBackend(llvm::heavyweight_hardware_concurrency());
 }
 
 LTO::LTO(Config Conf, ThinBackend Backend,
          unsigned ParallelCodeGenParallelismLevel)
     : Conf(std::move(Conf)),
       RegularLTO(ParallelCodeGenParallelismLevel, this->Conf),
       ThinLTO(std::move(Backend)) {}
 
+// Requires a destructor for MapVector<BitcodeModule>.
+LTO::~LTO() = default;
+
 // Add the given symbol to the GlobalResolutions map, and resolve its partition.
 void LTO::addSymbolToGlobalRes(SmallPtrSet<GlobalValue *, 8> &Used,
                                const InputFile::Symbol &Sym,
                                SymbolResolution Res, unsigned Partition) {
   GlobalValue *GV = Sym.isGV() ? Sym.getGV() : nullptr;
 
   auto &GlobalRes = GlobalResolutions[Sym.getName()];
   if (GV) {
     GlobalRes.UnnamedAddr &= GV->hasGlobalUnnamedAddr();
     if (Res.Prevailing)
       GlobalRes.IRName = GV->getName();
   }
   if (Res.VisibleToRegularObj || (GV && Used.count(GV)) ||
       (GlobalRes.Partition != GlobalResolution::Unknown &&
        GlobalRes.Partition != Partition))
     GlobalRes.Partition = GlobalResolution::External;
   else
     GlobalRes.Partition = Partition;
 }
 
 static void writeToResolutionFile(raw_ostream &OS, InputFile *Input,
                                   ArrayRef<SymbolResolution> Res) {
-  StringRef Path = Input->getMemoryBufferRef().getBufferIdentifier();
+  StringRef Path = Input->getName();
   OS << Path << '\n';
   auto ResI = Res.begin();
   for (const InputFile::Symbol &Sym : Input->symbols()) {
     assert(ResI != Res.end());
     SymbolResolution Res = *ResI++;
 
     OS << "-r=" << Path << ',' << Sym.getName() << ',';
     if (Res.Prevailing)

 
 Error LTO::add(std::unique_ptr<InputFile> Input,
                ArrayRef<SymbolResolution> Res) {
   assert(!CalledGetMaxTasks);
 
   if (Conf.ResolutionFile)
     writeToResolutionFile(*Conf.ResolutionFile, Input.get(), Res);
 
+  const SymbolResolution *ResI = Res.begin();
+  for (InputFile::InputModule &IM : Input->Mods)
+    if (Error Err = addModule(*Input, IM, ResI, Res.end()))
+      return Err;
+
+  assert(ResI == Res.end());
+  return Error::success();
+}
+
+Error LTO::addModule(InputFile &Input, InputFile::InputModule &IM,
+                     const SymbolResolution *&ResI,
+                     const SymbolResolution *ResE) {
   // FIXME: move to backend
-  Module &M = *Input->Mod;
+  Module &M = *IM.Mod;
   if (!Conf.OverrideTriple.empty())
     M.setTargetTriple(Conf.OverrideTriple);
   else if (M.getTargetTriple().empty())
     M.setTargetTriple(Conf.DefaultTriple);
 
-  Expected<bool> HasThinLTOSummary = hasGlobalValueSummary(Input->MBRef);
+  Expected<bool> HasThinLTOSummary = IM.BM.hasSummary();
   if (!HasThinLTOSummary)
     return HasThinLTOSummary.takeError();
 
   if (*HasThinLTOSummary)
-    return addThinLTO(std::move(Input), Res);
+    return addThinLTO(IM.BM, M, Input.module_symbols(IM), ResI, ResE);
   else
-    return addRegularLTO(std::move(Input), Res);
+    return addRegularLTO(IM.BM, ResI, ResE);
 }
 
 // Add a regular LTO object to the link.
-Error LTO::addRegularLTO(std::unique_ptr<InputFile> Input,
-                         ArrayRef<SymbolResolution> Res) {
+Error LTO::addRegularLTO(BitcodeModule BM, const SymbolResolution *&ResI,
+                         const SymbolResolution *ResE) {
   if (!RegularLTO.CombinedModule) {
     RegularLTO.CombinedModule =
         llvm::make_unique<Module>("ld-temp.o", RegularLTO.Ctx);
     RegularLTO.Mover = llvm::make_unique<IRMover>(*RegularLTO.CombinedModule);
   }
   Expected<std::unique_ptr<Module>> MOrErr =
-      getLazyBitcodeModule(Input->MBRef, RegularLTO.Ctx,
-                           /*ShouldLazyLoadMetadata*/ true);
+      BM.getLazyModule(RegularLTO.Ctx, /*ShouldLazyLoadMetadata*/ true);
   if (!MOrErr)
     return MOrErr.takeError();
 
   Module &M = **MOrErr;
   if (Error Err = M.materializeMetadata())
     return Err;
   UpgradeDebugInfo(M);
 
   ModuleSymbolTable SymTab;
   SymTab.addModule(&M);
 
   SmallPtrSet<GlobalValue *, 8> Used;
   collectUsedGlobalVariables(M, Used, /*CompilerUsed*/ false);
 
   std::vector<GlobalValue *> Keep;
 
   for (GlobalVariable &GV : M.globals())
     if (GV.hasAppendingLinkage())
       Keep.push_back(&GV);
 
-  auto ResI = Res.begin();
   for (const InputFile::Symbol &Sym :
        make_range(InputFile::symbol_iterator(SymTab.symbols().begin(), SymTab,
                                              nullptr),
                   InputFile::symbol_iterator(SymTab.symbols().end(), SymTab,
                                              nullptr))) {
-    assert(ResI != Res.end());
+    assert(ResI != ResE);
     SymbolResolution Res = *ResI++;
     addSymbolToGlobalRes(Used, Sym, Res, 0);
 
     if (Sym.getFlags() & object::BasicSymbolRef::SF_Undefined)
       continue;
     if (Res.Prevailing && Sym.isGV()) {
       GlobalValue *GV = Sym.getGV();
       Keep.push_back(GV);
if (Sym.getFlags() & object::BasicSymbolRef::SF_Common) {
       auto &CommonRes = RegularLTO.Commons[Sym.getGV()->getName()];
       CommonRes.Size = std::max(CommonRes.Size, Sym.getCommonSize());
       CommonRes.Align = std::max(CommonRes.Align, Sym.getCommonAlignment());
       CommonRes.Prevailing |= Res.Prevailing;
     }
 
     // FIXME: use proposed local attribute for FinalDefinitionInLinkageUnit.
   }
-  assert(ResI == Res.end());
 
   return RegularLTO.Mover->move(std::move(*MOrErr), Keep,
                                 [](GlobalValue &, IRMover::ValueAdder) {},
                                 /* LinkModuleInlineAsm */ true,
                                 /* IsPerformingImport */ false);
 }
 
 // Add a ThinLTO object to the link.
-Error LTO::addThinLTO(std::unique_ptr<InputFile> Input,
-                      ArrayRef<SymbolResolution> Res) {
-  Module &M = *Input->Mod;
+// FIXME: This function should not need to take as many parameters once we have
+// a bitcode symbol table.
+Error LTO::addThinLTO(BitcodeModule BM, Module &M,
+                      iterator_range<InputFile::symbol_iterator> Syms,
+                      const SymbolResolution *&ResI,
+                      const SymbolResolution *ResE) {
   SmallPtrSet<GlobalValue *, 8> Used;
   collectUsedGlobalVariables(M, Used, /*CompilerUsed*/ false);
 
-  MemoryBufferRef MBRef = Input->MBRef;
-  Expected<std::unique_ptr<object::ModuleSummaryIndexObjectFile>>
-      SummaryObjOrErr = object::ModuleSummaryIndexObjectFile::create(MBRef);
-  if (!SummaryObjOrErr)
-    return SummaryObjOrErr.takeError();
-  ThinLTO.CombinedIndex.mergeFrom((*SummaryObjOrErr)->takeIndex(),
+  Expected<std::unique_ptr<ModuleSummaryIndex>> SummaryOrErr = BM.getSummary();
+  if (!SummaryOrErr)
+    return SummaryOrErr.takeError();
+  ThinLTO.CombinedIndex.mergeFrom(std::move(*SummaryOrErr),
                                   ThinLTO.ModuleMap.size());
 
-  auto ResI = Res.begin();
-  for (const InputFile::Symbol &Sym : Input->symbols()) {
-    assert(ResI != Res.end());
+  for (const InputFile::Symbol &Sym : Syms) {
+    assert(ResI != ResE);
     SymbolResolution Res = *ResI++;
     addSymbolToGlobalRes(Used, Sym, Res, ThinLTO.ModuleMap.size() + 1);
 
     if (Res.Prevailing && Sym.isGV())
       ThinLTO.PrevailingModuleForGUID[Sym.getGV()->getGUID()] =
-          MBRef.getBufferIdentifier();
+          BM.getModuleIdentifier();
   }
-  assert(ResI == Res.end());
 
-  ThinLTO.ModuleMap[MBRef.getBufferIdentifier()] = MBRef;
+  if (!ThinLTO.ModuleMap.insert({BM.getModuleIdentifier(), BM}).second)
+    return make_error<StringError>(
+        "Expected at most one ThinLTO module per bitcode file",
+        inconvertibleErrorCode());
+
   return Error::success();
 }
 
 unsigned LTO::getMaxTasks() const {
   CalledGetMaxTasks = true;
   return RegularLTO.ParallelCodeGenParallelismLevel + ThinLTO.ModuleMap.size();
 }
 

 public:
   ThinBackendProc(Config &Conf, ModuleSummaryIndex &CombinedIndex,
                   const StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries)
       : Conf(Conf), CombinedIndex(CombinedIndex),
         ModuleToDefinedGVSummaries(ModuleToDefinedGVSummaries) {}
 
   virtual ~ThinBackendProc() {}
   virtual Error start(
-      unsigned Task, MemoryBufferRef MBRef,
+      unsigned Task, BitcodeModule BM,
       const FunctionImporter::ImportMapTy &ImportList,
       const FunctionImporter::ExportSetTy &ExportList,
       const std::map<GlobalValue::GUID, GlobalValue::LinkageTypes> &ResolvedODR,
-      MapVector<StringRef, MemoryBufferRef> &ModuleMap) = 0;
+      MapVector<StringRef, BitcodeModule> &ModuleMap) = 0;
   virtual Error wait() = 0;
 };
 
 namespace {
 class InProcessThinBackend : public ThinBackendProc {
   ThreadPool BackendThreadPool;
   AddStreamFn AddStream;
   NativeObjectCache Cache;
 
   Optional<Error> Err;
   std::mutex ErrMu;
 
 public:
   InProcessThinBackend(
       Config &Conf, ModuleSummaryIndex &CombinedIndex,
       unsigned ThinLTOParallelismLevel,
       const StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries,
       AddStreamFn AddStream, NativeObjectCache Cache)
       : ThinBackendProc(Conf, CombinedIndex, ModuleToDefinedGVSummaries),
         BackendThreadPool(ThinLTOParallelismLevel),
         AddStream(std::move(AddStream)), Cache(std::move(Cache)) {}
 
   Error runThinLTOBackendThread(
       AddStreamFn AddStream, NativeObjectCache Cache, unsigned Task,
-      MemoryBufferRef MBRef, ModuleSummaryIndex &CombinedIndex,
+      BitcodeModule BM, ModuleSummaryIndex &CombinedIndex,
       const FunctionImporter::ImportMapTy &ImportList,
       const FunctionImporter::ExportSetTy &ExportList,
       const std::map<GlobalValue::GUID, GlobalValue::LinkageTypes> &ResolvedODR,
       const GVSummaryMapTy &DefinedGlobals,
-      MapVector<StringRef, MemoryBufferRef> &ModuleMap) {
+      MapVector<StringRef, BitcodeModule> &ModuleMap) {
     auto RunThinBackend = [&](AddStreamFn AddStream) {
       LTOLLVMContext BackendContext(Conf);
-      Expected<std::unique_ptr<Module>> MOrErr =
-          parseBitcodeFile(MBRef, BackendContext);
+      Expected<std::unique_ptr<Module>> MOrErr = BM.parseModule(BackendContext);
       if (!MOrErr)
         return MOrErr.takeError();
 
       return thinBackend(Conf, Task, AddStream, **MOrErr, CombinedIndex,
                          ImportList, DefinedGlobals, ModuleMap);
     };
 
-    auto ModuleID = MBRef.getBufferIdentifier();
+    auto ModuleID = BM.getModuleIdentifier();
 
     if (!Cache || !CombinedIndex.modulePaths().count(ModuleID) ||
         all_of(CombinedIndex.getModuleHash(ModuleID),
                [](uint32_t V) { return V == 0; }))
       // Cache disabled or no entry for this module in the combined index or
       // no module hash.
       return RunThinBackend(AddStream);
 
     SmallString<40> Key;
     // The module may be cached, this helps handling it.
     computeCacheKey(Key, Conf, CombinedIndex, ModuleID, ImportList, ExportList,
                     ResolvedODR, DefinedGlobals);
     if (AddStreamFn CacheAddStream = Cache(Task, Key))
       return RunThinBackend(CacheAddStream);
 
     return Error::success();
   }
 
   Error start(
-      unsigned Task, MemoryBufferRef MBRef,
+      unsigned Task, BitcodeModule BM,
       const FunctionImporter::ImportMapTy &ImportList,
       const FunctionImporter::ExportSetTy &ExportList,
       const std::map<GlobalValue::GUID, GlobalValue::LinkageTypes> &ResolvedODR,
-      MapVector<StringRef, MemoryBufferRef> &ModuleMap) override {
-    StringRef ModulePath = MBRef.getBufferIdentifier();
+      MapVector<StringRef, BitcodeModule> &ModuleMap) override {
+    StringRef ModulePath = BM.getModuleIdentifier();
     assert(ModuleToDefinedGVSummaries.count(ModulePath));
     const GVSummaryMapTy &DefinedGlobals =
         ModuleToDefinedGVSummaries.find(ModulePath)->second;
     BackendThreadPool.async(
-        [=](MemoryBufferRef MBRef, ModuleSummaryIndex &CombinedIndex,
+        [=](BitcodeModule BM, ModuleSummaryIndex &CombinedIndex,
             const FunctionImporter::ImportMapTy &ImportList,
             const FunctionImporter::ExportSetTy &ExportList,
             const std::map<GlobalValue::GUID, GlobalValue::LinkageTypes>
                 &ResolvedODR,
             const GVSummaryMapTy &DefinedGlobals,
-            MapVector<StringRef, MemoryBufferRef> &ModuleMap) {
+            MapVector<StringRef, BitcodeModule> &ModuleMap) {
           Error E = runThinLTOBackendThread(
-              AddStream, Cache, Task, MBRef, CombinedIndex, ImportList,
+              AddStream, Cache, Task, BM, CombinedIndex, ImportList,
               ExportList, ResolvedODR, DefinedGlobals, ModuleMap);
           if (E) {
             std::unique_lock<std::mutex> L(ErrMu);
             if (Err)
               Err = joinErrors(std::move(*Err), std::move(E));
             else
               Err = std::move(E);
           }
         },
-        MBRef, std::ref(CombinedIndex), std::ref(ImportList),
+        BM, std::ref(CombinedIndex), std::ref(ImportList),
         std::ref(ExportList), std::ref(ResolvedODR), std::ref(DefinedGlobals),
         std::ref(ModuleMap));
     return Error::success();
   }
 
   Error wait() override {
     BackendThreadPool.wait();
     if (Err)
WriteIndexesThinBackend(
       std::string OldPrefix, std::string NewPrefix, bool ShouldEmitImportsFiles,
       std::string LinkedObjectsFileName)
       : ThinBackendProc(Conf, CombinedIndex, ModuleToDefinedGVSummaries),
         OldPrefix(OldPrefix), NewPrefix(NewPrefix),
         ShouldEmitImportsFiles(ShouldEmitImportsFiles),
         LinkedObjectsFileName(LinkedObjectsFileName) {}
 
   Error start(
-      unsigned Task, MemoryBufferRef MBRef,
+      unsigned Task, BitcodeModule BM,
       const FunctionImporter::ImportMapTy &ImportList,
       const FunctionImporter::ExportSetTy &ExportList,
       const std::map<GlobalValue::GUID, GlobalValue::LinkageTypes> &ResolvedODR,
-      MapVector<StringRef, MemoryBufferRef> &ModuleMap) override {
-    StringRef ModulePath = MBRef.getBufferIdentifier();
+      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);

llvm/trunk/lib/LTO/LTOBackend.cpp

Error lto::backend(Config &C, AddStreamFn AddStream,
   }
   return Error::success();
 }
 
 Error lto::thinBackend(Config &Conf, unsigned Task, AddStreamFn AddStream,
                        Module &Mod, ModuleSummaryIndex &CombinedIndex,
                        const FunctionImporter::ImportMapTy &ImportList,
                        const GVSummaryMapTy &DefinedGlobals,
-                       MapVector<StringRef, MemoryBufferRef> &ModuleMap) {
+                       MapVector<StringRef, BitcodeModule> &ModuleMap) {
   Expected<const Target *> TOrErr = initAndLookupTarget(Conf, Mod);
   if (!TOrErr)
     return TOrErr.takeError();
 
   std::unique_ptr<TargetMachine> TM =
       createTargetMachine(Conf, Mod.getTargetTriple(), *TOrErr);
 
   handleAsmUndefinedRefs(Mod, *TM);
Error lto::thinBackend(Config &Conf, unsigned Task, AddStreamFn AddStream,
 
   if (Conf.PostInternalizeModuleHook &&
       !Conf.PostInternalizeModuleHook(Task, Mod))
     return Error::success();
 
   auto ModuleLoader = [&](StringRef Identifier) {
     assert(Mod.getContext().isODRUniquingDebugTypes() &&
            "ODR Type uniquing should be enabled on the context");
-    return getLazyBitcodeModule(ModuleMap[Identifier], Mod.getContext(),
+    auto I = ModuleMap.find(Identifier);
+    assert(I != ModuleMap.end());
+    return I->second.getLazyModule(Mod.getContext(),
                                    /*ShouldLazyLoadMetadata=*/true);
   };
 
   FunctionImporter Importer(CombinedIndex, ModuleLoader);
   if (Error Err = Importer.importFunctions(Mod, ImportList).takeError())
     return Err;
 
   if (Conf.PostImportModuleHook && !Conf.PostImportModuleHook(Task, Mod))
     return Error::success();
 
   if (!opt(Conf, TM.get(), Task, Mod, /*IsThinLTO=*/true))
     return Error::success();
 
   codegen(Conf, TM.get(), AddStream, Task, Mod);
   return Error::success();
 }

llvm/trunk/test/LTO/Resolution/X86/empty-bitcode.test

+RUN: llvm-cat -o %t.o
+RUN: not llvm-lto2 -o %t2 %t.o 2>&1 | FileCheck %s
+CHECK: Bitcode file does not contain any modules

llvm/trunk/test/LTO/Resolution/X86/mixed_lto.ll

 ; Test mixed-mode LTO (mix of regular and thin LTO objects)
 ; RUN: opt %s -o %t1.o
 ; RUN: opt -module-summary %p/Inputs/mixed_lto.ll -o %t2.o
 
 ; RUN: llvm-lto2 -o %t3.o %t2.o %t1.o -r %t2.o,main,px -r %t2.o,g, -r %t1.o,g,px
 
 ; Task 0 is the regular LTO file (this file)
 ; RUN: llvm-nm %t3.o.0 | FileCheck %s --check-prefix=NM0
 ; NM0: T g
 
 ; Task 1 is the (first) ThinLTO file (Inputs/mixed_lto.ll)
 ; RUN: llvm-nm %t3.o.1 | FileCheck %s --check-prefix=NM1
 ; NM1-DAG: T main
 ; NM1-DAG: U g
 
+; Do the same test again, but with the regular and thin LTO modules in the same file.
+; RUN: llvm-cat -b -o %t4.o %t2.o %t1.o
+; RUN: llvm-lto2 -o %t5.o %t4.o -r %t4.o,main,px -r %t4.o,g, -r %t4.o,g,px
+; RUN: llvm-nm %t5.o.0 | FileCheck %s --check-prefix=NM0
+; RUN: llvm-nm %t5.o.1 | FileCheck %s --check-prefix=NM1
+
 target triple = "x86_64-unknown-linux-gnu"
 define i32 @g() {
   ret i32 0
 }

llvm/trunk/test/LTO/Resolution/X86/multi-thinlto.ll

+; RUN: opt -module-summary %s -o %t.o
+; RUN: llvm-cat -b -o %t2.o %t.o %t.o
+; RUN: not llvm-lto2 -o %t3.o %t2.o 2>&1 | FileCheck %s
+; CHECK: Expected at most one ThinLTO module per bitcode file
+
+target triple = "x86_64-unknown-linux-gnu"