Skip to content

Commit afb1c57

Browse files
committed
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. Closes GH-18814.
1 parent 0cd3ebf commit afb1c57

File tree

3 files changed

+6
-3
lines changed

3 files changed

+6
-3
lines changed

NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ PHP NEWS
1818
- FPM:
1919
. Fixed GH-18662 (fpm_get_status segfault). (txuna)
2020

21+
- Hash:
22+
. Fixed bug GH-14551 (PGO build fails with xxhash). (nielsdos)
23+
2124
- Intl:
2225
. Fix memory leak in intl_datetime_decompose() on failure. (nielsdos)
2326
. Fix memory leak in locale lookup on failure. (nielsdos)

ext/hash/hash_xxhash.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ const php_hash_ops php_hash_xxh3_64_ops = {
154154
typedef XXH_errorcode (*xxh3_reset_with_secret_func_t)(XXH3_state_t*, const void*, size_t);
155155
typedef XXH_errorcode (*xxh3_reset_with_seed_func_t)(XXH3_state_t*, XXH64_hash_t);
156156

157-
zend_always_inline static void _PHP_XXH3_Init(PHP_XXH3_64_CTX *ctx, HashTable *args,
157+
static void _PHP_XXH3_Init(PHP_XXH3_64_CTX *ctx, HashTable *args,
158158
xxh3_reset_with_seed_func_t func_init_seed, xxh3_reset_with_secret_func_t func_init_secret, const char* algo_name)
159159
{
160160
memset(&ctx->s, 0, sizeof ctx->s);

ext/hash/xxhash/xxhash.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -931,7 +931,7 @@ XXH_PUBLIC_API int XXH128_cmp(const void* h128_1, const void* h128_2);
931931

932932
/******* Canonical representation *******/
933933
typedef struct { unsigned char digest[sizeof(XXH128_hash_t)]; } XXH128_canonical_t;
934-
XXH_PUBLIC_API void XXH128_canonicalFromHash(XXH128_canonical_t* dst, XXH128_hash_t hash);
934+
static zend_always_inline void XXH128_canonicalFromHash(XXH128_canonical_t* dst, XXH128_hash_t hash);
935935
XXH_PUBLIC_API XXH128_hash_t XXH128_hashFromCanonical(const XXH128_canonical_t* src);
936936

937937

@@ -5503,7 +5503,7 @@ XXH_PUBLIC_API int XXH128_cmp(const void* h128_1, const void* h128_2)
55035503

55045504
/*====== Canonical representation ======*/
55055505
/*! @ingroup xxh3_family */
5506-
XXH_PUBLIC_API void
5506+
static zend_always_inline void
55075507
XXH128_canonicalFromHash(XXH128_canonical_t* dst, XXH128_hash_t hash)
55085508
{
55095509
XXH_STATIC_ASSERT(sizeof(XXH128_canonical_t) == sizeof(XXH128_hash_t));

0 commit comments

Comments
 (0)