Skip to content

Fix GH-14551: PGO build fails with xxhash #18814

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos commented Jun 9, 2025

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.

As a side note, since we already have some custom patches to xxhash.h, it would be beneficial to:

  1. Sync upstream
  2. Use a patchfile for PHP-specific additions

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.
@nielsdos nielsdos linked an issue Jun 9, 2025 that may be closed by this pull request
@nielsdos nielsdos requested a review from Girgias June 10, 2025 16:11
Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MSTM, but yes would be nice to have an easy way to make sure we don't forget something if we sync up with upstream

@nielsdos
Copy link
Member Author

So we don't forget: #18842

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PGO build fails with xxhash
2 participants