serge-sans-paille created this revision.
serge-sans-paille added reviewers: dexonsmith, zturner.
serge-sans-paille added a project: clang.
Herald added a subscriber: cfe-commits.
Previous implementation was incorrectly passing an integer, that got converted
to a pointer, to finalize the hash computation. This led to different functions
having the same hash if they only differ by the remaining statements, which is
incorrect.
Added a new test case that trivially tests that a small function change is
reflected in the hash value.
Not that as this patch fixes the hash computation, it invalidates all hashes
computed before that patch applies, which could be an issue for large build
system that pre-compute the profile data and let client download them as part of
the build process.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D79961
Files:
clang/lib/CodeGen/CodeGenPGO.cpp
clang/test/Profile/c-collision.c
Index: clang/test/Profile/c-collision.c
===================================================================
--- /dev/null
+++ clang/test/Profile/c-collision.c
@@ -0,0 +1,26 @@
+// Test that a slight change in the code leads to a different hash.
+// RUN: %clang_cc1 -UEXTRA -triple x86_64-unknown-linux-gnu -main-file-name
c-collision.c %s -o - -emit-llvm -fprofile-instrument=llvm | FileCheck %s
--check-prefix=CHECK-NOEXTRA
+// RUN: %clang_cc1 -DEXTRA -triple x86_64-unknown-linux-gnu -main-file-name
c-collision.c %s -o - -emit-llvm -fprofile-instrument=llvm | FileCheck %s
--check-prefix=CHECK-EXTRA
+
+// CHECK-NOEXTRA: @__profd_main = private global { {{.*}} } { i64
-2624081020897602054, i64 88870893692,
+// CHECK-EXTRA: @__profd_main = private global { {{.*}} } { i64
-2624081020897602054, i64 101970072141,
+
+int main(int n, char **v) {
+ if (n)
+ n += 1;
+ if (n)
+ n += 1;
+ if (n)
+ n += 1;
+ if (n)
+ n += 1;
+ if (n)
+ n += 1;
+ if (n)
+ n += 1;
+#ifdef EXTRA
+ if (n)
+ n += 1;
+#endif
+ return n;
+}
Index: clang/lib/CodeGen/CodeGenPGO.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenPGO.cpp
+++ clang/lib/CodeGen/CodeGenPGO.cpp
@@ -747,8 +747,11 @@
return Working;
// Check for remaining work in Working.
- if (Working)
- MD5.update(Working);
+ if (Working) {
+ using namespace llvm::support;
+ uint64_t Swapped = endian::byte_swap<uint64_t, little>(Working);
+ MD5.update(llvm::makeArrayRef((uint8_t *)&Swapped, sizeof(Swapped)));
+ }
// Finalize the MD5 and return the hash.
llvm::MD5::MD5Result Result;
Index: clang/test/Profile/c-collision.c
===================================================================
--- /dev/null
+++ clang/test/Profile/c-collision.c
@@ -0,0 +1,26 @@
+// Test that a slight change in the code leads to a different hash.
+// RUN: %clang_cc1 -UEXTRA -triple x86_64-unknown-linux-gnu -main-file-name c-collision.c %s -o - -emit-llvm -fprofile-instrument=llvm | FileCheck %s --check-prefix=CHECK-NOEXTRA
+// RUN: %clang_cc1 -DEXTRA -triple x86_64-unknown-linux-gnu -main-file-name c-collision.c %s -o - -emit-llvm -fprofile-instrument=llvm | FileCheck %s --check-prefix=CHECK-EXTRA
+
+// CHECK-NOEXTRA: @__profd_main = private global { {{.*}} } { i64 -2624081020897602054, i64 88870893692,
+// CHECK-EXTRA: @__profd_main = private global { {{.*}} } { i64 -2624081020897602054, i64 101970072141,
+
+int main(int n, char **v) {
+ if (n)
+ n += 1;
+ if (n)
+ n += 1;
+ if (n)
+ n += 1;
+ if (n)
+ n += 1;
+ if (n)
+ n += 1;
+ if (n)
+ n += 1;
+#ifdef EXTRA
+ if (n)
+ n += 1;
+#endif
+ return n;
+}
Index: clang/lib/CodeGen/CodeGenPGO.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenPGO.cpp
+++ clang/lib/CodeGen/CodeGenPGO.cpp
@@ -747,8 +747,11 @@
return Working;
// Check for remaining work in Working.
- if (Working)
- MD5.update(Working);
+ if (Working) {
+ using namespace llvm::support;
+ uint64_t Swapped = endian::byte_swap<uint64_t, little>(Working);
+ MD5.update(llvm::makeArrayRef((uint8_t *)&Swapped, sizeof(Swapped)));
+ }
// Finalize the MD5 and return the hash.
llvm::MD5::MD5Result Result;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
