Object: Factor out the code for creating the irsymtab for an arbitrary bitcode file.

[pcc]

Summary

This code now lives in lib/Object. The idea is that it can now be reused by
IRObjectFile among other things.

[tejohnson]

It seems odd to me to have the invocation of the module reader as well in irsymtab, which is just a piece of the file. Why not in IRObjectFile.cpp in llvm::object?

[pcc]

The irsymtab interface lives at a lower layer than IRObjectFile, as the irsymtab interface is specifically designed for IR symbol tables as opposed to the generic object file interface exposed by ObjectFile. I see this function as the layer between the raw irsymtab reader interface and clients such as IRObjectFile (and LTO, etc). So it doesn't seem appropriate for this function to live alongside one specific client, and it doesn't really deserve its own file/namespace, so it might as well live alongside irsymtab.

[tejohnson]

In D31921#724231, @pcc wrote:

The irsymtab interface lives at a lower layer than IRObjectFile, as the irsymtab interface is specifically designed for IR symbol tables as opposed to the generic object file interface exposed by ObjectFile. I see this function as the layer between the raw irsymtab reader interface and clients such as IRObjectFile (and LTO, etc). So it doesn't seem appropriate for this function to live alongside one specific client, and it doesn't really deserve its own file/namespace, so it might as well live alongside irsymtab.

What I don't like is that this is not just parsing the irsymtab structures, but also the modules themselves.

[pcc]

In D31921#724251, @tejohnson wrote:

In D31921#724231, @pcc wrote:

The irsymtab interface lives at a lower layer than IRObjectFile, as the irsymtab interface is specifically designed for IR symbol tables as opposed to the generic object file interface exposed by ObjectFile. I see this function as the layer between the raw irsymtab reader interface and clients such as IRObjectFile (and LTO, etc). So it doesn't seem appropriate for this function to live alongside one specific client, and it doesn't really deserve its own file/namespace, so it might as well live alongside irsymtab.

What I don't like is that this is not just parsing the irsymtab structures, but also the modules themselves.

True, but that is only an implementation detail. When we start writing the irsymtab to disk, the modules will only be parsed on the "upgrade" code path.

[tejohnson]

In D31921#724280, @pcc wrote:

In D31921#724251, @tejohnson wrote:

In D31921#724231, @pcc wrote:

The irsymtab interface lives at a lower layer than IRObjectFile, as the irsymtab interface is specifically designed for IR symbol tables as opposed to the generic object file interface exposed by ObjectFile. I see this function as the layer between the raw irsymtab reader interface and clients such as IRObjectFile (and LTO, etc). So it doesn't seem appropriate for this function to live alongside one specific client, and it doesn't really deserve its own file/namespace, so it might as well live alongside irsymtab.

What I don't like is that this is not just parsing the irsymtab structures, but also the modules themselves.

True, but that is only an implementation detail. When we start writing the irsymtab to disk, the modules will only be parsed on the "upgrade" code path.

Ok, I forgot about that. Looking now at the InputFile class, the comments indicate it only holds what is necessary for symbol resolution, so I guess the idea is that the Mods field will go away (except I guess on the upgrade path) when we have a real IR symbol table on disk? But then currently the Mods field is used when we add the module for regular LTO, where we do need the full module.

[pcc]

In D31921#724520, @tejohnson wrote:

In D31921#724280, @pcc wrote:

In D31921#724251, @tejohnson wrote:

In D31921#724231, @pcc wrote:

The irsymtab interface lives at a lower layer than IRObjectFile, as the irsymtab interface is specifically designed for IR symbol tables as opposed to the generic object file interface exposed by ObjectFile. I see this function as the layer between the raw irsymtab reader interface and clients such as IRObjectFile (and LTO, etc). So it doesn't seem appropriate for this function to live alongside one specific client, and it doesn't really deserve its own file/namespace, so it might as well live alongside irsymtab.

