[ThinLTO] Add caching to the new LTO API

[mehdi_amini]

Summary

Add the ability to plug a cache on the LTO API.
I tried to write such that a linker implementation can
control the cache backend. I tried multiple design before
settling on this one, even if I'm not totally happy about
it. Suggestions welcome if any.

[dberris]

include/llvm/LTO/Caching.h

38 ↗

(On Diff #68305)

Is it worth typedef'ing std::function<void(std::unique_ptr<MemoryBuffer>)> such that you don't repeat yourself here?

[dberris]

include/llvm/LTO/Caching.h

36 ↗

(On Diff #68305)

Incomplete documentation?

67 ↗

(On Diff #68305)

Unsure what this means in Doxygen, but should there be a closing @} somewhere else?

[mehdi_amini]

Address Dean's comments.

[mehdi_amini]

Thanks for catching this :)

include/llvm/LTO/Caching.h

67 ↗

(On Diff #68305)

It is for grouping (\defgroup), but here it is a bad copy/paste. I removed all this code.

[mehdi_amini]

Missed one...

[mehdi_amini]

Bugfix in Caching.cpp

[tejohnson]

I had a bit of trouble following the flow, so I added in some suggestions for more comments and some assertion checking. Also, I think I found a couple of errors, see below. Once you have a fixed patch uploaded I will give it a try with gold-plugin.

include/llvm/LTO/Caching.h

31 ↗

(On Diff #68313)

Is it really optional? It seems like we should always have a TempFilename if we tried to write to the cache via getStream(). Maybe: "Path to temporary file used to buffer output that will be written to a cache entry when this object is destroyed."

32 ↗

(On Diff #68313)

Needs better description. E.g. "User-supplied callback to write the buffer loaded from cache into the native output object."

49 ↗

(On Diff #68313)

"return true and set EntryPath on cache hit."

53 ↗

(On Diff #68313)

Is it valid to create a CacheObjectOutput with an empty CacheDirectoryPath? Perhaps the constructor could assert that it is non-empty, and this could simply return true.

lib/LTO/Caching.cpp

30 ↗

(On Diff #68313)

Suggest comment like "Return if client did not attempt caching via tryLoadFromCache".

32 ↗

(On Diff #68313)

I think TempFilename would only be empty here if tryLoadFromCache() returned true (so we didn't need to write a new cache entry). A comment would be helpful. E.g. something like "TempFilename will be non-empty if a new cache entry was created, so save it to EntryPath in the cache."

50 ↗

(On Diff #68313)

It seems wrong that we don't have a return after this, otherwise in the case where a new cache entry is written AddBuffer() will be called twice. I guess it is simply inefficient but will work - you will load again from EntryPath that was just written, and rewrite to the output file.

53 ↗

(On Diff #68313)

Add a comment above here that this is the handling when tryLoadFromCache returned true and we can simply load the existing entry from the cache.

62 ↗

(On Diff #68313)

This would happen if the caching client didn't first call tryLoadFromCache before passing along the created CacheObjectOutput. Seems like it would be clearer to make this assert here, as that seems to be wrong usage of this class.

lib/LTO/LTO.cpp

48 ↗

(On Diff #68313)

add "and other global analysis results" after "export/import".

55 ↗

(On Diff #68313)

Add "." at end.

88 ↗

(On Diff #68313)

This is what the old version was doing, but here we don't have the list of preserved symbols here. Wouldn't this be the ExportedGUIDs set created in runThinLTO? That should presumably be passed down here too and used as in your existing cache key computation in ThinLTOCodeGenerator.

92 ↗

(On Diff #68313)

This should be hashing GUIDs not linkage.

501 ↗

(On Diff #68313)

As per offline discussion, move this block under isCachingEnabled()

514 ↗

(On Diff #68313)

What if caching not enabled? In that case we shouldn't call addOutput above, so we won't have an Output. It should presumably just call addOutput() here then before returning. Add a comment that this returns the cached output if enabled.

[mehdi_amini]

Address comments.

[mehdi_amini]

In D23599#521562, @tejohnson wrote:

I had a bit of trouble following the flow, so I added in some suggestions for more comments and some assertion checking.

I added a high level description above class NativeObjectOutput and class CacheObjectOutput. Let me know if it helps.

include/llvm/LTO/Caching.h

49 ↗

(On Diff #68313)

EntryPath is unconditionally set, I added this.

lib/LTO/Caching.cpp

50 ↗

(On Diff #68313)

Right, indeed I didn't intend to callback here, we should delete the temporary and reload the cache entry (mmap) and call the callback.

lib/LTO/LTO.cpp

88 ↗

(On Diff #68313)

Since internalization is done on the summary now, we don't really need to hash the "preserved symbols". We hash the linkage type from the index instead.

I updated the comment to reflect the change in the code.

92 ↗

(On Diff #68313)

Let me know if the above comment is enough.

514 ↗

(On Diff #68313)

We need the output in the first place to be able to check if the caching is enabled or not.

[mehdi_amini]

Minor update to a comment

[tejohnson]

Do the tests pass for you? I got a seg fault in llvm-lto2 in new test case, but didn't get a chance to look into it.

lib/LTO/Caching.cpp

21 ↗

(On Diff #68791)

FileSystem.h

[mehdi_amini]

In D23599#521798, @tejohnson wrote:

Do the tests pass for you? I got a seg fault in llvm-lto2 in new test case, but didn't get a chance to look into it.

Yes it is passing for me, I'll give a try with ASAN.

[mehdi_amini]

In D23599#521807, @mehdi_amini wrote:

In D23599#521798, @tejohnson wrote:

Do the tests pass for you? I got a seg fault in llvm-lto2 in new test case, but didn't get a chance to look into it.

Yes it is passing for me, I'll give a try with ASAN.

Oh wrong patch, I thought we were talking about the commons one, I'll retest this one later.

[tejohnson]

Looked at the llvm-lto2 core dump. The problem is that the Path variable declared in the AddOutput lambda is destroyed by the time it is used in the AddBuffer callback. Presumably it should be passed into the CacheObjectOutput constructor and a copy saved there.

[tejohnson]

The new comments help a lot. A few minor suggestions. Will add callback to gold-plugin and retry when the coredump resolved.

include/llvm/LTO/Caching.h

50 ↗

(On Diff #68791)

Note that when we return and Output is destroyed, the existing entry pulled from cache and passed back to client via CallBack.

56 ↗

(On Diff #68791)

Note that here, when Output is destroyed, the cache entry is updated in the cache and CallBack invoked to pass the contents back to the client.

71 ↗

(On Diff #68791)

or pulls it out of the cache if it was already there, and calls AddBuffer.

90 ↗

(On Diff #68791)

Remove "!CacheDirectoryPath.empty()"

lib/LTO/Caching.cpp

53 ↗

(On Diff #68791)

Combine the above 2 comment lines

lib/LTO/LTO.cpp

88 ↗

(On Diff #68791)

Ok got it.

[mehdi_amini]

In D23599#522167, @tejohnson wrote:

The new comments help a lot.

Yeah, I think it is a good habit, I really like such overview when I have to read code I don't know about, but I always forget to do the same on my code. Do no hesitate to point it out in future reviews :)

[mehdi_amini]

Address comments

[tejohnson]

Still needs fix for coredump (see my earlier response this morning on why that happens).

[mehdi_amini]

Make a copy of Path in the lambda.

[mehdi_amini]

Luckily you're a sharp reviewer :)

[tejohnson]

lib/LTO/Caching.cpp

71 ↗

(On Diff #68873)

My gold testing is hitting this assert many times. The reason is that when we call back to AddOutput, we don't know whether this is a regular or thin LTO call, so if there is a CacheDir specified we always create a CacheObjectOutput. If it is regularLTO, then we will eventually call getStream() from codegen without previously calling tryLoadFromCache.

This is happening in a couple of instances:

• since for ThinLTO when we have save-temps we setup a hook and are still unconditionally creating a combined LTO module and trying to code gen it
• my initial round of gold-plugin support was being lazy just for testing and always setting a CacheDir to something hardcoded (this would obviously go away and be set via a plugin-opt)

Even with the above two scenarios fixed, we want to support a mixed Thin and Regular LTO compilation model, so it needs to be handled. The easiest way seems to be to add a parameter to the AddOutput callback that takes an indication of which type of LTO this file is.

[mehdi_amini]

lib/LTO/Caching.cpp

71 ↗

(On Diff #68873)

Right, I was supporting this at some point and it got lost after some reviews...

I'd like to *not* change the AddOutput callback, and go back to what I was doing before, i.e. either:

• make it optional to call tryLoadFromCache, in which case we are using a temporary.
• Have LTO call tryLoadFromCache with an empty key, and have the logic handles the empty key as "uncached".

This would allow the monolithic LTO compilation to be cached as well in the future.

[tejohnson]

lib/LTO/Caching.cpp

71 ↗

(On Diff #68873)

IIRC correctly, what it did before was to return a nullptr when getStream() was called with an EmptyPath, which wouldn't work either.

make it optional to call tryLoadFromCache, in which case we are using a temporary.

I assume you mean when EntryPath is empty? In that case CacheObjectOutput::~CacheObjectOutput would also have to handle the empty EntryPath (load from temporary and call AddBuffer but don't cache I guess?).

Have LTO call tryLoadFromCache with an empty key, and have the logic handles the empty key as "uncached".

This would be similar I guess?

[mehdi_amini]

lib/LTO/Caching.cpp

71 ↗

(On Diff #68873)

I don't remember at which point it was lost, but at some point I had a call with an empty key in the regular LTO path, it is possible that it never made it to phab.

[mehdi_amini]

Correctly handle mix of ThinLTO and monolithic LTO together.

[tejohnson]

This now works with the support I added to gold-plugin (will commit as a follow-on) with the following change. This is required because append() will cause EntryPath to be CacheDirectoryPath+"/" and the test later in the destructor will not think they are equal.

diff --git a/lib/LTO/Caching.cpp b/lib/LTO/Caching.cpp
index 11e9fd6..9bba3ab 100644

• a/lib/LTO/Caching.cpp

+++ b/lib/LTO/Caching.cpp
@@ -93,10 +93,12 @@ std::unique_ptr<raw_pwrite_stream> CacheObjectOutput::getStream() {
bool CacheObjectOutput::tryLoadFromCache(StringRef Key) {

assert(!CacheDirectoryPath.empty() &&
       "CacheObjectOutput was initialized without a cache path");

• sys::path::append(EntryPath, CacheDirectoryPath, Key);
• if (Key.empty())
• // Client didn't compute a valid key. EntryPath has been set to

+ if (Key.empty()) {
+ // Client didn't compute a valid key. Set EntryPath to

// CacheDirectoryPath.

+ EntryPath = CacheDirectoryPath;

return false;

+ }
+ sys::path::append(EntryPath, CacheDirectoryPath, Key);

return sys::fs::exists(EntryPath);

}

lib/LTO/LTOBackend.cpp

248 ↗

(On Diff #68909)

Consider refactoring this so the above block from the AddOutput call through the codegen call are in a helper passed a ThreadId, then it can be called here and in splitCodeGen.

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

+//===- Caching.h - LLVM Link Time Optimizer Configuration -----------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines the lto::CacheObjectOutput data structure, which allows
+// clients to add a filesystem cache to ThinLTO
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LTO_CACHING_H
+#define LLVM_LTO_CACHING_H
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/LTO/Config.h"
+#include "llvm/Support/MemoryBuffer.h"
+
+namespace llvm {
+namespace lto {
+/// Type for client-supplied callback when a buffer is loaded from the cache.
+typedef std::function<void(std::unique_ptr<MemoryBuffer>)> AddBufferFn;
+
+/// Manage caching on the filesystem.
+///
+/// The general scheme is the following:
+///
+/// void do_stuff(AddBufferFn CallBack) {
+///   /* ... */
+///   {
+///     /* Create the CacheObjectOutput pointing to a cache directory */
+///     auto Output = CacheObjectOutput("/tmp/cache", CallBack)
+///
+///     /* Call some processing function */
+///     process(Output);
+///
+///   } /* Callback is only called now, on destruction of the Output object */
+///   /* ... */
+/// }
+///
+///
+/// void process(NativeObjectOutput &Output) {
+///   /* check if caching is supported */
+///   if (Output.isCachingEnabled()) {
+///     auto Key = ComputeKeyForEntry(...); // "expensive" call
+///     if (Output.tryLoadFromCache())
+///        return; // Cache hit
+///   }
+///
+///   auto OS = Output.getStream();
+///
+///   OS << ...;
+///   /* Note that the callback is not called here, but only when the caller
+///      destroys Output */
+/// }
+///
+class CacheObjectOutput : public NativeObjectOutput {
+  /// Path to the on-disk cache directory
+  StringRef CacheDirectoryPath;
+  /// Path to this entry in the cache, initialized by tryLoadFromCache().
+  SmallString<128> EntryPath;
+  /// Path to temporary file used to buffer output that will be committed to the
+  /// cache entry when this object is destroyed
+  SmallString<128> TempFilename;
+  /// User-supplied callback, called when the buffer is pulled out of the cache
+  /// (potentially after creating it).
+  AddBufferFn AddBuffer;
+
+public:
+  /// The destructor pulls the entry from the cache and calls the AddBuffer
+  /// callback, after committing the entry into the cache on miss.
+  ~CacheObjectOutput();
+
+  /// Create a CacheObjectOutput: the client is supposed to create it in the
+  /// callback supplied to LTO::run. The \p CacheDirectoryPath points to the
+  /// directory on disk where to store the cache, and \p AddBuffer will be
+  /// called when the buffer is pulled out of the cache (potentially after
+  /// creating it).
+  CacheObjectOutput(StringRef CacheDirectoryPath, AddBufferFn AddBuffer)
+      : CacheDirectoryPath(CacheDirectoryPath), AddBuffer(AddBuffer) {}
+
+  /// Return an allocated stream for the output, or null in case of failure.
+  std::unique_ptr<raw_pwrite_stream> getStream() override;
+
+  /// Set EntryPath, try loading from a possible cache first, return true on
+  /// cache hit.
+  bool tryLoadFromCache(StringRef Key) override;
+
+  /// Returns true to signal that this implementation of NativeObjectFile
+  /// support caching.
+  bool isCachingEnabled() const override { return true; }
+};
+
+} // namespace lto
+} // namespace llvm
+
+#endif

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

 class Module;
 class ModuleSummaryIndex;
 class raw_pwrite_stream;
 
 namespace lto {
 
 /// Abstract class representing a single Task output to be implemented by the
 /// client of the LTO API.
+///
+/// The general scheme the API is called is the following:
+///
+/// void process(NativeObjectOutput &Output) {
+///   /* check if caching is supported */
+///   if (Output.isCachingEnabled()) {
+///     auto Key = ComputeKeyForEntry(...); // "expensive" call
+///     if (Output.tryLoadFromCache())
+///        return; // Cache hit
+///   }
+///
+///   auto OS = Output.getStream();
+///
+///   OS << ....;
+/// }
+///
 class NativeObjectOutput {
 public:
   // Return an allocated stream for the output, or null in case of failure.
   virtual std::unique_ptr<raw_pwrite_stream> getStream() = 0;
+
+  // Try loading from a possible cache first, return true on cache hit.
+  virtual bool tryLoadFromCache(StringRef Key) { return false; }
+
+  // Returns true if a cache is available
+  virtual bool isCachingEnabled() const { return false; }
+
   virtual ~NativeObjectOutput() = default;
 };
 
 /// LTO configuration. A linker can configure LTO by setting fields in this data
 /// structure and passing it to the lto::LTO constructor.
 struct Config {
   std::string CPU;
   std::string Features;

llvm/trunk/lib/LTO/CMakeLists.txt

set_source_files_properties(Version.cpp
     PROPERTIES COMPILE_DEFINITIONS "HAVE_SVN_VERSION_INC")
 else()
   # Not producing a VC revision include.
   set(version_inc)
 endif()
 
 
 add_llvm_library(LLVMLTO
+  Caching.cpp
   LTO.cpp
   LTOBackend.cpp
   LTOModule.cpp
   LTOCodeGenerator.cpp
   UpdateCompilerUsed.cpp
   ThinLTOCodeGenerator.cpp
   ${version_inc}
 
   ADDITIONAL_HEADER_DIRS
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/LTO
 )
 
 add_dependencies(LLVMLTO intrinsics_gen)

llvm/trunk/lib/LTO/Caching.cpp

+//===-Caching.cpp - LLVM Link Time Optimizer Cache Handling ---------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file implements the Caching for ThinLTO.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/LTO/Caching.h"
+
+#ifdef HAVE_LLVM_REVISION
+#include "LLVMLTORevision.h"
+#endif
+
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace llvm;
+using namespace llvm::lto;
+
+static void commitEntry(StringRef TempFilename, StringRef EntryPath) {
+  // Rename to final destination (hopefully race condition won't matter here)
+  auto EC = sys::fs::rename(TempFilename, EntryPath);
+  if (EC) {
+    // Renaming failed, probably not the same filesystem, copy and delete.
+    {
+      auto ReloadedBufferOrErr = MemoryBuffer::getFile(TempFilename);
+      if (auto EC = ReloadedBufferOrErr.getError())
+        report_fatal_error(Twine("Failed to open temp file '") + TempFilename +
+                           "': " + EC.message() + "\n");
+
+      raw_fd_ostream OS(EntryPath, EC, sys::fs::F_None);
+      if (EC)
+        report_fatal_error(Twine("Failed to open ") + EntryPath +
+                           " to save cached entry\n");
+      // I'm not sure what are the guarantee if two processes are doing this
+      // at the same time.
+      OS << (*ReloadedBufferOrErr)->getBuffer();
+    }
+    sys::fs::remove(TempFilename);
+  }
+}
+
+CacheObjectOutput::~CacheObjectOutput() {
+  if (EntryPath.empty())
+    // The entry was never used by the client (tryLoadFromCache() wasn't called)
+    return;
+  // TempFilename is only set if getStream() was called, i.e. on cache miss when
+  // tryLoadFromCache() returned false. And EntryPath is valid if a Key was
+  // submitted, otherwise it has been set to CacheDirectoryPath in
+  // tryLoadFromCache.
+  if (!TempFilename.empty()) {
+    if (EntryPath == CacheDirectoryPath)
+      // The Key supplied to tryLoadFromCache was empty, do not commit the temp.
+      EntryPath = TempFilename;
+    else
+      // We commit the tempfile into the cache now, by moving it to EntryPath.
+      commitEntry(TempFilename, EntryPath);
+  }
+  // Load the entry from the cache now.
+  auto ReloadedBufferOrErr = MemoryBuffer::getFile(EntryPath);
+  if (auto EC = ReloadedBufferOrErr.getError())
+    report_fatal_error(Twine("Can't reload cached file '") + EntryPath + "': " +
+                       EC.message() + "\n");
+
+  // Supply the resulting buffer to the user.
+  AddBuffer(std::move(*ReloadedBufferOrErr));
+}
+
+// Return an allocated stream for the output, or null in case of failure.
+std::unique_ptr<raw_pwrite_stream> CacheObjectOutput::getStream() {
+  assert(!EntryPath.empty() && "API Violation: client didn't call "
+                               "tryLoadFromCache() before getStream()");
+  // Write to a temporary to avoid race condition
+  int TempFD;
+  std::error_code EC =
+      sys::fs::createTemporaryFile("Thin", "tmp.o", TempFD, TempFilename);
+  if (EC) {
+    errs() << "Error: " << EC.message() << "\n";
+    report_fatal_error("ThinLTO: Can't get a temporary file");
+  }
+  return llvm::make_unique<raw_fd_ostream>(TempFD, /* ShouldClose */ true);
+}
+
+// Try loading from a possible cache first, return true on cache hit.
+bool CacheObjectOutput::tryLoadFromCache(StringRef Key) {
+  assert(!CacheDirectoryPath.empty() &&
+         "CacheObjectOutput was initialized without a cache path");
+  if (Key.empty()) {
+    // Client didn't compute a valid key. EntryPath has been set to
+    // CacheDirectoryPath.
+    EntryPath = CacheDirectoryPath;
+    return false;
+  }
+  sys::path::append(EntryPath, CacheDirectoryPath, Key);
+  return sys::fs::exists(EntryPath);
+}

llvm/trunk/lib/LTO/LTO.cpp

 #include "llvm/IR/DiagnosticPrinter.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/LTO/LTOBackend.h"
 #include "llvm/Linker/IRMover.h"
 #include "llvm/Object/ModuleSummaryIndexObjectFile.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/SHA1.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Target/TargetOptions.h"
 #include "llvm/Transforms/IPO.h"
 #include "llvm/Transforms/IPO/PassManagerBuilder.h"
 #include "llvm/Transforms/Utils/SplitModule.h"
 
 #include <set>
 
 using namespace llvm;
 using namespace lto;
 using namespace object;
 
+#define DEBUG_TYPE "lto"
+
+// Returns a unique hash for the Module considering the current list of
+// export/import and other global analysis results.
+// The hash is produced in \p Key.
+static void computeCacheKey(
+    SmallString<40> &Key, const ModuleSummaryIndex &Index, StringRef ModuleID,
+    const FunctionImporter::ImportMapTy &ImportList,
+    const FunctionImporter::ExportSetTy &ExportList,
+    const std::map<GlobalValue::GUID, GlobalValue::LinkageTypes> &ResolvedODR,
+    const GVSummaryMapTy &DefinedGlobals) {
+  // Compute the unique hash for this entry.
+  // This is based on the current compiler version, the module itself, the
+  // export list, the hash for every single module in the import list, the
+  // list of ResolvedODR for the module, and the list of preserved symbols.
+  SHA1 Hasher;
+
+  // Start with the compiler revision
+  Hasher.update(LLVM_VERSION_STRING);
+#ifdef HAVE_LLVM_REVISION
+  Hasher.update(LLVM_REVISION);
+#endif
+
+  // Include the hash for the current module
+  auto ModHash = Index.getModuleHash(ModuleID);
+  Hasher.update(ArrayRef<uint8_t>((uint8_t *)&ModHash[0], sizeof(ModHash)));
+  for (auto F : ExportList)
+    // The export list can impact the internalization, be conservative here
+    Hasher.update(ArrayRef<uint8_t>((uint8_t *)&F, sizeof(F)));
+
+  // Include the hash for every module we import functions from
+  for (auto &Entry : ImportList) {
+    auto ModHash = Index.getModuleHash(Entry.first());
+    Hasher.update(ArrayRef<uint8_t>((uint8_t *)&ModHash[0], sizeof(ModHash)));
+  }
+
+  // Include the hash for the resolved ODR.
+  for (auto &Entry : ResolvedODR) {
+    Hasher.update(ArrayRef<uint8_t>((const uint8_t *)&Entry.first,
+                                    sizeof(GlobalValue::GUID)));
+    Hasher.update(ArrayRef<uint8_t>((const uint8_t *)&Entry.second,
+                                    sizeof(GlobalValue::LinkageTypes)));
+  }
+
+  // Include the hash for the linkage type to reflect internalization and weak
+  // resolution.
+  for (auto &GS : DefinedGlobals) {
+    GlobalValue::LinkageTypes Linkage = GS.second->linkage();
+    Hasher.update(
+        ArrayRef<uint8_t>((const uint8_t *)&Linkage, sizeof(Linkage)));
+  }
+
+  Key = toHex(Hasher.result());
+}
+
 // Simple helper to load a module from bitcode
 std::unique_ptr<Module>
 llvm::loadModuleFromBuffer(const MemoryBufferRef &Buffer, LLVMContext &Context,
                            bool Lazy) {
   SMDiagnostic Err;
   ErrorOr<std::unique_ptr<Module>> ModuleOrErr(nullptr);
   if (Lazy) {
     ModuleOrErr =

 
 public:
   ThinBackendProc(Config &Conf, ModuleSummaryIndex &CombinedIndex,
                   StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries)
       : Conf(Conf), CombinedIndex(CombinedIndex),
         ModuleToDefinedGVSummaries(ModuleToDefinedGVSummaries) {}
 
   virtual ~ThinBackendProc() {}
-  virtual Error start(unsigned Task, MemoryBufferRef MBRef,
-                      const FunctionImporter::ImportMapTy &ImportList,
-                      MapVector<StringRef, MemoryBufferRef> &ModuleMap) = 0;
+  virtual Error start(
+      unsigned Task, MemoryBufferRef MBRef,
+      const FunctionImporter::ExportSetTy &ExportList,
+      const std::map<GlobalValue::GUID, GlobalValue::LinkageTypes> &ResolvedODR,
   virtual Error wait() = 0;
 };
 
 class InProcessThinBackend : public ThinBackendProc {
   ThreadPool BackendThreadPool;
   AddOutputFn AddOutput;
 
   Optional<Error> Err;
   std::mutex ErrMu;
 
 public:
   InProcessThinBackend(Config &Conf, ModuleSummaryIndex &CombinedIndex,
                        unsigned ThinLTOParallelismLevel,
                        StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries,
                        AddOutputFn AddOutput)
       : ThinBackendProc(Conf, CombinedIndex, ModuleToDefinedGVSummaries),
         BackendThreadPool(ThinLTOParallelismLevel),
         AddOutput(std::move(AddOutput)) {}
 
-  Error
-  runThinLTOBackendThread(AddOutputFn AddOutput, unsigned Task,
-                          MemoryBufferRef MBRef,
-                          ModuleSummaryIndex &CombinedIndex,
-                          const FunctionImporter::ImportMapTy &ImportList,
-                          const GVSummaryMapTy &DefinedGlobals,
-                          MapVector<StringRef, MemoryBufferRef> &ModuleMap) {
-    LTOLLVMContext BackendContext(Conf);
+  Error runThinLTOBackendThread(
+      AddOutputFn AddOutput, unsigned Task, MemoryBufferRef MBRef,
+      const FunctionImporter::ExportSetTy &ExportList,
+      const std::map<GlobalValue::GUID, GlobalValue::LinkageTypes> &ResolvedODR,
 
+    auto ModuleIdentifier = MBRef.getBufferIdentifier();
+    auto Output = AddOutput(Task);
+    if (Output->isCachingEnabled()) {
+      SmallString<40> Key;
+      // The module may be cached, this helps handling it.
+      computeCacheKey(Key, CombinedIndex, ModuleIdentifier, ImportList,
+                      ExportList, ResolvedODR, DefinedGlobals);
+      if (Output->tryLoadFromCache(Key))
+        return Error();
+    }
+
+    LTOLLVMContext BackendContext(Conf);
     ErrorOr<std::unique_ptr<Module>> MOrErr =
         parseBitcodeFile(MBRef, BackendContext);
     assert(MOrErr && "Unable to load module in thread?");
 
-    return thinBackend(Conf, Task, AddOutput, **MOrErr, CombinedIndex,
+    auto AddOutputWrapper = [&](unsigned TaskId) {
+      assert(Task == TaskId && "Unexpexted TaskId mismatch");
+      return std::move(Output);
+    };
+    return thinBackend(Conf, Task, AddOutputWrapper, **MOrErr, CombinedIndex,
                        ImportList, DefinedGlobals, ModuleMap);
   }
 
-  Error start(unsigned Task, MemoryBufferRef MBRef,
-              const FunctionImporter::ImportMapTy &ImportList,
-              MapVector<StringRef, MemoryBufferRef> &ModuleMap) override {
+  Error start(
+      unsigned Task, MemoryBufferRef MBRef,
+      const FunctionImporter::ExportSetTy &ExportList,
+      const std::map<GlobalValue::GUID, GlobalValue::LinkageTypes> &ResolvedODR,
     StringRef ModulePath = MBRef.getBufferIdentifier();
     BackendThreadPool.async(
         [=](MemoryBufferRef MBRef, ModuleSummaryIndex &CombinedIndex,
             const FunctionImporter::ImportMapTy &ImportList,
+            const FunctionImporter::ExportSetTy &ExportList,
+            const std::map<GlobalValue::GUID, GlobalValue::LinkageTypes>
+                &ResolvedODR,
             GVSummaryMapTy &DefinedGlobals,
             MapVector<StringRef, MemoryBufferRef> &ModuleMap) {
-          Error E =
-              runThinLTOBackendThread(AddOutput, Task, MBRef, CombinedIndex,
-                                      ImportList, DefinedGlobals, ModuleMap);
+          Error E = runThinLTOBackendThread(
+              AddOutput, Task, MBRef, 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),
+        std::ref(ExportList), std::ref(ResolvedODR),
         std::ref(ModuleToDefinedGVSummaries[ModulePath]), std::ref(ModuleMap));
     return Error();
   }
 
   Error wait() override {
     BackendThreadPool.wait();
     if (Err)
       return std::move(*Err);
if (!ParentPath.empty()) {
       // Make sure the new directory exists, creating it if necessary.
       if (std::error_code EC = llvm::sys::fs::create_directories(ParentPath))
         llvm::errs() << "warning: could not create directory '" << ParentPath
                      << "': " << EC.message() << '\n';
     }
     return NewPath.str();
   }
 
-  Error start(unsigned Task, MemoryBufferRef MBRef,
-              const FunctionImporter::ImportMapTy &ImportList,
-              MapVector<StringRef, MemoryBufferRef> &ModuleMap) override {
+  Error start(
+      unsigned Task, MemoryBufferRef MBRef,
+      const FunctionImporter::ExportSetTy &ExportList,
+      const std::map<GlobalValue::GUID, GlobalValue::LinkageTypes> &ResolvedODR,
     StringRef ModulePath = MBRef.getBufferIdentifier();
     std::string NewModulePath =
         getThinLTOOutputFile(ModulePath, OldPrefix, NewPrefix);
 
     std::error_code EC;
     if (!LinkedObjectsFileName.empty()) {
       if (!LinkedObjectsFile) {
         LinkedObjectsFile = llvm::make_unique<raw_fd_ostream>(
Error LTO::runThinLTO(AddOutputFn AddOutput) {
   };
   auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) {
     const auto &ExportList = ExportLists.find(ModuleIdentifier);
     return (ExportList != ExportLists.end() &&
             ExportList->second.count(GUID)) ||
            ExportedGUIDs.count(GUID);
   };
   thinLTOInternalizeAndPromoteInIndex(ThinLTO.CombinedIndex, isExported);
-  thinLTOResolveWeakForLinkerInIndex(
-      ThinLTO.CombinedIndex, isPrevailing,
-      [](StringRef, GlobalValue::GUID, GlobalValue::LinkageTypes) {});
+
+  StringMap<std::map<GlobalValue::GUID, GlobalValue::LinkageTypes>> ResolvedODR;
+  auto recordNewLinkage = [&](StringRef ModuleIdentifier,
+                              GlobalValue::GUID GUID,
+                              GlobalValue::LinkageTypes NewLinkage) {
+    ResolvedODR[ModuleIdentifier][GUID] = NewLinkage;
+  };
+
+  thinLTOResolveWeakForLinkerInIndex(ThinLTO.CombinedIndex, isPrevailing,
+                                     recordNewLinkage);
 
   std::unique_ptr<ThinBackendProc> BackendProc = ThinLTO.Backend(
       Conf, ThinLTO.CombinedIndex, ModuleToDefinedGVSummaries, AddOutput);
 
   // Partition numbers for ThinLTO jobs start at 1 (see comments for
   // GlobalResolution in LTO.h). Task numbers, however, start at
-  // ParallelCodeGenParallelismLevel, as tasks 0 through
-  // ParallelCodeGenParallelismLevel-1 are reserved for parallel code generation
-  // partitions.
+  // ParallelCodeGenParallelismLevel if an LTO module is present, as tasks 0
+  // through ParallelCodeGenParallelismLevel-1 are reserved for parallel code
+  // generation partitions.
   unsigned Task = RegularLTO.CombinedModule
                       ? RegularLTO.ParallelCodeGenParallelismLevel
                       : 0;
   unsigned Partition = 1;
 
   for (auto &Mod : ThinLTO.ModuleMap) {
     if (Error E = BackendProc->start(Task, Mod.second, ImportLists[Mod.first],
-                                     ThinLTO.ModuleMap))
+                                     ExportLists[Mod.first],
+                                     ResolvedODR[Mod.first], ThinLTO.ModuleMap))
       return E;
 
     ++Task;
     ++Partition;
   }
 
   return BackendProc->wait();
 }

llvm/trunk/lib/LTO/LTOBackend.cpp

bool opt(Config &C, TargetMachine *TM, unsigned Task, Module &M,
   passes.run(M);
 
   if (C.PostOptModuleHook && !C.PostOptModuleHook(Task, M))
     return false;
 
   return true;
 }
 
+/// Monolithic LTO does not support caching (yet), this is a convenient wrapper
+/// around AddOutput to workaround this.
+static AddOutputFn getUncachedOutputWrapper(AddOutputFn &AddOutput,
+                                            unsigned Task) {
+  return [Task, &AddOutput](unsigned TaskId) {
+    auto Output = AddOutput(Task);
+    if (Output->isCachingEnabled() && Output->tryLoadFromCache(""))
+      report_fatal_error("Cache hit without a valid key?");
+    errs() << Task << " == " << TaskId << "\n";
+    assert(Task == TaskId && "Unexpexted TaskId mismatch");
+    return Output;
+  };
+}
+
 void codegen(Config &C, TargetMachine *TM, AddOutputFn AddOutput, unsigned Task,
              Module &M) {
   if (C.PreCodeGenModuleHook && !C.PreCodeGenModuleHook(Task, M))
     return;
 
   auto Output = AddOutput(Task);
   std::unique_ptr<raw_pwrite_stream> OS = Output->getStream();
   legacy::PassManager CodeGenPasses;
SplitModule(
                   MemoryBufferRef(StringRef(BC.data(), BC.size()), "ld-temp.o"),
                   Ctx);
               if (!MOrErr)
                 report_fatal_error("Failed to read bitcode");
               std::unique_ptr<Module> MPartInCtx = std::move(MOrErr.get());
 
               std::unique_ptr<TargetMachine> TM =
                   createTargetMachine(C, MPartInCtx->getTargetTriple(), T);
-              codegen(C, TM.get(), AddOutput, ThreadId, *MPartInCtx);
+
+              codegen(C, TM.get(),
+                      getUncachedOutputWrapper(AddOutput, ThreadId), ThreadId,
+                      *MPartInCtx);
             },
             // Pass BC using std::move to ensure that it get moved rather than
             // copied into the thread's context.
             std::move(BC), ThreadCount++);
       },
       false);
 }
 
Error lto::backend(Config &C, AddOutputFn AddOutput,
 
   std::unique_ptr<TargetMachine> TM =
       createTargetMachine(C, M->getTargetTriple(), *TOrErr);
 
   if (!C.CodeGenOnly)
     if (!opt(C, TM.get(), 0, *M, /*IsThinLto=*/false))
       return Error();
 
-  if (ParallelCodeGenParallelismLevel == 1)
-    codegen(C, TM.get(), AddOutput, 0, *M);
-  else
+  if (ParallelCodeGenParallelismLevel == 1) {
+    codegen(C, TM.get(), getUncachedOutputWrapper(AddOutput, 0), 0, *M);
+  } else {
     splitCodeGen(C, TM.get(), AddOutput, ParallelCodeGenParallelismLevel,
                  std::move(M));
+  }
   return Error();
 }
 
 Error lto::thinBackend(Config &Conf, unsigned Task, AddOutputFn AddOutput,
                        Module &Mod, ModuleSummaryIndex &CombinedIndex,
                        const FunctionImporter::ImportMapTy &ImportList,
                        const GVSummaryMapTy &DefinedGlobals,
                        MapVector<StringRef, MemoryBufferRef> &ModuleMap) {

llvm/trunk/test/ThinLTO/X86/cache.ll

 ; RUN: opt -module-summary %s -o %t.bc
-; RUN: opt -module-summary %p/Inputs/funcimport.ll -o %t2.bc
+; RUN: opt -module-summary %p/Inputs/cache.ll -o %t2.bc
 
 ; Verify that enabling caching is working
 ; RUN: rm -Rf %t.cache && mkdir %t.cache
 ; RUN: llvm-lto -thinlto-action=run -exported-symbol=globalfunc %t2.bc  %t.bc -thinlto-cache-dir %t.cache
 ; RUN: ls %t.cache/llvmcache.timestamp
 ; RUN: ls %t.cache | count 3
 
+; Verify that enabling caching is working with llvm-lto2
+; RUN: rm -Rf %t.cache && mkdir %t.cache
+; RUN: llvm-lto2 -o %t.o %t2.bc  %t.bc -cache-dir %t.cache \
+; RUN:  -r=%t2.bc,_main,plx \
+; RUN:  -r=%t2.bc,_globalfunc,lx \
+; RUN:  -r=%t.bc,_globalfunc,plx
+; RUN: ls %t.cache | count 2
+
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.11.0"
 
 define void @globalfunc() #0 {
 entry:
   ret void
 }

llvm/trunk/tools/llvm-lto2/llvm-lto2.cpp

 // link-time optimization according to the provided symbol resolutions using the
 // resolution-based LTO interface, and outputs one or more object files.
 //
 // This program is intended to eventually replace llvm-lto which uses the legacy
 // LTO interface.
 //
 //===----------------------------------------------------------------------===//
 
+#include "llvm/LTO/Caching.h"
 #include "llvm/LTO/LTO.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/TargetSelect.h"
 
 using namespace llvm;
 using namespace lto;
 using namespace object;
 
 static cl::list<std::string> InputFilenames(cl::Positional, cl::OneOrMore,
                                             cl::desc("<input bitcode files>"));
 
 static cl::opt<std::string> OutputFilename("o", cl::Required,
                                            cl::desc("Output filename"),
                                            cl::value_desc("filename"));
 
+static cl::opt<std::string> CacheDir("cache-dir", cl::desc("Cache Directory"),
+                                     cl::value_desc("directory"));
+
 static cl::opt<bool> SaveTemps("save-temps", cl::desc("Save temporary files"));
 
 static cl::opt<bool>
     ThinLTODistributedIndexes("thinlto-distributed-indexes", cl::init(false),
                               cl::desc("Write out individual index and "
                                        "import files for the "
                                        "distributed backend case"));
 
if (!CommandLineResolutions.empty()) {
     for (auto UnusedRes : CommandLineResolutions)
       llvm::errs() << argv[0] << ": unused symbol resolution for "
                    << UnusedRes.first.first << ',' << UnusedRes.first.second
                    << '\n';
   }
   if (HasErrors)
     return 1;
 
-  auto AddOutput = [&](size_t Task) {
+  auto AddOutput =
+      [&](size_t Task) -> std::unique_ptr<lto::NativeObjectOutput> {
     std::string Path = OutputFilename + "." + utostr(Task);
+    if (CacheDir.empty())
       return llvm::make_unique<LTOOutput>(std::move(Path));
+
+    return llvm::make_unique<CacheObjectOutput>(
+        CacheDir, [Path](std::unique_ptr<MemoryBuffer> Buffer) {
+          *LTOOutput(Path).getStream() << Buffer->getBuffer();
+        });
   };
 
   check(Lto.run(AddOutput), "LTO::run failed");
 }