perf: fix LTO build with gcc (hybrid VM) - #23602
Conversation
| [x86_64*|amd64*], [AS_VAR_APPEND([LDFLAGS], [" -ffixed-r14 -ffixed-r15"])], | ||
| [x86*|amd*|i?86*|pentium], [AS_VAR_APPEND([LDFLAGS], [" -ffixed-esi -ffixed-edi"])], | ||
| [aarch64*|arm64*], [AS_VAR_APPEND([LDFLAGS], [" -ffixed-x27 -ffixed-x28"])], | ||
| [ppc64*|powerpc64*], [AS_VAR_APPEND([LDFLAGS], [" -ffixed-r14 -ffixed-r15"])], |
There was a problem hiding this comment.
I m certain powerpc is correct, I was digging a bit about riscv64 and it looks correct too according to this doc, those registers are equivalent to s2/s3 confirmed with the gcc header (see REGISTER_NAMES/ADDITIONAL_REGISTER_NAMES).
There was a problem hiding this comment.
I certainly hope they're correct, because otherwise php would crash, I copied them from
Line 52 in 810a9e5
|
What I m less sure about is if the target is appropriate, it s border line improvement/bug fix, I have no issue targetting stable branch personally but I ll leave it to @arnaud-lb |
|
It's a bugfix in the sense that lto compilation previously failed with the hybrid vm. Nothing changes about existing builds, so it's safe to merge into supported branches. |
|
Nice! I've tried to compare gcc-lto vs clang-lto: The Clang build is about 1.5% faster on the Symfony benchmark, on PHPStan it's the reverse: The GCC build is about 1.5% faster. This was on amd64, possibly aarch64 will be different. Given that this requires to patch ltmain.sh, and that it's more a GCC bug than a php-src bug, I would prefer if this targeted master only. I believe that the version of ltmain.sh on master doesn't need any change. |
| # -ffixed-*, -fno-lto GCC LTO register reservations and cancellation | ||
| -ffixed-*|-fno-lto| \ |
There was a problem hiding this comment.
We should avoid modifying ltmain.sh directly as it's a generated/3rd party file. The version on master is much more recent, so it's possible that these changes are not necessary on that branch.
There was a problem hiding this comment.
The changes are required there too, -ffixed is missing. (-fno-lto isn't required, I've only added it because -flto is in the line above, so passing "-flto -fno-lto" would create unexpected outcomes)
There was a problem hiding this comment.
@petk @NattyNarwhal do you have thoughts about the ltmain.sh changes?
There was a problem hiding this comment.
Next time libtool gets updated, it'll get clobbered. If this is a bug in libtool, we could carry it locally (and ship patches that could be applied), but you should probably seek to upstream it there.
Yes, it's similar for me, GCC tailcall is fastest in phpstan, phoronix and zend/bench, followed by clang tailcall and last gcc hybrid. My symfony demo tests were +/- 0 between the tailcall vm's, but gcc tailcall executed around 1% less instructions. Strict aliasing optimizations and different inlining behaviour explained most of it. The more interesting architecture to test here would be aarch64, because clang tailcall was slower than gcc hybrid there. But I don't have a VM ready to test it atm.
I've re-opened the other PR at #23606. The ltmain change is unfortunately still required for embed/apache. |

#23588 but on php 8.4/8.5