f*::min/max: fix comparing with libm and IEEE operations #149563
Merged
+88
−80
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
Collaborator
rustbot
added
S-waiting-on-review
Collaborator
|
|
tgross35
reviewed
Dec 5, 2025
library/core/src/intrinsics/mod.rs
Outdated
Comment on lines
2950
to
2953
| /// Returns the minimum of two `f16` values, ignoring NaN. | ||
| /// | ||
| /// This behaves like IEEE 754-2008 minNum. In particular: | ||
| /// If one of the arguments is NaN, then the other argument is returned. If the inputs compare equal | ||
| /// If one of the arguments is NaN (quiet or signaling), then the other argument is returned. If the inputs compare equal | ||
| /// (such as for the case of `+0.0` and `-0.0`), either input may be returned non-deterministically. |
Contributor
There was a problem hiding this comment.
Is it worth also mentioning the (NaN,NaN) case? Similar to #149477
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Dec 5, 2025
…r=mati865 interpret: test SNaN handling of float min/max and update comments Also see rust-lang#149563. I also renamed these enum variants so they are not almost identical.
tgross35
approved these changes
Dec 5, 2025
Contributor
|
@bors r+ rollup |
Collaborator
bors
added
S-waiting-on-bors
github-actions bot
pushed a commit
to rust-lang/miri
that referenced
this pull request
Dec 6, 2025
interpret: test SNaN handling of float min/max and update comments Also see rust-lang/rust#149563. I also renamed these enum variants so they are not almost identical.
bors
added a commit
that referenced
this pull request
Dec 6, 2025
Rollup of 4 pull requests Successful merges: - #149563 (f*::min/max: fix comparing with libm and IEEE operations) - #149592 (`is_const_default_method` is completely handled by the `constness` query) - #149662 (Move attribute lints to `rustc_lint`) - #149684 (rustc-dev-guide subtree update) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
that referenced
this pull request
Dec 6, 2025
Rollup merge of #149563 - RalfJung:f-min-max, r=tgross35 f*::min/max: fix comparing with libm and IEEE operations What we document actually doesn't match what libm does any more, libm got "fixed"/changed in https://sourceware.org/bugzilla/show_bug.cgi?id=20947. So better remove the remark. Instead, explicitly call out that this is a mix of `minNum` and `minimumNumber`. Also fix the intrinsics which incorrectly claimed to be like `minNum`, but their intended SNaN behavior is actually different from that. r? `@tgross35`
github-actions bot
pushed a commit
to rust-lang/rustc-dev-guide
that referenced
this pull request
Dec 13, 2025
Rollup of 4 pull requests Successful merges: - rust-lang/rust#149563 (f*::min/max: fix comparing with libm and IEEE operations) - rust-lang/rust#149592 (`is_const_default_method` is completely handled by the `constness` query) - rust-lang/rust#149662 (Move attribute lints to `rustc_lint`) - rust-lang/rust#149684 (rustc-dev-guide subtree update) r? `@ghost` `@rustbot` modify labels: rollup
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
What we document actually doesn't match what libm does any more, libm got "fixed"/changed in https://sourceware.org/bugzilla/show_bug.cgi?id=20947. So better remove the remark. Instead, explicitly call out that this is a mix of
minNumandminimumNumber.Also fix the intrinsics which incorrectly claimed to be like
minNum, but their intended SNaN behavior is actually different from that.r? @tgross35