What I don't like is that this is not just parsing the irsymtab structures, but also the modules themselves.

True, but that is only an implementation detail. When we start writing the irsymtab to disk, the modules will only be parsed on the "upgrade" code path.

Ok, I forgot about that. Looking now at the InputFile class, the comments indicate it only holds what is necessary for symbol resolution, so I guess the idea is that the Mods field will go away (except I guess on the upgrade path) when we have a real IR symbol table on disk? But then currently the Mods field is used when we add the module for regular LTO, where we do need the full module.

We will always need references to the actual modules in the InputFile class as part of the implementation (not only for full LTO but for ThinLTO to read the summaries and the modules themselves in the backends). I see this function as returning the conceptual top-level entities in the bitcode file, i.e. the symbol table as well as references to the modules.

[tejohnson]

In D31921#725184, @pcc wrote:

In D31921#724520, @tejohnson wrote:

In D31921#724280, @pcc wrote:

In D31921#724251, @tejohnson wrote:

In D31921#724231, @pcc wrote:

The irsymtab interface lives at a lower layer than IRObjectFile, as the irsymtab interface is specifically designed for IR symbol tables as opposed to the generic object file interface exposed by ObjectFile. I see this function as the layer between the raw irsymtab reader interface and clients such as IRObjectFile (and LTO, etc). So it doesn't seem appropriate for this function to live alongside one specific client, and it doesn't really deserve its own file/namespace, so it might as well live alongside irsymtab.

What I don't like is that this is not just parsing the irsymtab structures, but also the modules themselves.

True, but that is only an implementation detail. When we start writing the irsymtab to disk, the modules will only be parsed on the "upgrade" code path.

Ok, I forgot about that. Looking now at the InputFile class, the comments indicate it only holds what is necessary for symbol resolution, so I guess the idea is that the Mods field will go away (except I guess on the upgrade path) when we have a real IR symbol table on disk? But then currently the Mods field is used when we add the module for regular LTO, where we do need the full module.

We will always need references to the actual modules in the InputFile class as part of the implementation (not only for full LTO but for ThinLTO to read the summaries and the modules themselves in the backends). I see this function as returning the conceptual top-level entities in the bitcode file, i.e. the symbol table as well as references to the modules.

Ok, but then I am confused about your earlier response that eventually the modules will only be parsed on the "upgrade" code path? If this outlined function will continue to parse all top level entities in the bitcode file, then I am not sure it belongs in irsymtab.

[pcc]

In D31921#725214, @tejohnson wrote:

Ok, but then I am confused about your earlier response that eventually the modules will only be parsed on the "upgrade" code path? If this outlined function will continue to parse all top level entities in the bitcode file, then I am not sure it belongs in irsymtab.

I see the symbol table becoming the top-level entity in the bitcode file in the long term (if we switch to a new bitcode format). But I may be wrong, and I suppose that from that perspective it doesn't matter where the code lives, because it will all be rewritten anyway. So I'm fine with moving it to IRObjectFile if you still think it sohuld go there.

[tejohnson]

In D31921#725225, @pcc wrote:

In D31921#725214, @tejohnson wrote:

Ok, but then I am confused about your earlier response that eventually the modules will only be parsed on the "upgrade" code path? If this outlined function will continue to parse all top level entities in the bitcode file, then I am not sure it belongs in irsymtab.

I see the symbol table becoming the top-level entity in the bitcode file in the long term (if we switch to a new bitcode format). But I may be wrong, and I suppose that from that perspective it doesn't matter where the code lives, because it will all be rewritten anyway. So I'm fine with moving it to IRObjectFile if you still think it sohuld go there.

To me conceptually the module also a top level entity, but I guess it depends on how you look at it. I'd prefer it in IRObjectFile over having it within irsymtab.

[pcc]

In D31921#725253, @tejohnson wrote:

In D31921#725225, @pcc wrote:

