Add round_to_even to floating point types #82273
Conversation
|
Some changes occured to rustc_codegen_cranelift cc @bjorn3 |
rust-highfive
added
the
S-waiting-on-review
m-ou-se
added
A-floating-point
This comment has been minimized.
This comment has been minimized.
| roundevenf32(flt) -> f32 => roundevenf, | ||
| roundevenf64(flt) -> f64 => roundeven, |
There was a problem hiding this comment.
If I understand correctly, this will cause calls to the roundeven and roundevenf libc functions. But those are non-standard, and not available on all libc implementations.
cc @bjorn3
There was a problem hiding this comment.
Cranelift doesn't have an instruction for this. If this ever becomes a problem, I can request one.
| ifn!("llvm.roundeven.f32", fn(t_f32) -> t_f32); | ||
| ifn!("llvm.roundeven.f64", fn(t_f64) -> t_f64); |
There was a problem hiding this comment.
@nagisa Can you confirm it's okay to add these llvm intrinsics?
There was a problem hiding this comment.
Yes. It is documented here. How available it is across different backends is a different question, however – a quick look suggests these lower to native instructions on aarch64 and x86 only (and maybe amdgpu). Everything else will lower to libm calls.
dsprenkels
force-pushed
the
round_to_even
branch
from
68074e0
to
2f9185d
Compare
This comment has been minimized.
This comment has been minimized.
dsprenkels
force-pushed
the
round_to_even
branch
from
2f9185d
to
ddd8edc
Compare
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
I find the name |
|
@dsprenkels Looks like this is failing to compile on llvm 9 at least. Can you investigate? (I'm guessing the intrinsics are simply not available on that version?) |
m-ou-se
added
S-waiting-on-author
|
The support for Does this mean that we have to wait a while until we can merge this? |
|
I did not realize that LLVM introduced this intrinsic so recently. I am presuming that, all this time, rust have just used the round-away-from-zero semantics, because that was the default behaviour of LLVM. Would actually maybe be an interest in updating the default behaviour to round-to-even? |
@nagisa Can we only use intrinsics that have been available since llvm 9, or is there a mechanism to have some kind of fallback for older llvm versions? @dsprenkels As an alternative you could directly call the |
|
It is very often the case that compiling to an LLVM "intrinsic" becomes manifested as a call to libm anyways, as already noted. |
|
Hey all, The reason why this issue is stale for me is related to the comment by @workingjubilee. After learning that the round-to-even behaviour should be the default behaviour, I would like to see it changed in the However, this should probably be done by writing an RFC, as (as mentioned) it changes the currently documented behaviour. I have not done this before, so it may take some time until I have motivation to do this. If anybody is interested in collaborating with me on this, feel free to ping me. :) |
|
Oh no, I didn't mean to take the wind out of your sails! |
|
@workingjubilee Don't worry about it. I had already decided this before the comment. :) |
|
@dsprenkels Ping from triage, anything new on this? If you want to chat about it maybe just creating a topic on Zulip t-lang stream will suffice. |
crlf0710
added
S-waiting-on-author
|
@crlf0710 The current status is that I would actually like to not merge this PR, but rather want to update language to uses round-to-even by default instead. This would obviously need an RFC. That's an involved process (and I don't really know exactly what the requirements are, so I'm putting that off at the moment. Any help is appreciated! :) Closing this PR for now. |
…=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`?
…=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`?
…=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`?
When rounding floats that lie exactly inbetween two integers (i.e., every float that ends with
.5), The default rounding behaviour in Rust is rounding away from0. Sometimes however, it is desired to round to the closest even number instead (banker's rounding).It is currently not very easy to use rounding to even numbers in Rust, although LLVM has an intrinsic that supports this.
This patch adds support for rounding using the round-to-even mode.
When does this matter?
It is often not very relevant how this edge case is handled. However, one example of how it could matter is when you are rounding floats in a list, and afterward computing the sum or average. For example:
In this example you see how an unintended statistical bias was introduced, just because of how we handled the rounding of half-integral numbers.
Other langagues
Here is an overview of the default behaviour in some other languages.
[*] Go has a separate
Math.RoundToEvenfunction which does rounding-to-even.See JuliaLang/julia#8750 for a more complete (albeit somewhat outdated) survey.
Alternatives
.round_from_zero()function instead. This may actually not be a very weird idea. I myself was surprised that the default rounding behaviour in Rust was not tie-to-even, and when I look for example at the discussion in float rounding is slow #55107 it seems that I'm not the only one that expected it to behave differently. Obviously, because the documentation currently states promises that the behaviour is tie-away-from-zero, that would probably be a breaking change.Related discussions
const fnbehave exactly the same at runtime as at compile-time? #77745r? @m-ou-se