Skip to content

Fix wrong rounding behaviour about div_euclid with fma operation. #158154

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.

Sign up for GitHub

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

Open
Neutron3529 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix wrong rounding behaviour about div_euclid with fma operation. #158154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions library/core/src/num/f128.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1867,11 +1867,21 @@ impl f128 {
#[unstable(feature = "f128", issue = "116909")]
#[must_use = "method returns a new number and does not mutate the original value"]
pub fn div_euclid(self, rhs: f128) -> f128 {
let q = (self / rhs).trunc();
if self % rhs < 0.0 {
return if rhs > 0.0 { q - 1.0 } else { q + 1.0 };
// Use floor() directly as the initial guess (Euclidean division
// with positive divisor is the same as floor division).
let q = (self / rhs).floor();
// Compute b * q - a with a single rounding error.
let diff = rhs.mul_add(q, -self);
if diff > 0.0 { // take care of NaN: NaN > 0.0 is false, keeping the return result.
if rhs > 0.0 {
// q was one unit too high – move down to the next representable value.
q.next_down().floor()

@dotacow dotacow Jun 19, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: is this not effectively q - 1.0, since q is already floored and thus an integer value?

View changes since the review

@Neutron3529 Neutron3529 Jun 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This trying keeps q * rhs <= left always hold.
for larger floats.q-1.0 may equals to q itself, which might cause q * rhs <= left not hold.

Maybe it is worth discussion that whether we need this version of div_euclid?

@dotacow dotacow Jun 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, yeah the logic holds.

I went over the paper linked in #107904, and your implementation seems to fall in line nicely with its general outline, LGTM.

Maybe it is worth discussion that whether we need this version of div_euclid?

This is arguably a slower version (in contrast to the previous implementation), but it is correct. I think delegating the implementation to user level would just cause many of them to fall in the same trap.

} else {
q.next_up().ceil()
}
} else {
q
}
q
}

/// Calculates the least nonnegative remainder of `self` when
Expand Down
18 changes: 14 additions & 4 deletions library/core/src/num/f16.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1853,11 +1853,21 @@ impl f16 {
#[unstable(feature = "f16", issue = "116909")]
#[must_use = "method returns a new number and does not mutate the original value"]
pub fn div_euclid(self, rhs: f16) -> f16 {
let q = (self / rhs).trunc();
if self % rhs < 0.0 {
return if rhs > 0.0 { q - 1.0 } else { q + 1.0 };
// Use floor() directly as the initial guess (Euclidean division
// with positive divisor is the same as floor division).
let q = (self / rhs).floor();
// Compute b * q - a with a single rounding error.
let diff = rhs.mul_add(q, -self);
if diff > 0.0 { // take care of NaN: NaN > 0.0 is false, keeping the return result.
if rhs > 0.0 {
// q was one unit too high – move down to the next representable value.
q.next_down().floor()
} else {
q.next_up().ceil()
}
} else {
q
}
q
}

/// Calculates the least nonnegative remainder of `self` when
Expand Down
18 changes: 14 additions & 4 deletions library/core/src/num/f32.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2007,11 +2007,21 @@ pub mod math {
#[unstable(feature = "core_float_math", issue = "137578")]
#[must_use = "method returns a new number and does not mutate the original value"]
pub fn div_euclid(x: f32, rhs: f32) -> f32 {
let q = trunc(x / rhs);
if x % rhs < 0.0 {
return if rhs > 0.0 { q - 1.0 } else { q + 1.0 };
// Use floor() directly as the initial guess (Euclidean division
// with positive divisor is the same as floor division).
let q = (x / rhs).floor();
// Compute b * q - a with a single rounding error.
let diff = rhs.mul_add(q, -x);
if diff > 0.0 { // take care of NaN: NaN > 0.0 is false, keeping the return result.
if rhs > 0.0 {
// q was one unit too high – move down to the next representable value.
q.next_down().floor()
} else {
q.next_up().ceil()
}
} else {
q
}
q
}

/// Experimental version of `rem_euclid` in `core`. See [`f32::rem_euclid`] for details.
Expand Down
18 changes: 14 additions & 4 deletions library/core/src/num/f64.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1987,11 +1987,21 @@ pub mod math {
#[unstable(feature = "core_float_math", issue = "137578")]
#[must_use = "method returns a new number and does not mutate the original value"]
pub fn div_euclid(x: f64, rhs: f64) -> f64 {
let q = trunc(x / rhs);
if x % rhs < 0.0 {
return if rhs > 0.0 { q - 1.0 } else { q + 1.0 };
// Use floor() directly as the initial guess (Euclidean division
// with positive divisor is the same as floor division).
let q = (x / rhs).floor();
// Compute b * q - a with a single rounding error.
let diff = rhs.mul_add(q, -x);
if diff > 0.0 { // take care of NaN: NaN > 0.0 is false, keeping the return result.
if rhs > 0.0 {
// q was one unit too high – move down to the next representable value.
q.next_down().floor()
} else {
q.next_up().ceil()
}
} else {
q
}
q
}

/// Experimental version of `rem_euclid` in `core`. See [`f64::rem_euclid`] for details.
Expand Down
16 changes: 16 additions & 0 deletions library/coretests/tests/num/floats.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -994,6 +994,22 @@ float_test! {
}
}

float_test! {
name: div_rem_euclid,
attrs: {
const: #[cfg(false)],
// Miri only uses softfloats here, so that always works
f16: #[cfg(any(miri, target_has_reliable_f16_math))],
f128: #[cfg(any(miri, target_has_reliable_f128_math))],
},
test {
assert_approx_eq!(flt(11.0).div_euclid(flt(2.2)) * flt(2.2) + flt(11.0).rem_euclid(flt(2.2)), flt(11.0));
assert_approx_eq!(flt(12.0).div_euclid(flt(2.4)) * flt(2.4) + flt(12.0).rem_euclid(flt(2.4)), flt(12.0));
assert_approx_eq!(flt(13.0).div_euclid(flt(2.6)) * flt(2.6) + flt(13.0).rem_euclid(flt(2.6)), flt(13.0));
assert_approx_eq!(flt(14.0).div_euclid(flt(2.8)) * flt(2.8) + flt(14.0).rem_euclid(flt(2.8)), flt(14.0));
}
}

float_test! {
name: floor,
attrs: {
Expand Down
Loading