Skip to content

Assertion `0 && "too long jmp distance"' failed with new JIT on AArch64 #12560

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
pfustc opened this issue Oct 30, 2023 · 7 comments
Closed

Assertion `0 && "too long jmp distance"' failed with new JIT on AArch64 #12560

pfustc opened this issue Oct 30, 2023 · 7 comments

Comments

@pfustc
Copy link
Contributor

pfustc commented Oct 30, 2023

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

@dstogov
Copy link
Member

dstogov commented 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
Copy link
Contributor Author

pfustc commented Nov 1, 2023

Thanks for your quick fix.

@pfustc
Copy link
Contributor Author

pfustc commented Nov 2, 2023

Hi @dstogov, I just found functions aarch64_may_use_adr() and aarch64_may_use_adrp() declared in ir_aarch64.dasc are 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
Copy link
Member

dstogov commented Nov 2, 2023

Hi @dstogov, I just found functions aarch64_may_use_adr() and aarch64_may_use_adrp() declared in ir_aarch64.dasc are 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.

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
Copy link
Member

dstogov commented Nov 2, 2023

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?

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
Copy link
Contributor Author

pfustc commented 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.

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
Copy link
Member

dstogov commented 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.

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

No branches or pull requests

3 participants