In D31921#725214, @tejohnson wrote:

Ok, but then I am confused about your earlier response that eventually the modules will only be parsed on the "upgrade" code path? If this outlined function will continue to parse all top level entities in the bitcode file, then I am not sure it belongs in irsymtab.

I see the symbol table becoming the top-level entity in the bitcode file in the long term (if we switch to a new bitcode format). But I may be wrong, and I suppose that from that perspective it doesn't matter where the code lives, because it will all be rewritten anyway. So I'm fine with moving it to IRObjectFile if you still think it sohuld go there.

To me conceptually the module also a top level entity, but I guess it depends on how you look at it. I'd prefer it in IRObjectFile over having it within irsymtab.

I also realised that moving the code to IRObjectFile means that IRObjectFile and irsymtab will need to conspire to use the same producer string to decide whether to upgrade, whereas without the move the logic is isolated to irsymtab. So I am slightly more against the move now. I will upload my change to start writing the irsymtab files to disk, so you can see what I mean.

[pcc]

D32061 shows how I expect irsymtab::read to evolve. Let me know if that makes sense.

[pcc]

Refresh

[tejohnson]

In D31921#725260, @pcc wrote:

In D31921#725253, @tejohnson wrote:

In D31921#725225, @pcc wrote:

In D31921#725214, @tejohnson wrote:

Ok, but then I am confused about your earlier response that eventually the modules will only be parsed on the "upgrade" code path? If this outlined function will continue to parse all top level entities in the bitcode file, then I am not sure it belongs in irsymtab.

I see the symbol table becoming the top-level entity in the bitcode file in the long term (if we switch to a new bitcode format). But I may be wrong, and I suppose that from that perspective it doesn't matter where the code lives, because it will all be rewritten anyway. So I'm fine with moving it to IRObjectFile if you still think it sohuld go there.

To me conceptually the module also a top level entity, but I guess it depends on how you look at it. I'd prefer it in IRObjectFile over having it within irsymtab.

I also realised that moving the code to IRObjectFile means that IRObjectFile and irsymtab will need to conspire to use the same producer string to decide whether to upgrade, whereas without the move the logic is isolated to irsymtab. So I am slightly more against the move now. I will upload my change to start writing the irsymtab files to disk, so you can see what I mean.

Can you clarify how this would make the upgrade decision difficult? I.e. I was envisioning having the main entry point to create and return the new "File" struct in IRObjectFile, which would then invoke irsymtab::read to fill in the Symtab and Strtab. In D32061, it looks like the decision to take the upgrade path based on the producer string only involves those two structures. Why would the IRObjectFile need to know about the producer string?

[pcc]

In D31921#769184, @tejohnson wrote:

In D31921#725260, @pcc wrote:

In D31921#725253, @tejohnson wrote:

In D31921#725225, @pcc wrote:

In D31921#725214, @tejohnson wrote:

Ok, but then I am confused about your earlier response that eventually the modules will only be parsed on the "upgrade" code path? If this outlined function will continue to parse all top level entities in the bitcode file, then I am not sure it belongs in irsymtab.

I see the symbol table becoming the top-level entity in the bitcode file in the long term (if we switch to a new bitcode format). But I may be wrong, and I suppose that from that perspective it doesn't matter where the code lives, because it will all be rewritten anyway. So I'm fine with moving it to IRObjectFile if you still think it sohuld go there.

To me conceptually the module also a top level entity, but I guess it depends on how you look at it. I'd prefer it in IRObjectFile over having it within irsymtab.

I also realised that moving the code to IRObjectFile means that IRObjectFile and irsymtab will need to conspire to use the same producer string to decide whether to upgrade, whereas without the move the logic is isolated to irsymtab. So I am slightly more against the move now. I will upload my change to start writing the irsymtab files to disk, so you can see what I mean.

