Skip to content
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

float::min/max: mention the non-determinism around signed 0 #136296

Merged
merged 1 commit into from
Jan 31, 2025

Conversation

RalfJung
Copy link
Member

Turns out this can actually produce different results on different machines in practice; that seems worth documenting. I assume LLVM will happily const-fold these operations so so there could be different results for the same input even on the same machine, depending on whether things get const-folded or not.

@nikic I remember there was an LLVM soundness fix regarding scalar evolution for loops that had to recognize certain operations as non-deterministic... it seems to me that pass would also have to avoid predicting the result of llvm.{min,max}num, for the same reason?

r? @tgross35
Cc @rust-lang/libs-api

If this lands we should also make Miri non-deterministic here.

Fixes #83984

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jan 30, 2025
@tgross35
Copy link
Contributor

tgross35 commented Jan 30, 2025

This documents existing behavior without making any new guarantees, and seems to be about the best we can do here based on discussion at #83984.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jan 30, 2025

📌 Commit 6b699cc has been approved by tgross35

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 30, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 31, 2025
Rollup of 8 pull requests

Successful merges:

 - rust-lang#135414 (Stabilize `const_black_box`)
 - rust-lang#136150 (ci: use windows 2025 for i686-mingw)
 - rust-lang#136258 (rustdoc: rename `issue-\d+.rs` tests to have meaningful names (part 11))
 - rust-lang#136270 (Remove `NamedVarMap`.)
 - rust-lang#136278 (add constraint graph to polonius MIR dump)
 - rust-lang#136287 (LLVM changed the nocapture attribute to captures(none))
 - rust-lang#136291 (some test suite cleanups)
 - rust-lang#136296 (float::min/max: mention the non-determinism around signed 0)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 08dc8c9 into rust-lang:master Jan 31, 2025
6 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 31, 2025
Rollup merge of rust-lang#136296 - RalfJung:float-min-max, r=tgross35

float::min/max: mention the non-determinism around signed 0

Turns out this can actually produce different results on different machines [in practice](rust-lang#83984 (comment)); that seems worth documenting. I assume LLVM will happily const-fold these operations so so there could be different results for the same input even on the same machine, depending on whether things get const-folded or not.

`@nikic` I remember there was an LLVM soundness fix regarding scalar evolution for loops that had to recognize certain operations as non-deterministic... it seems to me that pass would also have to avoid predicting the result of `llvm.{min,max}num`, for the same reason?

r? `@tgross35`
Cc `@rust-lang/libs-api`

If this lands we should also make Miri non-deterministic here.

Fixes rust-lang#83984
@rustbot rustbot added this to the 1.86.0 milestone Jan 31, 2025
@RalfJung RalfJung deleted the float-min-max branch January 31, 2025 10:03
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Jan 31, 2025
…-obk

miri: make float min/max non-deterministic

This makes Miri match the documentation that landed in rust-lang#136296.

r? `@oli-obk`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 1, 2025
…-obk

miri: make float min/max non-deterministic

This makes Miri match the documentation that landed in rust-lang#136296.

r? ``@oli-obk``
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 1, 2025
Rollup merge of rust-lang#136348 - RalfJung:miri-float-min-max, r=oli-obk

miri: make float min/max non-deterministic

This makes Miri match the documentation that landed in rust-lang#136296.

r? ``@oli-obk``
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Feb 2, 2025
miri: make float min/max non-deterministic

This makes Miri match the documentation that landed in rust-lang/rust#136296.

r? ``@oli-obk``
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Float min/max/clamp follow IEEE754-2008 (and not IEEE754-2019) WRT negative zeros
4 participants