Object: Make IRObjectFile own multiple modules and enumerate symbols from all modules.

[pcc]

Summary

This implements multi-module support in IRObjectFile.

[pcc]

Upon further reflection, I think we want to go in a slightly different direction here:

• factor the "create a symbol table from a module" code out of IRObjectFile and into a new ModuleSymbolTable class, which would conceptually be responsible for maintaining the mapping between symbol table entries and GlobalValues. This would be similar to what I proposed in D23132, but at least to begin with it would just be a straight refactoring of the post- D26928 IRObjectFile code
• use that class from IRObjectFile
• change lib/LTO to use ModuleSymbolTable directly
• remove getModule, takeModule, getSymbolGV interfaces from IRObjectFile

When we get around to implementing bitcode symbol tables we can:

• implement the bitcode symbol table writer in terms of ModuleSymbolTable
• change IRObjectFile to read from the bitcode symbol table instead of using ModuleSymbolTable directly

I will start working on ModuleSymbolTable.

[pcc]

An important point which I forgot to mention: the symbol table stored by ModuleSymbolTable would correspond to any number of modules (all of the same target triple).

[mehdi_amini]

In D26951#603439, @pcc wrote:

An important point which I forgot to mention: the symbol table stored by ModuleSymbolTable would correspond to any number of modules (all of the same target triple).

That seems an arbitrary choice, that is only driven by the current use case of splitting vtables for LTO.

[pcc]

In D26951#603447, @mehdi_amini wrote:

In D26951#603439, @pcc wrote:

An important point which I forgot to mention: the symbol table stored by ModuleSymbolTable would correspond to any number of modules (all of the same target triple).

That seems an arbitrary choice, that is only driven by the current use case of splitting vtables for LTO.

I think in general there are two possible cases:

1. where by design the client needs to have multiple conceptual "views" into the input file (e.g. fat binaries, CUDA, OpenMP)
2. where the client has a single "view" and does not care about which symbol is defined in which module (e.g. regular/thin LTO splitting)

The client's use of ModuleSymbolTable (and the rest of the lib/Object interface in general) needs to be driven by that fundamental design decision of where the split lies. So for the fat binary scenario I would see the client creating one ModuleSymbolTable (and one bitcode symbol table) for each architecture, and the IRObjectFile growing a way to choose the architecture (as we do in MachOObjectFile for example).

[mehdi_amini]

The client's use of ModuleSymbolTable (and the rest of the lib/Object interface in general) needs to be driven by that fundamental design decision of where the split lies. So for the fat binary scenario I would see the client creating one ModuleSymbolTable (and one bitcode symbol table) for each architecture, and the IRObjectFile growing a way to choose the architecture (as we do in MachOObjectFile for example).

Right, but the "triple" as a discriminator seems arbitrary to me: what about use cases where we ship a "fat" object file containing bitcode for a non-optimized debug build of the module and an optimized one? Or for building with and without options like freestanding? Or with and without the sanitizers?

[mehdi_amini]

I'm fine with being pragmatic and making it work for CFI in LTO, I just want to make sure that the API and the design of the ModuleSymbolTable / IRObjectFile relationship does not make too many assumptions about it.

[pcc]

In D26951#603478, @mehdi_amini wrote:

Right, but the "triple" as a discriminator seems arbitrary to me: what about use cases where we ship a "fat" object file containing bitcode for a non-optimized debug build of the module and an optimized one? Or for building with and without options like freestanding? Or with and without the sanitizers?

I think we're confusing a couple of things here. I am not saying that the triple would be the discriminator; the discriminator could in principle be anything the client wants (and could be chosen at BitcodeWriter time). The reason I mentioned that modules associated with a single ModuleSymbolTable should have the same triple is to ensure that the eventual object files are compatible and that name mangling happens consistently. I am not precluding having multiple ModuleSymbolTables whose modules happen to have the same target triple.

[mehdi_amini]

Sure, this part is fine, I think what tickled me was "the IRObjectFile growing a way to choose the architecture", which I read as "you pass in the architecture and it get all the right modules from the bitcode file and get a ModuleSymbolTable for them".

[pcc]

In D26951#603493, @mehdi_amini wrote:

Sure, this part is fine, I think what tickled me was "the IRObjectFile growing a way to choose the architecture", which I read as "you pass in the architecture and it get all the right modules from the bitcode file and get a ModuleSymbolTable for them".

Right, "architecture" was just an example here, we can make a better decision about what exactly the discriminator should be when the time comes to implement a feature that depends on it.

[pcc]

Depends on D27079

[mehdi_amini]

LGTM.

llvm/include/llvm/Object/IRObjectFile.h

31

I rather avoid the ultra-contraction, what about: Mods

[pcc]

• Address review comments; pick up tool rename