Can you clarify how this would make the upgrade decision difficult? I.e. I was envisioning having the main entry point to create and return the new "File" struct in IRObjectFile, which would then invoke irsymtab::read to fill in the Symtab and Strtab. In D32061, it looks like the decision to take the upgrade path based on the producer string only involves those two structures. Why would the IRObjectFile need to know about the producer string?

Oh, so you want the code to look like this:

in irsymtab.cpp:

Expected<Reader> readWithoutUpgrading(StringRef Symtab, StringRef Strtab) {
  // try to read the Symtab

  if (needs to upgrade)
    return Reader();

  Reader R = {Symtab, Strtab};
  return R;
}

in irobjectfile.cpp:

Expected<File> read(MemoryBufferRef MBRef) {
  auto BFC = getBitcodeFileContents(MBRef);
  Reader R = readWithoutUpgrading(BFC.Symtab, BFC.StrtabForSymtab);
  if (!R.isValid())
    return upgrade(BFC);
  return File(R);
}

I dunno, to me it seems less clear and less maintainable because now people have to look at two files to figure out how upgrading works. But I guess I can tolerate it.

[tejohnson]

In D31921#769239, @pcc wrote:

In D31921#769184, @tejohnson wrote:

In D31921#725260, @pcc wrote:

In D31921#725253, @tejohnson wrote:

In D31921#725225, @pcc wrote:

In D31921#725214, @tejohnson wrote:

Ok, but then I am confused about your earlier response that eventually the modules will only be parsed on the "upgrade" code path? If this outlined function will continue to parse all top level entities in the bitcode file, then I am not sure it belongs in irsymtab.

I see the symbol table becoming the top-level entity in the bitcode file in the long term (if we switch to a new bitcode format). But I may be wrong, and I suppose that from that perspective it doesn't matter where the code lives, because it will all be rewritten anyway. So I'm fine with moving it to IRObjectFile if you still think it sohuld go there.

To me conceptually the module also a top level entity, but I guess it depends on how you look at it. I'd prefer it in IRObjectFile over having it within irsymtab.

I also realised that moving the code to IRObjectFile means that IRObjectFile and irsymtab will need to conspire to use the same producer string to decide whether to upgrade, whereas without the move the logic is isolated to irsymtab. So I am slightly more against the move now. I will upload my change to start writing the irsymtab files to disk, so you can see what I mean.

Can you clarify how this would make the upgrade decision difficult? I.e. I was envisioning having the main entry point to create and return the new "File" struct in IRObjectFile, which would then invoke irsymtab::read to fill in the Symtab and Strtab. In D32061, it looks like the decision to take the upgrade path based on the producer string only involves those two structures. Why would the IRObjectFile need to know about the producer string?

Oh, so you want the code to look like this:

No, but I may be missing something... Why can't there be a single call into irsymtab to a function that takes the BFC, and returns a structure containing the Reader, Strtab, and Symtab to use in the File being created? I.e. using those in the BFC if no upgrade needed, or doing the upgrade if necessary.

in irsymtab.cpp:

Expected<Reader> readWithoutUpgrading(StringRef Symtab, StringRef Strtab) {
  // try to read the Symtab

  if (needs to upgrade)
    return Reader();

  Reader R = {Symtab, Strtab};
  return R;
}

in irobjectfile.cpp:

Expected<File> read(MemoryBufferRef MBRef) {
  auto BFC = getBitcodeFileContents(MBRef);
  Reader R = readWithoutUpgrading(BFC.Symtab, BFC.StrtabForSymtab);
  if (!R.isValid())
    return upgrade(BFC);
  return File(R);
}

I dunno, to me it seems less clear and less maintainable because now people have to look at two files to figure out how upgrading works. But I guess I can tolerate it.

[pcc]

In D31921#769264, @tejohnson wrote:

In D31921#769239, @pcc wrote:

In D31921#769184, @tejohnson wrote:

In D31921#725260, @pcc wrote:

