ThinLTO: handles modules with empty summaries

We need to add an entry in the combined-index for modules that have
a hash but otherwise empty summary, this is needed so that we can
get the hash for the module.

Also, if no entry is present in the combined index for a module, we
need to skip it when trying to compute a cache entry.

Differential Revision: https://reviews.llvm.org/D25300

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@283654 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp
index 101e8eb..df9ab1c 100644 (file)
--- a/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -6147,8 +6147,8 @@ std::error_code ModuleSummaryIndexBitcodeReader::parseModule() {
           if (!TheIndex)
             break;
           if (TheIndex->modulePaths().empty())
-            // Does not have any summary emitted.
-            break;
+            // We always seed the index with the module.
+            TheIndex->addModulePath(Buffer->getBufferIdentifier(), 0);
           if (TheIndex->modulePaths().size() != 1)
             return error("Don't expect multiple modules defined?");
           auto &Hash = TheIndex->modulePaths().begin()->second.second;

diff --git a/lib/IR/ModuleSummaryIndex.cpp b/lib/IR/ModuleSummaryIndex.cpp
index 6107cf4..9072f4b 100644 (file)
--- a/lib/IR/ModuleSummaryIndex.cpp
+++ b/lib/IR/ModuleSummaryIndex.cpp
@@ -20,8 +20,17 @@ using namespace llvm;
 // per-module instances.
 void ModuleSummaryIndex::mergeFrom(std::unique_ptr<ModuleSummaryIndex> Other,
                                    uint64_t NextModuleId) {
+  if (Other->modulePaths().empty())
+    return;
+
+  assert(Other->modulePaths().size() == 1 &&
+         "Can only merge from an single-module index at that time");
+
+  StringRef OtherModPath = Other->modulePaths().begin()->first();
+  StringRef ModPath = addModulePath(OtherModPath, NextModuleId,
+                                    Other->getModuleHash(OtherModPath))
+                          ->first();
 
-  StringRef ModPath;
   for (auto &OtherGlobalValSummaryLists : *Other) {
     GlobalValue::GUID ValueGUID = OtherGlobalValSummaryLists.first;
     GlobalValueSummaryList &List = OtherGlobalValSummaryLists.second;
@@ -31,16 +40,6 @@ void ModuleSummaryIndex::mergeFrom(std::unique_ptr<ModuleSummaryIndex> Other,
     assert(List.size() == 1);
     std::unique_ptr<GlobalValueSummary> Summary = std::move(List.front());
 
-    // Add the module path string ref for this module if we haven't already
-    // saved a reference to it.
-    if (ModPath.empty()) {
-      auto Path = Summary->modulePath();
-      ModPath = addModulePath(Path, NextModuleId, Other->getModuleHash(Path))
-                    ->first();
-    } else
-      assert(ModPath == Summary->modulePath() &&
-             "Each module in the combined map should have a unique ID");
-
     // Note the module path string ref was copied above and is still owned by
     // the original per-module index. Reset it to the new module path
     // string reference owned by the combined index.

diff --git a/lib/LTO/LTO.cpp b/lib/LTO/LTO.cpp
index b2d42f4..94a4abf 100644 (file)
--- a/lib/LTO/LTO.cpp
+++ b/lib/LTO/LTO.cpp
@@ -541,13 +541,15 @@ public:
                          ImportList, DefinedGlobals, ModuleMap);
     };
 
-    if (!Cache)
+    auto ModuleID = MBRef.getBufferIdentifier();
+    if (!Cache || !CombinedIndex.modulePaths().count(ModuleID))
+      // Cache disabled or no entry for this module in the combined index
       return RunThinBackend(AddStream);
 
     SmallString<40> Key;
     // The module may be cached, this helps handling it.
-    computeCacheKey(Key, CombinedIndex, MBRef.getBufferIdentifier(),
-                    ImportList, ExportList, ResolvedODR, DefinedGlobals);
+    computeCacheKey(Key, CombinedIndex, ModuleID, ImportList, ExportList,
+                    ResolvedODR, DefinedGlobals);
     if (AddStreamFn CacheAddStream = Cache(Task, Key))
       return RunThinBackend(CacheAddStream);
 

diff --git a/lib/LTO/ThinLTOCodeGenerator.cpp b/lib/LTO/ThinLTOCodeGenerator.cpp
index f4232dc..8afe13d 100644 (file)
--- a/lib/LTO/ThinLTOCodeGenerator.cpp
+++ b/lib/LTO/ThinLTOCodeGenerator.cpp
@@ -234,6 +234,10 @@ public:
     if (CachePath.empty())
       return;
 
+    if (!Index.modulePaths().count(ModuleID))
+      // The module does not have an entry, it can't have a hash at all
+      return;
+
     // 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

diff --git a/test/ThinLTO/X86/Inputs/empty_module_with_cache.ll b/test/ThinLTO/X86/Inputs/empty_module_with_cache.ll
new file mode 100644 (file)
index 0000000..1c55e07
--- /dev/null
+++ b/test/ThinLTO/X86/Inputs/empty_module_with_cache.ll
@@ -0,0 +1,8 @@
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.11.0"
+
+
+define i32 @main() {
+entry:
+  ret i32 0
+}

diff --git a/test/ThinLTO/X86/empty_module_with_cache.ll b/test/ThinLTO/X86/empty_module_with_cache.ll
new file mode 100644 (file)
index 0000000..c627451
--- /dev/null
+++ b/test/ThinLTO/X86/empty_module_with_cache.ll
@@ -0,0 +1,35 @@
+; RUN: opt -module-hash -module-summary %s -o %t.bc
+; RUN: opt -module-hash -module-summary %p/Inputs/empty_module_with_cache.ll -o %t2.bc
+
+; Verify that enabling caching is working, even if the module is empty
+; RUN: rm -Rf %t.cache && mkdir %t.cache
+; RUN: llvm-lto -thinlto-action=run %t2.bc  %t.bc -exported-symbol=main -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: ls %t.cache | count 2
+
+; Same, but without hash, the index will be empty and caching should not happen
+
+; RUN: opt -module-summary %s -o %t.bc
+; RUN: opt -module-summary %p/Inputs/empty_module_with_cache.ll -o %t2.bc
+
+; Verify that caching is disabled for module without hash
+; RUN: rm -Rf %t.cache && mkdir %t.cache
+; RUN: llvm-lto -thinlto-action=run %t2.bc  %t.bc -exported-symbol=main -thinlto-cache-dir %t.cache
+; RUN: ls %t.cache/llvmcache.timestamp
+; RUN: ls %t.cache | count 2
+
+; Verify that caching is disabled for module without hash, 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: ls %t.cache | count 1
+
+
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.11.0"

diff --git a/tools/llvm-lto/llvm-lto.cpp b/tools/llvm-lto/llvm-lto.cpp
index 2e9d8b7..ddaedd0 100644 (file)
--- a/tools/llvm-lto/llvm-lto.cpp
+++ b/tools/llvm-lto/llvm-lto.cpp
@@ -490,6 +490,8 @@ private:
     }
 
     auto CombinedIndex = ThinGenerator.linkCombinedIndex();
+    if (!CombinedIndex)
+      report_fatal_error("ThinLink didn't create an index");
     std::error_code EC;
     raw_fd_ostream OS(OutputFilename, EC, sys::fs::OpenFlags::F_None);
     error(EC, "error opening the file '" + OutputFilename + "'");