[llvm-branch-commits] [llvm] cff7a77 - [ThinLTO] Use module hash instead of module ID for cache key

[Tobias Hieta via llvm-branch-commits]

Author: Nikita Popov
Date: 2023-07-29T08:40:45+02:00
New Revision: cff7a7747db02d1214b20e98677e5ddcb402ffe0

URL: https://github.com/llvm/llvm-project/commit/cff7a7747db02d1214b20e98677e5ddcb402ffe0
DIFF: https://github.com/llvm/llvm-project/commit/cff7a7747db02d1214b20e98677e5ddcb402ffe0.diff

LOG: [ThinLTO] Use module hash instead of module ID for cache key

This is a followup to D151165. Instead of using the module ID, use
the module hash for sorting the import list. The module hash is what
will actually be included in the hash.

This has the advantage of being independent of the module order,
which is something that Rust relies on.

A caveat here is that the test doesn't quite work for linkonce_odr
functions, because the function may be imported from two different
modules, and the first one on the llvm-lto2 command line gets picked
(rather than, say, the prevailing copy). This doesn't really matter
for Rust's purposes (because it does not use linkonce_odr linkage),
but may still be worth addressing. For now I'm using a variant of
the test using internal instead of linkonce_odr functions.

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

(cherry picked from commit 279c2971951c2ea58a2bd1e6687ce61451f9d329)

Added: 
    llvm/test/ThinLTO/X86/Inputs/cache-import-lists3.ll
    llvm/test/ThinLTO/X86/Inputs/cache-import-lists4.ll
    llvm/test/ThinLTO/X86/cache-decoupled-from-order.ll

Modified: 
    llvm/lib/LTO/LTO.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index 6803d6ab1285a1..bc8abb751221ce 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -184,7 +184,6 @@ void llvm::computeLTOCacheKey(
     }
 
     const ModuleHash &getHash() const { return ModInfo->second.second; }
-    uint64_t getId() const { return ModInfo->second.first; }
   };
 
   std::vector<ImportModule> ImportModulesVector;
@@ -194,11 +193,11 @@ void llvm::computeLTOCacheKey(
        ++It) {
     ImportModulesVector.push_back({It, Index.getModule(It->getKey())});
   }
-  // Order using moduleId integer which is based on the order the module was
-  // added.
+  // Order using module hash, to be both independent of module name and
+  // module order.
   llvm::sort(ImportModulesVector,
              [](const ImportModule &Lhs, const ImportModule &Rhs) -> bool {
-               return Lhs.getId() < Rhs.getId();
+               return Lhs.getHash() < Rhs.getHash();
              });
   for (const ImportModule &Entry : ImportModulesVector) {
     auto ModHash = Entry.getHash();

diff  --git a/llvm/test/ThinLTO/X86/Inputs/cache-import-lists3.ll b/llvm/test/ThinLTO/X86/Inputs/cache-import-lists3.ll
new file mode 100644
index 00000000000000..92247d1f613669
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/Inputs/cache-import-lists3.ll
@@ -0,0 +1,11 @@
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @f1() {
+  call void @internal()
+  ret void
+}
+
+define internal void @internal() {
+  ret void
+}

diff  --git a/llvm/test/ThinLTO/X86/Inputs/cache-import-lists4.ll b/llvm/test/ThinLTO/X86/Inputs/cache-import-lists4.ll
new file mode 100644
index 00000000000000..281905f6a0efd0
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/Inputs/cache-import-lists4.ll
@@ -0,0 +1,11 @@
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @f2() {
+  call void @internal()
+  ret void
+}
+
+define internal void @internal() {
+  ret void
+}

diff  --git a/llvm/test/ThinLTO/X86/cache-decoupled-from-order.ll b/llvm/test/ThinLTO/X86/cache-decoupled-from-order.ll
new file mode 100644
index 00000000000000..88717507c52210
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/cache-decoupled-from-order.ll
@@ -0,0 +1,24 @@
+; RUN: rm -rf %t && mkdir -p %t
+; RUN: opt -module-hash -module-summary %s -o %t/t.bc
+; RUN: opt -module-hash -module-summary %S/Inputs/cache-import-lists3.ll -o %t/a.bc
+; RUN: opt -module-hash -module-summary %S/Inputs/cache-import-lists4.ll -o %t/b.bc
+
+; Tests that the hash for t is insensitive to the order of the modules.
+
+; RUN: llvm-lto2 run -cache-dir %t/cache -o %t.o %t/t.bc %t/a.bc %t/b.bc -r=%t/t.bc,main,plx -r=%t/t.bc,f1,lx -r=%t/t.bc,f2,lx -r=%t/a.bc,f1,plx -r=%t/b.bc,f2,plx
+; RUN: ls %t/cache | count 3
+
+; RUN: llvm-lto2 run -cache-dir %t/cache -o %t.o %t/b.bc %t/a.bc %t/t.bc -r=%t/t.bc,main,plx -r=%t/t.bc,f1,lx -r=%t/t.bc,f2,lx -r=%t/a.bc,f1,plx -r=%t/b.bc,f2,plx
+; RUN: ls %t/cache | count 3
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @main() {
+  call void @f1()
+  call void @f2()
+  ret void
+}
+
+declare void @f1()
+declare void @f2()