In D31921#725253, @tejohnson wrote:

In D31921#725225, @pcc wrote:

In D31921#725214, @tejohnson wrote:

Ok, but then I am confused about your earlier response that eventually the modules will only be parsed on the "upgrade" code path? If this outlined function will continue to parse all top level entities in the bitcode file, then I am not sure it belongs in irsymtab.

I see the symbol table becoming the top-level entity in the bitcode file in the long term (if we switch to a new bitcode format). But I may be wrong, and I suppose that from that perspective it doesn't matter where the code lives, because it will all be rewritten anyway. So I'm fine with moving it to IRObjectFile if you still think it sohuld go there.

To me conceptually the module also a top level entity, but I guess it depends on how you look at it. I'd prefer it in IRObjectFile over having it within irsymtab.

I also realised that moving the code to IRObjectFile means that IRObjectFile and irsymtab will need to conspire to use the same producer string to decide whether to upgrade, whereas without the move the logic is isolated to irsymtab. So I am slightly more against the move now. I will upload my change to start writing the irsymtab files to disk, so you can see what I mean.

Can you clarify how this would make the upgrade decision difficult? I.e. I was envisioning having the main entry point to create and return the new "File" struct in IRObjectFile, which would then invoke irsymtab::read to fill in the Symtab and Strtab. In D32061, it looks like the decision to take the upgrade path based on the producer string only involves those two structures. Why would the IRObjectFile need to know about the producer string?

Oh, so you want the code to look like this:

No, but I may be missing something... Why can't there be a single call into irsymtab to a function that takes the BFC, and returns a structure containing the Reader, Strtab, and Symtab to use in the File being created? I.e. using those in the BFC if no upgrade needed, or doing the upgrade if necessary.

That is basically the same as what irsymtab::read is doing in D32061, right? I think the only difference is that irsymtab::read would be taking a list of BitcodeModules rather than returning a list of BitcodeModules. And we'd be adding a function in IRObjectFile that converts from that interface to the current irsymtab::read interface.

If so, I guess that's also fine with me.

[tejohnson]

In D31921#769284, @pcc wrote:

In D31921#769264, @tejohnson wrote:

No, but I may be missing something... Why can't there be a single call into irsymtab to a function that takes the BFC, and returns a structure containing the Reader, Strtab, and Symtab to use in the File being created? I.e. using those in the BFC if no upgrade needed, or doing the upgrade if necessary.

That is basically the same as what irsymtab::read is doing in D32061, right? I think the only difference is that irsymtab::read would be taking a list of BitcodeModules rather than returning a list of BitcodeModules. And we'd be adding a function in IRObjectFile that converts from that interface to the current irsymtab::read interface.

Right - essentially the main interface to the File creation is in IRObjectFile, which handles the creation of the BitcodeModules. To me that seems preferable.

[pcc]

• Address review comments

[pcc]

In D31921#769375, @tejohnson wrote:

In D31921#769284, @pcc wrote:

In D31921#769264, @tejohnson wrote:

No, but I may be missing something... Why can't there be a single call into irsymtab to a function that takes the BFC, and returns a structure containing the Reader, Strtab, and Symtab to use in the File being created? I.e. using those in the BFC if no upgrade needed, or doing the upgrade if necessary.

That is basically the same as what irsymtab::read is doing in D32061, right? I think the only difference is that irsymtab::read would be taking a list of BitcodeModules rather than returning a list of BitcodeModules. And we'd be adding a function in IRObjectFile that converts from that interface to the current irsymtab::read interface.

Right - essentially the main interface to the File creation is in IRObjectFile, which handles the creation of the BitcodeModules. To me that seems preferable.

I still don't understand why you think that interface is better, but I have made the changes that you have requested.

[tejohnson]

llvm/include/llvm/Object/IRObjectFile.h

67 ↗

