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

30

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;
   std::unique_ptr<Mangler> Mang;
   typedef std::pair<std::string, uint32_t> AsmSymbol;
   SpecificBumpPtrAllocator<AsmSymbol> AsmSymbols;
 
   typedef PointerUnion<GlobalValue *, AsmSymbol *> Sym;
   std::vector<Sym> SymTab;
   static Sym getSym(DataRefImpl &Symb) {
     return *reinterpret_cast<Sym *>(Symb.p);
   }
 
+  void addModule(Module &M);
+
 public:
-  IRObjectFile(MemoryBufferRef Object, std::unique_ptr<Module> M);
+  IRObjectFile(MemoryBufferRef Object, std::vector<std::unique_ptr<Module>> Ms);
   ~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;
   GlobalValue *getSymbolGV(DataRefImpl Symb);
   const GlobalValue *getSymbolGV(DataRefImpl Symb) const {
     return const_cast<IRObjectFile *>(this)->getSymbolGV(Symb);
   }
   basic_symbol_iterator symbol_begin_impl() const override;
   basic_symbol_iterator symbol_end_impl() const override;
 
   const Module &getModule() const {
     return const_cast<IRObjectFile*>(this)->getModule();
   }
   Module &getModule() {
-    return *M;
+    return *Ms[0];
   }
   std::unique_ptr<Module> takeModule();
 
   static inline bool classof(const Binary *v) {
     return v->isIR();
   }
 
   /// \brief Finds and returns bitcode embedded in the given object file, or an

llvm/lib/LTO/LTOModule.cpp

if (Triple.isOSDarwin()) {
     else if (Triple.getArch() == llvm::Triple::aarch64)
       CPU = "cyclone";
   }
 
   TargetMachine *target =
       march->createTargetMachine(TripleStr, CPU, FeatureStr, options, None);
   M->setDataLayout(target->createDataLayout());
 
-  std::unique_ptr<object::IRObjectFile> IRObj(
-      new object::IRObjectFile(Buffer, std::move(M)));
+  std::vector<std::unique_ptr<Module>> Ms;
+  Ms.push_back(std::move(M));
+  std::unique_ptr<object::IRObjectFile> IRObj(new object::IRObjectFile(
+      Buffer, std::move(Ms)));
 
   std::unique_ptr<LTOModule> Ret(new LTOModule(std::move(IRObj), target));
   Ret->parseSymbols();
   Ret->parseMetadata();
 
   return std::move(Ret);
 }
 

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)) {
   Mang.reset(new Mangler());
+  for (auto &M : this->Ms)
+    addModule(*M);
+}
 
-  for (Function &F : *M)
+void IRObjectFile::addModule(Module &M) {
+  for (Function &F : M)
     SymTab.push_back(&F);
-  for (GlobalVariable &GV : M->globals())
+  for (GlobalVariable &GV : M.globals())
     SymTab.push_back(&GV);
-  for (GlobalAlias &GA : M->aliases())
+  for (GlobalAlias &GA : M.aliases())
     SymTab.push_back(&GA);
-  for (GlobalIFunc &GIF : M->ifuncs())
+  for (GlobalIFunc &GIF : M.ifuncs())
     SymTab.push_back(&GIF);
 
-  CollectAsmUndefinedRefs(Triple(M->getTargetTriple()), M->getModuleInlineAsm(),
+  CollectAsmUndefinedRefs(Triple(M.getTargetTriple()), M.getModuleInlineAsm(),
                           [this](StringRef Name, BasicSymbolRef::Flags Flags) {
                             SymTab.push_back(new (AsmSymbols.Allocate())
                                                  AsmSymbol(Name, Flags));
                           });
 }
 
 // Parse inline ASM and collect the list of symbols that are not defined in
 // the current module. This is inspired from IRObjectFile.
uint32_t IRObjectFile::getSymbolFlags(DataRefImpl Symb) const {
 
   return Res;
 }
 
 GlobalValue *IRObjectFile::getSymbolGV(DataRefImpl Symb) {
   return getSym(Symb).dyn_cast<GlobalValue *>();
 }
 
-std::unique_ptr<Module> IRObjectFile::takeModule() { return std::move(M); }
+std::unique_ptr<Module> IRObjectFile::takeModule() { return std::move(Ms[0]); }
 
 basic_symbol_iterator IRObjectFile::symbol_begin_impl() const {
   DataRefImpl Ret;
   Ret.p = reinterpret_cast<uintptr_t>(SymTab.data());
   return basic_symbol_iterator(BasicSymbolRef(Ret, this));
 }
 
 basic_symbol_iterator IRObjectFile::symbol_end_impl() const {
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();
-  return llvm::make_unique<IRObjectFile>(BCOrErr.get(), std::move(M));
+    Ms.push_back(std::move(*MOrErr));
+  }
+
+  return llvm::make_unique<IRObjectFile>(BCOrErr.get(), 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
+}