Repository navigation
Assertion `0 && "too long jmp distance"' failed with new JIT on AArch64 #12560
Description
Description
Hi @dstogov, we recently monitored below assertion failure after the integration of the new PHP JIT (#12079). The issue is reproducible by running some tests in the phpunit repo on AArch64.
- Checkout and build
$ git clone https://github.com/sebastianbergmann/phpunit.git
$ cd phpunit
$ composer install
- Run a test
$ php -d zend_extension=/home/work/php-src/modules/opcache.so -d opcache.jit_buffer_size=128M \
-d opcache.enable_cli=1 ./phpunit tests/unit/Framework/AssertTest.php
PHPUnit 11.0-gddc736d9b by Sebastian Bergmann and contributors.
Runtime: PHP 8.4.0-dev
Configuration: /tmp/phpunit/phpunit.xml
............................................................... 63 / 598 ( 10%)
............................................................... 126 / 598 ( 21%)
............................................................... 189 / 598 ( 31%)
............................................................... 252 / 598 ( 42%)
............................................................... 315 / 598 ( 52%)
............................................................... 378 / 598 ( 63%)
............................................................... 441 / 598 ( 73%)
............................................................... 504 / 598 ( 84%)
php: /home/work/php-src/ext/opcache/jit/ir/ir_aarch64.dasc:5609: ir_add_veneer: Assertion `0 && "too long jmp distance"' failed.
Aborted (core dumped)
PHP Version
master @ 4c6dbe0
Operating System
Ubuntu 22.04
Activity
dstogov commentedon Oct 30, 2023
Thanks for the report. The problem should be fixed in PHP master branch.
In case you still see the problem, please reopen the issue.
pfustc commentedon Nov 1, 2023
Thanks for your quick fix.
pfustc commentedon Nov 2, 2023
Hi @dstogov, I just found functions
aarch64_may_use_adr()andaarch64_may_use_adrp()declared inir_aarch64.dascare not used anywhere in the new JIT. Would you like to use ADR/ADRP in the future, or just remove the two functions? They cause build warnings on AArch64.BTW: I created another two issues #12596 and #12597 today. They also seem to be related to the new JIT. Could you help take a look?
dstogov commentedon Nov 2, 2023
I know. I don't plan to actively work especially on AArch64 back-end, but in long run I plan to replace DynAsm with something more useful and this will of course affect AArch64 back-end as well. For now I'm going keep these functions.
dstogov commentedon Nov 2, 2023
Thanks. I'll take a look.
In the future you may assign new JIT related issues to me, or ping me (like now) if you don't have rights, otherwise I may miss them.
pfustc commentedon Nov 2, 2023
Thanks for all your quick fixes. Recently we monitored several PHP test failures after the integration of the new JIT. I'm currently running more tests on AArch64 with various of JIT options. So I will probably ping you in more GH issues in the coming weeks.
I'm also looking into your implementation of the new JIT. From what I can see and understand so far, the AArch64 back-end of the new JIT has been well implemented. We appreciate your great work! I'm also looking for what I can do to make the AArch64 back-end better. Please let me know if there's anything we can help with the AArch64 codegen part.
dstogov commentedon Nov 2, 2023
I'm not satisfied with current IR backends. Their implementations are too complex. Usage of DynAsm on one hand made the implementation more readable, but on the other hand leaded to code duplication and unnecessary slowdown.
After fixing most new bugs and some stabilization, I plan to think how to make the back-ends better.
May be switch to some BURS/BURG tool or at least rewrite instruction matcher in a declarative way,
Anyway, it's going to be next year. Ideas are welcome.
Fixed phpGH-12560: Assertion `0 && "too long jmp distance"' failed wi…