(On Diff #101594)

why have this in the irsymtab namespace and not object? My main motivation for the move here is that irsymtab doesn't seem like the right namespace/layer for having the main entry point reading/building the full bitcode file contents. Or a new namespace (e.g. bitcodefile or bitcodeobject or the like).

llvm/include/llvm/Object/IRSymtab.h

322 ↗

(On Diff #101594)

Maybe IRSymtabContents? I.e. this doesn't contain the full contents of the bitcode file.

328 ↗

(On Diff #101594)

maybe readIRSymtab?

[pcc]

llvm/include/llvm/Object/IRObjectFile.h

67 ↗

(On Diff #101594)

To me these are still conceptually part of the irsymtab, I moved them here at your request. But I'm not going to pick this particular battle either, so ok, let's move them to llvm::object. Does llvm::object::IRSymtabFile/llvm::object::readIRSymtab sound fine?

llvm/include/llvm/Object/IRSymtab.h

322 ↗

(On Diff #101594)

It is already in a namespace called irsymtab. There's no need to repeat that in the name here.

That said, I should update the comment to reflect that this just contains the irsymtab contents.

328 ↗

(On Diff #101594)

Same.

[tejohnson]

llvm/include/llvm/Object/IRObjectFile.h

67 ↗

(On Diff #101594)

My belief is that the BitcodeModules are not part of the irsymtab (it seems we disagree on this point though). So I think llvm::object::File and llvm::object::readFile seem better.

[tejohnson]

llvm/include/llvm/Object/IRObjectFile.h

67 ↗

(On Diff #101594)

Based on our offline discussion, your proposal seems like a reasonable interface.

[pcc]

• Repaint the bikeshed

[tejohnson]

LGTM, thanks

llvm/trunk/include/llvm/Object/IRObjectFile.h

 // This file declares the IRObjectFile template class.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_OBJECT_IROBJECTFILE_H
 #define LLVM_OBJECT_IROBJECTFILE_H
 
 #include "llvm/ADT/PointerUnion.h"
+#include "llvm/Object/IRSymtab.h"
 #include "llvm/Object/ModuleSymbolTable.h"
 #include "llvm/Object/SymbolicFile.h"
 
 namespace llvm {
 class Mangler;
 class Module;
 class GlobalValue;
 class Triple;
public:
   /// be either a bitcode file or a native object file with embedded bitcode),
   /// or an error code if not found.
   static ErrorOr<MemoryBufferRef>
   findBitcodeInMemBuffer(MemoryBufferRef Object);
 
   static Expected<std::unique_ptr<IRObjectFile>> create(MemoryBufferRef Object,
                                                         LLVMContext &Context);
 };
+
+/// The contents of a bitcode file and its irsymtab. Any underlying data
+/// for the irsymtab are owned by Symtab and Strtab.
+struct IRSymtabFile {
+  std::vector<BitcodeModule> Mods;
+  SmallVector<char, 0> Symtab, Strtab;
+  irsymtab::Reader TheReader;
+};
+
+/// Reads a bitcode file, creating its irsymtab if necessary.
+Expected<IRSymtabFile> readIRSymtab(MemoryBufferRef MBRef);
+
 }
+
 }
 
 #endif

llvm/trunk/include/llvm/Object/IRSymtab.h

 #include "llvm/Object/SymbolicFile.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Error.h"
 #include <cassert>
 #include <cstdint>
 #include <vector>
 
 namespace llvm {
+
+class BitcodeModule;
+
 namespace irsymtab {
 
 namespace storage {
 
 // The data structures in this namespace define the low-level serialization
 // format. Clients that just want to read a symbol table should use the
 // irsymtab::Reader class.
 

 inline Reader::symbol_range Reader::module_symbols(unsigned I) const {
   const storage::Module &M = Modules[I];
   const storage::Symbol *MBegin = Symbols.begin() + M.Begin,
                         *MEnd = Symbols.begin() + M.End;
   return {SymbolRef(MBegin, MEnd, Uncommons.begin() + M.UncBegin, this),
           SymbolRef(MEnd, MEnd, nullptr, this)};
 }
 
+/// The contents of the irsymtab in a bitcode file. Any underlying data for the
+/// irsymtab are owned by Symtab and Strtab.
+struct FileContents {
+  SmallVector<char, 0> Symtab, Strtab;
+  Reader TheReader;
+};
+
+/// Reads the contents of a bitcode file, creating its irsymtab if necessary.
+Expected<FileContents> readBitcode(ArrayRef<BitcodeModule> Mods);
+
 } // end namespace irsymtab
 } // end namespace llvm
 
 #endif // LLVM_OBJECT_IRSYMTAB_H

llvm/trunk/lib/LTO/LTO.cpp

 }
 
 // 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());
-
-  Expected<std::vector<BitcodeModule>> BMsOrErr =
-      getBitcodeModuleList(*BCOrErr);
-  if (!BMsOrErr)
-    return BMsOrErr.takeError();
+  Expected<IRSymtabFile> FOrErr = readIRSymtab(Object);
+  if (!FOrErr)
+    return FOrErr.takeError();
+
+  File->TargetTriple = FOrErr->TheReader.getTargetTriple();
+  File->SourceFileName = FOrErr->TheReader.getSourceFileName();
+  File->COFFLinkerOpts = FOrErr->TheReader.getCOFFLinkerOpts();
+  File->ComdatTable = FOrErr->TheReader.getComdatTable();
 
-  if (BMsOrErr->empty())
-    return make_error<StringError>("Bitcode file does not contain any modules",
-                                   inconvertibleErrorCode());
-
-  File->Mods = *BMsOrErr;
-
-  LLVMContext Ctx;
-  std::vector<Module *> Mods;
-  std::vector<std::unique_ptr<Module>> OwnedMods;
-  for (auto BM : *BMsOrErr) {
-    Expected<std::unique_ptr<Module>> MOrErr =
-        BM.getLazyModule(Ctx, /*ShouldLazyLoadMetadata*/ true,
-                         /*IsImporting*/ false);
-    if (!MOrErr)
-      return MOrErr.takeError();
-
-    if ((*MOrErr)->getDataLayoutStr().empty())
-      return make_error<StringError>("input module has no datalayout",
-                                     inconvertibleErrorCode());
-
-    Mods.push_back(MOrErr->get());
-    OwnedMods.push_back(std::move(*MOrErr));
-  }
-
-  SmallVector<char, 0> Symtab;
-  if (Error E = irsymtab::build(Mods, Symtab, File->Strtab))
-    return std::move(E);
-
-  irsymtab::Reader R({Symtab.data(), Symtab.size()},
-                     {File->Strtab.data(), File->Strtab.size()});
-  File->TargetTriple = R.getTargetTriple();
-  File->SourceFileName = R.getSourceFileName();
-  File->COFFLinkerOpts = R.getCOFFLinkerOpts();
-  File->ComdatTable = R.getComdatTable();
-
-  for (unsigned I = 0; I != Mods.size(); ++I) {
+  for (unsigned I = 0; I != FOrErr->Mods.size(); ++I) {
     size_t Begin = File->Symbols.size();
-    for (const irsymtab::Reader::SymbolRef &Sym : R.module_symbols(I))
+    for (const irsymtab::Reader::SymbolRef &Sym :
+         FOrErr->TheReader.module_symbols(I))
       // Skip symbols that are irrelevant to LTO. Note that this condition needs
       // to match the one in Skip() in LTO::addRegularLTO().
       if (Sym.isGlobal() && !Sym.isFormatSpecific())
         File->Symbols.push_back(Sym);
     File->ModuleSymIndices.push_back({Begin, File->Symbols.size()});
   }
 
+  File->Mods = FOrErr->Mods;
+  File->Strtab = std::move(FOrErr->Strtab);
   return std::move(File);
 }
 
 StringRef InputFile::getName() const {
   return Mods[0].getModuleIdentifier();
 }
 
 LTO::RegularLTOState::RegularLTOState(unsigned ParallelCodeGenParallelismLevel,

llvm/trunk/lib/Object/IRObjectFile.cpp

if (!MOrErr)
       return MOrErr.takeError();
 
     Mods.push_back(std::move(*MOrErr));
   }
 
   return std::unique_ptr<IRObjectFile>(
       new IRObjectFile(*BCOrErr, std::move(Mods)));
 }
+
+Expected<IRSymtabFile> object::readIRSymtab(MemoryBufferRef MBRef) {
+  IRSymtabFile F;
+  ErrorOr<MemoryBufferRef> BCOrErr =
+      IRObjectFile::findBitcodeInMemBuffer(MBRef);
+  if (!BCOrErr)
+    return errorCodeToError(BCOrErr.getError());
+
+  Expected<std::vector<BitcodeModule>> BMsOrErr =
+      getBitcodeModuleList(*BCOrErr);
+  if (!BMsOrErr)
+    return BMsOrErr.takeError();
+
+  Expected<irsymtab::FileContents> FCOrErr = irsymtab::readBitcode(*BMsOrErr);
+  if (!FCOrErr)
+    return FCOrErr.takeError();
+
+  F.Mods = std::move(*BMsOrErr);
+  F.Symtab = std::move(FCOrErr->Symtab);
+  F.Strtab = std::move(FCOrErr->Strtab);
+  F.TheReader = std::move(FCOrErr->TheReader);
+  return std::move(F);
+}

llvm/trunk/lib/Object/IRSymtab.cpp

 #include "llvm/Analysis/ObjectUtils.h"
 #include "llvm/IR/Comdat.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/GlobalAlias.h"
 #include "llvm/IR/GlobalObject.h"
 #include "llvm/IR/Mangler.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
+#include "llvm/Bitcode/BitcodeReader.h"
 #include "llvm/MC/StringTableBuilder.h"
+#include "llvm/Object/IRObjectFile.h"
 #include "llvm/Object/ModuleSymbolTable.h"
 #include "llvm/Object/SymbolicFile.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>

 }
 
 } // end anonymous namespace
 
 Error irsymtab::build(ArrayRef<Module *> Mods, SmallVector<char, 0> &Symtab,
                       SmallVector<char, 0> &Strtab) {
   return Builder(Symtab, Strtab).build(Mods);
 }
+
+Expected<FileContents> irsymtab::readBitcode(ArrayRef<BitcodeModule> BMs) {
+  FileContents FC;
+  if (BMs.empty())
+    return make_error<StringError>("Bitcode file does not contain any modules",
+                                   inconvertibleErrorCode());
+
+  LLVMContext Ctx;
+  std::vector<Module *> Mods;
+  std::vector<std::unique_ptr<Module>> OwnedMods;
+  for (auto BM : BMs) {
+    Expected<std::unique_ptr<Module>> MOrErr =
+        BM.getLazyModule(Ctx, /*ShouldLazyLoadMetadata*/ true,
+                         /*IsImporting*/ false);
+    if (!MOrErr)
+      return MOrErr.takeError();
+
+    if ((*MOrErr)->getDataLayoutStr().empty())
+      return make_error<StringError>("input module has no datalayout",
+                                     inconvertibleErrorCode());
+
+    Mods.push_back(MOrErr->get());
+    OwnedMods.push_back(std::move(*MOrErr));
+  }
+
+  if (Error E = build(Mods, FC.Symtab, FC.Strtab))
+    return std::move(E);
+
+  FC.TheReader = {{FC.Symtab.data(), FC.Symtab.size()},
+                  {FC.Strtab.data(), FC.Strtab.size()}};
+  return std::move(FC);
+}