llvm/include/llvm/Object/IRObjectFile.h

 class Module;
 class GlobalValue;
 class Triple;
 
 namespace object {
 class ObjectFile;
 
 class IRObjectFile : public SymbolicFile {
-  std::unique_ptr<Module> M;
+  std::vector<std::unique_ptr<Module>> Ms;
   ModuleSymbolTable SymTab;
-  IRObjectFile(MemoryBufferRef Object, std::unique_ptr<Module> M);
+  IRObjectFile(MemoryBufferRef Object, std::vector<std::unique_ptr<Module>> Ms);
 
 public:
   ~IRObjectFile() override;
   void moveSymbolNext(DataRefImpl &Symb) const override;
   std::error_code printSymbolName(raw_ostream &OS,
                                   DataRefImpl Symb) const override;
   uint32_t getSymbolFlags(DataRefImpl Symb) const override;
   basic_symbol_iterator symbol_begin() const override;

llvm/lib/Object/IRObjectFile.cpp

 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace llvm;
 using namespace object;
 
-IRObjectFile::IRObjectFile(MemoryBufferRef Object, std::unique_ptr<Module> Mod)
-    : SymbolicFile(Binary::ID_IR, Object), M(std::move(Mod)) {
+IRObjectFile::IRObjectFile(MemoryBufferRef Object,
+                           std::vector<std::unique_ptr<Module>> Ms)
+    : SymbolicFile(Binary::ID_IR, Object), Ms(std::move(Ms)) {
+  for (auto &M : this->Ms)
     SymTab.addModule(M.get());
 }
 
 IRObjectFile::~IRObjectFile() {}
 
 static ModuleSymbolTable::Symbol getSym(DataRefImpl &Symb) {
   return *reinterpret_cast<ModuleSymbolTable::Symbol *>(Symb.p);
 }
 

 
 basic_symbol_iterator IRObjectFile::symbol_end() const {
   DataRefImpl Ret;
   Ret.p = reinterpret_cast<uintptr_t>(SymTab.symbols().data() +
                                       SymTab.symbols().size());
   return basic_symbol_iterator(BasicSymbolRef(Ret, this));
 }
 
-StringRef IRObjectFile::getTargetTriple() const { return M->getTargetTriple(); }
+StringRef IRObjectFile::getTargetTriple() const {
+  // Each module must have the same target triple, so we arbitrarily access the
+  // first one.
+  return Ms[0]->getTargetTriple();
+}
 
 ErrorOr<MemoryBufferRef> IRObjectFile::findBitcodeInObject(const ObjectFile &Obj) {
   for (const SectionRef &Sec : Obj.sections()) {
     if (Sec.isBitcode()) {
       StringRef SecContents;
       if (std::error_code EC = Sec.getContents(SecContents))
         return EC;
       return MemoryBufferRef(SecContents, Obj.getFileName());
case sys::fs::file_magic::coff_object: {
     return findBitcodeInObject(*ObjFile->get());
   }
   default:
     return object_error::invalid_file_type;
   }
 }
 
 Expected<std::unique_ptr<IRObjectFile>>
-llvm::object::IRObjectFile::create(MemoryBufferRef Object,
-                                   LLVMContext &Context) {
+IRObjectFile::create(MemoryBufferRef Object, LLVMContext &Context) {
   ErrorOr<MemoryBufferRef> BCOrErr = findBitcodeInMemBuffer(Object);
   if (!BCOrErr)
     return errorCodeToError(BCOrErr.getError());
 
+  Expected<std::vector<BitcodeModule>> BMsOrErr =
+      getBitcodeModuleList(*BCOrErr);
+  if (!BMsOrErr)
+    return BMsOrErr.takeError();
+
+  std::vector<std::unique_ptr<Module>> Ms;
+  for (auto BM : *BMsOrErr) {
     Expected<std::unique_ptr<Module>> MOrErr =
-      getLazyBitcodeModule(*BCOrErr, Context,
-                           /*ShouldLazyLoadMetadata*/ true);
+        BM.getLazyModule(Context, /*ShouldLazyLoadMetadata*/ true);
     if (!MOrErr)
       return MOrErr.takeError();
 
-  std::unique_ptr<Module> &M = MOrErr.get();
+    Ms.push_back(std::move(*MOrErr));
+  }
+
   return std::unique_ptr<IRObjectFile>(
-      new IRObjectFile(*BCOrErr, std::move(M)));
+      new IRObjectFile(*BCOrErr, std::move(Ms)));
 }

llvm/test/Object/Inputs/multi-module.ll

+define void @f2() {
+  ret void
+}

llvm/test/Object/multi-module.ll

+; RUN: llvm-join -o - %s %S/Inputs/multi-module.ll | llvm-nm - | FileCheck %s
+
+; CHECK: T f1
+; CHECK: T f2
+
+define void @f1() {
+  ret void
+}