From c53c078f16e33d4b2e39e39afe8320f45a20b38a Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 9 Jun 2025 17:52:21 +0200 Subject: [PATCH 1/2] Fix GH-14551: PGO build fails with xxhash Turns out that the instrumentation added for gcov can change inlining decisions of the compiler, which results in a mismatch between the profile data CFG and the actual generated CFG between compiles. There are two functions that suffer from this issue: 1. _PHP_XXH3_Init: Removing the inline hint fixes this one. In fact, always inlining this makes no sense as there's no real opportunity for specialising. It just bloats the binary and increases I$ pressure. So besides fixing this issue it's beneficial on its own to drop the attribute. 2. PHP_XXH3_128_Final: Sometimes XXH128_canonicalFromHash gets inlined and sometimes not. Make sure it gets always inlined. --- ext/hash/hash_xxhash.c | 2 +- ext/hash/xxhash/xxhash.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/hash/hash_xxhash.c b/ext/hash/hash_xxhash.c index 24da754d8835a..070bd06bff070 100644 --- a/ext/hash/hash_xxhash.c +++ b/ext/hash/hash_xxhash.c @@ -154,7 +154,7 @@ const php_hash_ops php_hash_xxh3_64_ops = { typedef XXH_errorcode (*xxh3_reset_with_secret_func_t)(XXH3_state_t*, const void*, size_t); typedef XXH_errorcode (*xxh3_reset_with_seed_func_t)(XXH3_state_t*, XXH64_hash_t); -zend_always_inline static void _PHP_XXH3_Init(PHP_XXH3_64_CTX *ctx, HashTable *args, +static void _PHP_XXH3_Init(PHP_XXH3_64_CTX *ctx, HashTable *args, xxh3_reset_with_seed_func_t func_init_seed, xxh3_reset_with_secret_func_t func_init_secret, const char* algo_name) { memset(&ctx->s, 0, sizeof ctx->s); diff --git a/ext/hash/xxhash/xxhash.h b/ext/hash/xxhash/xxhash.h index 8e816c0584ebd..b926b8f0af4f3 100644 --- a/ext/hash/xxhash/xxhash.h +++ b/ext/hash/xxhash/xxhash.h @@ -931,7 +931,7 @@ XXH_PUBLIC_API int XXH128_cmp(const void* h128_1, const void* h128_2); /******* Canonical representation *******/ typedef struct { unsigned char digest[sizeof(XXH128_hash_t)]; } XXH128_canonical_t; -XXH_PUBLIC_API void XXH128_canonicalFromHash(XXH128_canonical_t* dst, XXH128_hash_t hash); +XXH_PUBLIC_API zend_always_inline void XXH128_canonicalFromHash(XXH128_canonical_t* dst, XXH128_hash_t hash); XXH_PUBLIC_API XXH128_hash_t XXH128_hashFromCanonical(const XXH128_canonical_t* src); @@ -5503,7 +5503,7 @@ XXH_PUBLIC_API int XXH128_cmp(const void* h128_1, const void* h128_2) /*====== Canonical representation ======*/ /*! @ingroup xxh3_family */ -XXH_PUBLIC_API void +XXH_PUBLIC_API zend_always_inline void XXH128_canonicalFromHash(XXH128_canonical_t* dst, XXH128_hash_t hash) { XXH_STATIC_ASSERT(sizeof(XXH128_canonical_t) == sizeof(XXH128_hash_t)); From 28c5864d5a0ddf8d25055f554d9557fdf743943c Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 9 Jun 2025 18:04:59 +0200 Subject: [PATCH 2/2] fix warn --- ext/hash/xxhash/xxhash.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/hash/xxhash/xxhash.h b/ext/hash/xxhash/xxhash.h index b926b8f0af4f3..5874c9a1f97b5 100644 --- a/ext/hash/xxhash/xxhash.h +++ b/ext/hash/xxhash/xxhash.h @@ -931,7 +931,7 @@ XXH_PUBLIC_API int XXH128_cmp(const void* h128_1, const void* h128_2); /******* Canonical representation *******/ typedef struct { unsigned char digest[sizeof(XXH128_hash_t)]; } XXH128_canonical_t; -XXH_PUBLIC_API zend_always_inline void XXH128_canonicalFromHash(XXH128_canonical_t* dst, XXH128_hash_t hash); +static zend_always_inline void XXH128_canonicalFromHash(XXH128_canonical_t* dst, XXH128_hash_t hash); XXH_PUBLIC_API XXH128_hash_t XXH128_hashFromCanonical(const XXH128_canonical_t* src); @@ -5503,7 +5503,7 @@ XXH_PUBLIC_API int XXH128_cmp(const void* h128_1, const void* h128_2) /*====== Canonical representation ======*/ /*! @ingroup xxh3_family */ -XXH_PUBLIC_API zend_always_inline void +static zend_always_inline void XXH128_canonicalFromHash(XXH128_canonical_t* dst, XXH128_hash_t hash) { XXH_STATIC_ASSERT(sizeof(XXH128_canonical_t) == sizeof(XXH128_hash_t));