Disable jump threading of float equality #128271
Merged
Conversation
This file contains 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
Jump threading stores values as `u128` (`ScalarInt`) and does its comparisons for equality as integer comparisons. This works great for integers. Sadly, not everything is an integer. Floats famously have wonky equality semantcs, with `NaN!=NaN` and `0.0 == -0.0`. This does not match our beautiful integer bitpattern equality and therefore causes things to go horribly wrong. While jump threading could be extended to support floats by remembering that they're floats in the value state and handling them properly, it's signficantly easier to just disable it for now.
rustbot
added
the
S-waiting-on-review
|
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
rustbot
added
the
T-compiler
saethlin
added
beta-nominated
|
@bors r+ |
bors
added
S-waiting-on-bors
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jul 27, 2024
Rollup of 8 pull requests Successful merges: - rust-lang#125897 (from_ref, from_mut: clarify documentation) - rust-lang#128207 (improve error message when `global_asm!` uses `asm!` options) - rust-lang#128241 (Remove logic to suggest clone of function output) - rust-lang#128259 ([illumos/solaris] set MSG_NOSIGNAL while writing to sockets) - rust-lang#128262 (Delete `SimplifyArmIdentity` and `SimplifyBranchSame` tests) - rust-lang#128266 (update `rust.channel` default value documentation) - rust-lang#128267 (Add rustdoc GUI test to check title with and without search) - rust-lang#128271 (Disable jump threading of float equality) r? `@ghost` `@rustbot` modify labels: rollup
Noratrieb
commented
Jul 27, 2024
| // and therefore we cannot use integer comparisons for them. | ||
| // Avoid handling them, though this could be extended in the future. | ||
| return None; | ||
| } | ||
| let value = value.const_.normalize(self.tcx, self.param_env).try_to_scalar_int()?; |
There was a problem hiding this comment.
While this PR is an easily backportable targeted fix, it might be worth thinking about how to prevent this more generally. It seems highly confusing that a method called try_to_scalar_int will return floats, and it doesn't surprise me that this pass was confused by this. I would also expect try_to_scalar_int to be OK for comparing bitwise.
Maybe it should be changed to not return anything for floats?
There was a problem hiding this comment.
Possibly bad suggestion, but maybe we shouldn't use valtrees to represent floats 🤔
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jul 27, 2024
Rollup merge of rust-lang#128271 - Nilstrieb:jump-into-a-can-of-worms-called-float-equality, r=compiler-errors Disable jump threading of float equality Jump threading stores values as `u128` (`ScalarInt`) and does its comparisons for equality as integer comparisons. This works great for integers. Sadly, not everything is an integer. Floats famously have wonky equality semantcs, with `NaN!=NaN` and `0.0 == -0.0`. This does not match our beautiful integer bitpattern equality and therefore causes things to go horribly wrong. While jump threading could be extended to support floats by remembering that they're floats in the value state and handling them properly, it's signficantly easier to just disable it for now. fixes rust-lang#128243
|
Beta backport approved as per compiler team on Zulip. A backport PR will be authored by the release team at the end of the current development cycle. Given the regression that it fixes and the fact that we have a report from a user, T-compiler is inclined that this backport is worth publishing a dot release alone. @rustbot label +beta-accepted +stable-accepted |
rustbot
added
beta-accepted
pietroalbini
removed
the
stable-nominated
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 6, 2024
Prepare Rust 1.80.1 point release The point release is scheduled to include: * rust-lang#128271 * rust-lang#128618
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 6, 2024
Prepare Rust 1.80.1 point release The point release is scheduled to include: * rust-lang#128271 * rust-lang#128618
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 6, 2024
Prepare Rust 1.80.1 point release The point release is scheduled to include: * rust-lang#128271 * rust-lang#128618
bors-ferrocene bot
added a commit
to ferrocene/ferrocene
that referenced
this pull request
Aug 7, 2024
826: Disable jump threading of float equality r=Hoverbear a=Veykril Jump threading stores values as `u128` (`ScalarInt`) and does its comparisons for equality as integer comparisons. This works great for integers. Sadly, not everything is an integer. Floats famously have wonky equality semantcs, with `NaN!=NaN` and `0.0 == -0.0`. This does not match our beautiful integer bitpattern equality and therefore causes things to go horribly wrong. While jump threading could be extended to support floats by remembering that they're floats in the value state and handling them properly, it's signficantly easier to just disable it for now. Ferrocene-backport-of: rust-lang/rust#128271 Co-authored-by: Nilstrieb <[email protected]> Co-authored-by: Pietro Albini <[email protected]>
cuviper
removed
the
beta-nominated
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 15, 2024
[beta] backports and bump stage0 - Disable jump threading of float equality rust-lang#128271 - Normalize when equating `dyn` tails in MIR borrowck rust-lang#128694 - Improve `Ord` violation help rust-lang#128273 - bump stage0 to stable 1.80.1 - Revert rust-lang#125915 on beta rust-lang#128760 - derive(SmartPointer): register helper attributes rust-lang#128925 - Fix bug in `Parser::look_ahead`. rust-lang#128994 r? cuviper
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Jump threading stores values as
u128(ScalarInt) and does its comparisons for equality as integer comparisons.This works great for integers. Sadly, not everything is an integer.
Floats famously have wonky equality semantcs, with
NaN!=NaNand0.0 == -0.0. This does not match our beautiful integer bitpattern equality and therefore causes things to go horribly wrong.While jump threading could be extended to support floats by remembering that they're floats in the value state and handling them properly, it's signficantly easier to just disable it for now.
fixes (but doesnt close until beta/stable nominations are settled) #128243