Fix floating point guard jcc x86-64 codegen #122
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Guard codegen relies on inverting the condition in `ir_emit_guard_cmp_fp`. However, for floating point this results in different behaviour w.r.t. NAN. For example, if we have `GUARD(_, LT(_, _), _)` then we should only continue executing code if `LT(_, _)` is satisfied. Inverting the condition and branch targets makes this `GE(_, _)`, but NANs don't trigger the `GE(_, _)` condition, resulting in continuing executing the code if a NAN was present. To solve this: track whether the condition was inverted and conditionally generate jumps based on the parity flag. Because of this change, the "swap operands to avoid P flag check" optimization also is affected, so fix this as well.
Contributor
Author
|
MacOS fails because of different test output: the decompiler outputs an address instead of the "exit" name. I'm not sure how to fix this. Code itself seems right. |
|
@ndossche ndossche#1 fixes it following the pattern from argval_002.irt |
Owner
|
Thanks for looking into the problem. |
Contributor
Author
|
Okay I see. Thanks. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Guard codegen relies on inverting the condition in
ir_emit_guard_cmp_fp. However, for floating point this results indifferent behaviour w.r.t. NAN.
For example, if we have
GUARD(_, LT(_, _), _)then we should onlycontinue executing code if
LT(_, _)is satisfied.Inverting the condition and branch targets makes this
GE(_, _), butNANs don't trigger the
GE(_, _)condition, resulting in continuingexecuting the code if a NAN was present.
To solve this: track whether the condition was inverted and
conditionally generate jumps based on the parity flag.
Because of this change, the "swap operands to avoid P flag check"
optimization also is affected, so fix this as well.
Related to php/php-src#20880