Skip to content

Fix function JIT JMPNZ smart branch#21596

Closed
iluuu1994 wants to merge 1 commit intophp:PHP-8.4from
iluuu1994:gh-21593
Closed

Fix function JIT JMPNZ smart branch#21596
iluuu1994 wants to merge 1 commit intophp:PHP-8.4from
iluuu1994:gh-21593

Conversation

@iluuu1994
Copy link
Copy Markdown
Member

When a smart branch and jump live in separate basic blocks, the JIT can't skip the jitting of the jump, as it may be reachable through another predecessor. When the smart branch is executed using zend_jit_handler(), we're manually writing the result of the branch to the given temporary so that the jump will work as expected. That happens in zend_jit_set_cond().

However, this was only correctly handled for JMPZ branches. The current opline was compared to opline following the jump, which would set the var to 1 if equal, i.e. the branch was not taken, meaning var was not zero. For JMPNZ we need to do the opposite.

Adding the "CI: All variations" label to run the function JIT.

Fixes GH-21593

When a smart branch and jump live in separate basic blocks, the JIT can't skip
the jitting of the jump, as it may be reachable through another predecessor.
When the smart branch is executed using zend_jit_handler(), we're manually
writing the result of the branch to the given temporary so that the jump will
work as expected. That happens in zend_jit_set_cond().

However, this was only correctly handled for JMPZ branches. The current opline
was compared to opline following the jump, which would set the var to 1 if
equal, i.e. the branch was not taken, meaning var was not zero. For JMPNZ we
need to do the opposite.

Fixes phpGH-21593
Copy link
Copy Markdown
Member

@dstogov dstogov left a comment

Choose a reason for hiding this comment

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

This seems right.
I wonder this wasn't caught early.
The whole PHPT test suite doesn't cover this case.
Thanks!

@iluuu1994
Copy link
Copy Markdown
Member Author

Merged as 455ae28.

@iluuu1994 iluuu1994 closed this Apr 2, 2026
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.

2 participants