Add round_ties_even to f32 and f64
#95317
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
rustbot
added
the
T-compiler
|
Some changes occured to rustc_codegen_gcc cc @antoyo |
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @yaahc (or someone else) soon. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
yaahc
added
the
T-libs-api
Jules-Bertholet
force-pushed
the
round_ties_to_even
branch
from
d769b92
to
ad51ecd
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Jules-Bertholet
force-pushed
the
round_ties_to_even
branch
from
ad51ecd
to
3a23dfc
Compare
This comment has been minimized.
This comment has been minimized.
Jules-Bertholet
force-pushed
the
round_ties_to_even
branch
from
3a23dfc
to
3ed85dd
Compare
|
Assuming that the symbol names are correct for the cranelift and gcc codegen backends, the compiler changes here look simple and correct. so I'd say r=me for the compiler side. I'll let @yaahc speak for the libs side. |
|
Don't think this should block merging this PR, but just to add my opinion to the mix, I've basically always seen this called |
9 tasks
|
I'm reassigning this PR because I'm taking a break from the review rotation for a little while. Thank you for your patience. r? rust-lang/libs-api |
|
Given that we have Other inspiration from wikipedia:
|
We can leave the bikeshed for later, after adding this as unstable but before stabilization. Can you open a tracking issue for this feature? Please include the open question about the name of the function, and the open question about whether this should be the default behaviour of |
3 tasks
|
Tracking issue opened. |
This comment was marked as outdated.
This comment was marked as outdated.
Jules-Bertholet
force-pushed
the
round_ties_to_even
branch
from
3ed85dd
to
d2c6c6e
Compare
JohnCSimon
added
S-waiting-on-review
|
Hello, is the status (waiting for review) of this PR correct? Any pending comments from |
scottmcm
added
S-waiting-on-author
|
Looks like this has a r=me for the compiler changes, and a "seems reasonable" from libs-api, so I think this is just waiting on a tracking issue (#95317 (comment)) to be mergeable, with bikesheds to happen on that tracking issue. So I've marked it S-author. |
bors
added
S-waiting-on-review
|
The job Click to see the possible cause of the failure (guessed by this bot) |
Looks spurious. |
bors
added
S-waiting-on-bors
|
⌛ Testing commit 371d570 with merge 9f10e462f6256a1db9f38ef3bc46f917dd753c95... |
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
|
This failure doesn't look related either |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (0a3b557): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
Why do the tests here use |
|
I just cargo-culted the |
bjorn3
pushed a commit
to bjorn3/rust
that referenced
this pull request
Mar 15, 2023
…=pnkfelix,m-ou-se,scottmcm Add `round_ties_even` to `f32` and `f64` Tracking issue: rust-lang#96710 Redux of rust-lang#82273. See also rust-lang#55107 Adds a new method, `round_ties_even`, to `f32` and `f64`, that rounds the float to the nearest integer , rounding halfway cases to the number with an even least significant bit. Uses the `roundeven` LLVM intrinsic to do this. Of the five IEEE 754 rounding modes, this is the only one that doesn't already have a round-to-integer function exposed by Rust (others are `round`, `floor`, `ceil`, and `trunc`). Ties-to-even is also the rounding mode used for int-to-float and float-to-float `as` casts, as well as float arithmentic operations. So not having an explicit rounding method for it seems like an oversight. Bikeshed: this PR currently uses `round_ties_even` for the name of the method. But maybe `round_ties_to_even` is better, or `round_even`, or `round_to_even`?
antoyo
pushed a commit
to antoyo/rust
that referenced
this pull request
Jun 19, 2023
…=pnkfelix,m-ou-se,scottmcm Add `round_ties_even` to `f32` and `f64` Tracking issue: rust-lang#96710 Redux of rust-lang#82273. See also rust-lang#55107 Adds a new method, `round_ties_even`, to `f32` and `f64`, that rounds the float to the nearest integer , rounding halfway cases to the number with an even least significant bit. Uses the `roundeven` LLVM intrinsic to do this. Of the five IEEE 754 rounding modes, this is the only one that doesn't already have a round-to-integer function exposed by Rust (others are `round`, `floor`, `ceil`, and `trunc`). Ties-to-even is also the rounding mode used for int-to-float and float-to-float `as` casts, as well as float arithmentic operations. So not having an explicit rounding method for it seems like an oversight. Bikeshed: this PR currently uses `round_ties_even` for the name of the method. But maybe `round_ties_to_even` is better, or `round_even`, or `round_to_even`?
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.
Tracking issue: #96710
Redux of #82273. See also #55107
Adds a new method,
round_ties_even, tof32andf64, that rounds the float to the nearest integer , rounding halfway cases to the number with an even least significant bit. Uses theroundevenLLVM intrinsic to do this.Of the five IEEE 754 rounding modes, this is the only one that doesn't already have a round-to-integer function exposed by Rust (others are
round,floor,ceil, andtrunc). Ties-to-even is also the rounding mode used for int-to-float and float-to-floatascasts, as well as float arithmentic operations. So not having an explicit rounding method for it seems like an oversight.Bikeshed: this PR currently uses
round_ties_evenfor the name of the method. But mayberound_ties_to_evenis better, orround_even, orround_to_even?