Skip to content

Commit

Permalink
TEST ONLY: remove some x87 work-arounds and make some functions exter…
Browse files Browse the repository at this point in the history
…n-C to try to reproduce an issue
  • Loading branch information
RalfJung committed Sep 1, 2024
1 parent dcab7d8 commit 386cae8
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 120 deletions.
4 changes: 0 additions & 4 deletions library/core/src/num/f128.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,10 +439,6 @@ impl f128 {
#[unstable(feature = "f128", issue = "116909")]
#[rustc_const_unstable(feature = "const_float_classify", issue = "72505")]
pub const fn classify(self) -> FpCategory {
// Other float types suffer from various platform bugs that violate the usual IEEE semantics
// and also make bitwise classification not always work reliably. However, `f128` cannot fit
// into any other float types so this is not a concern, and we can rely on bit patterns.

let bits = self.to_bits();
match (bits & Self::MAN_MASK, bits & Self::EXP_MASK) {
(0, Self::EXP_MASK) => FpCategory::Infinite,
Expand Down
44 changes: 7 additions & 37 deletions library/core/src/num/f16.rs
Original file line number Diff line number Diff line change
Expand Up @@ -424,43 +424,13 @@ impl f16 {
#[unstable(feature = "f16", issue = "116909")]
#[rustc_const_unstable(feature = "const_float_classify", issue = "72505")]
pub const fn classify(self) -> FpCategory {
// A previous implementation for f32/f64 tried to only use bitmask-based checks,
// using `to_bits` to transmute the float to its bit repr and match on that.
// If we only cared about being "technically" correct, that's an entirely legit
// implementation.
//
// Unfortunately, there are platforms out there that do not correctly implement the IEEE
// float semantics Rust relies on: some hardware flushes denormals to zero, and some
// platforms convert to `f32` to perform operations without properly rounding back (e.g.
// WASM, see llvm/llvm-project#96437). These are platforms bugs, and Rust will misbehave on
// such platforms, but we can at least try to make things seem as sane as possible by being
// careful here.
// Cc https://github.com/rust-lang/rust/issues/114479
if self.is_infinite() {
// Thus, a value may compare unequal to infinity, despite having a "full" exponent mask.
FpCategory::Infinite
} else if self.is_nan() {
// And it may not be NaN, as it can simply be an "overextended" finite value.
FpCategory::Nan
} else {
// However, std can't simply compare to zero to check for zero, either,
// as correctness requires avoiding equality tests that may be Subnormal == -0.0
// because it may be wrong under "denormals are zero" and "flush to zero" modes.
// Most of std's targets don't use those, but they are used for thumbv7neon.
// So, this does use bitpattern matching for the rest. On x87, due to the incorrect
// float codegen on this hardware, this doesn't actually return a right answer for NaN
// because it cannot correctly discern between a floating point NaN, and some normal
// floating point numbers truncated from an x87 FPU -- but we took care of NaN above, so
// we are fine.
// FIXME(jubilee): This probably could at least answer things correctly for Infinity,
// like the f64 version does, but I need to run more checks on how things go on x86.
// I fear losing mantissa data that would have answered that differently.
let b = self.to_bits();
match (b & Self::MAN_MASK, b & Self::EXP_MASK) {
(0, 0) => FpCategory::Zero,
(_, 0) => FpCategory::Subnormal,
_ => FpCategory::Normal,
}
let b = self.to_bits();
match (b & Self::MAN_MASK, b & Self::EXP_MASK) {
(0, Self::EXP_MASK) => FpCategory::Infinite,
(_, Self::EXP_MASK) => FpCategory::Nan,
(0, 0) => FpCategory::Zero,
(_, 0) => FpCategory::Subnormal,
_ => FpCategory::Normal,
}
}

Expand Down
49 changes: 10 additions & 39 deletions library/core/src/num/f32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -652,42 +652,13 @@ impl f32 {
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_float_classify", issue = "72505")]
pub const fn classify(self) -> FpCategory {
// A previous implementation tried to only use bitmask-based checks,
// using f32::to_bits to transmute the float to its bit repr and match on that.
// If we only cared about being "technically" correct, that's an entirely legit
// implementation.
//
// Unfortunately, there is hardware out there that does not correctly implement the IEEE
// float semantics Rust relies on: x87 uses a too-large mantissa and exponent, and some
// hardware flushes subnormals to zero. These are platforms bugs, and Rust will misbehave on
// such hardware, but we can at least try to make things seem as sane as possible by being
// careful here.
// Cc https://github.com/rust-lang/rust/issues/114479
if self.is_infinite() {
// A value may compare unequal to infinity, despite having a "full" exponent mask.
FpCategory::Infinite
} else if self.is_nan() {
// And it may not be NaN, as it can simply be an "overextended" finite value.
FpCategory::Nan
} else {
// However, std can't simply compare to zero to check for zero, either,
// as correctness requires avoiding equality tests that may be Subnormal == -0.0
// because it may be wrong under "denormals are zero" and "flush to zero" modes.
// Most of std's targets don't use those, but they are used for thumbv7neon.
// So, this does use bitpattern matching for the rest. On x87, due to the incorrect
// float codegen on this hardware, this doesn't actually return a right answer for NaN
// because it cannot correctly discern between a floating point NaN, and some normal
// floating point numbers truncated from an x87 FPU -- but we took care of NaN above, so
// we are fine.
// FIXME(jubilee): This probably could at least answer things correctly for Infinity,
// like the f64 version does, but I need to run more checks on how things go on x86.
// I fear losing mantissa data that would have answered that differently.
let b = self.to_bits();
match (b & Self::MAN_MASK, b & Self::EXP_MASK) {
(0, 0) => FpCategory::Zero,
(_, 0) => FpCategory::Subnormal,
_ => FpCategory::Normal,
}
let b = self.to_bits();
match (b & Self::MAN_MASK, b & Self::EXP_MASK) {
(0, Self::EXP_MASK) => FpCategory::Infinite,
(_, Self::EXP_MASK) => FpCategory::Nan,
(0, 0) => FpCategory::Zero,
(_, 0) => FpCategory::Subnormal,
_ => FpCategory::Normal,
}
}

Expand Down Expand Up @@ -773,7 +744,7 @@ impl f32 {
#[inline]
#[unstable(feature = "float_next_up_down", issue = "91399")]
#[rustc_const_unstable(feature = "float_next_up_down", issue = "91399")]
pub const fn next_up(self) -> Self {
pub const extern "C" fn next_up(self) -> Self {
// Some targets violate Rust's assumption of IEEE semantics, e.g. by flushing
// denormals to zero. This is in general unsound and unsupported, but here
// we do our best to still produce the correct result on such targets.
Expand Down Expand Up @@ -822,7 +793,7 @@ impl f32 {
#[inline]
#[unstable(feature = "float_next_up_down", issue = "91399")]
#[rustc_const_unstable(feature = "float_next_up_down", issue = "91399")]
pub const fn next_down(self) -> Self {
pub const extern "C" fn next_down(self) -> Self {
// Some targets violate Rust's assumption of IEEE semantics, e.g. by flushing
// denormals to zero. This is in general unsound and unsupported, but here
// we do our best to still produce the correct result on such targets.
Expand Down Expand Up @@ -1114,7 +1085,7 @@ impl f32 {
#[stable(feature = "float_bits_conv", since = "1.20.0")]
#[rustc_const_unstable(feature = "const_float_bits_conv", issue = "72447")]
#[inline]
pub const fn to_bits(self) -> u32 {
pub const extern "C" fn to_bits(self) -> u32 {
// SAFETY: `u32` is a plain old datatype so we can always transmute to it.
unsafe { mem::transmute(self) }
}
Expand Down
45 changes: 10 additions & 35 deletions library/core/src/num/f64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -651,38 +651,13 @@ impl f64 {
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_float_classify", issue = "72505")]
pub const fn classify(self) -> FpCategory {
// A previous implementation tried to only use bitmask-based checks,
// using f64::to_bits to transmute the float to its bit repr and match on that.
// If we only cared about being "technically" correct, that's an entirely legit
// implementation.
//
// Unfortunately, there is hardware out there that does not correctly implement the IEEE
// float semantics Rust relies on: x87 uses a too-large exponent, and some hardware flushes
// subnormals to zero. These are platforms bugs, and Rust will misbehave on such hardware,
// but we can at least try to make things seem as sane as possible by being careful here.
// Cc https://github.com/rust-lang/rust/issues/114479
//
// Thus, a value may compare unequal to infinity, despite having a "full" exponent mask.
// And it may not be NaN, as it can simply be an "overextended" finite value.
if self.is_nan() {
FpCategory::Nan
} else {
// However, std can't simply compare to zero to check for zero, either,
// as correctness requires avoiding equality tests that may be Subnormal == -0.0
// because it may be wrong under "denormals are zero" and "flush to zero" modes.
// Most of std's targets don't use those, but they are used for thumbv7neon.
// So, this does use bitpattern matching for the rest. On x87, due to the incorrect
// float codegen on this hardware, this doesn't actually return a right answer for NaN
// because it cannot correctly discern between a floating point NaN, and some normal
// floating point numbers truncated from an x87 FPU -- but we took care of NaN above, so
// we are fine.
let b = self.to_bits();
match (b & Self::MAN_MASK, b & Self::EXP_MASK) {
(0, Self::EXP_MASK) => FpCategory::Infinite,
(0, 0) => FpCategory::Zero,
(_, 0) => FpCategory::Subnormal,
_ => FpCategory::Normal,
}
let b = self.to_bits();
match (b & Self::MAN_MASK, b & Self::EXP_MASK) {
(0, Self::EXP_MASK) => FpCategory::Infinite,
(_, Self::EXP_MASK) => FpCategory::Nan,
(0, 0) => FpCategory::Zero,
(_, 0) => FpCategory::Subnormal,
_ => FpCategory::Normal,
}
}

Expand Down Expand Up @@ -786,7 +761,7 @@ impl f64 {
#[inline]
#[unstable(feature = "float_next_up_down", issue = "91399")]
#[rustc_const_unstable(feature = "float_next_up_down", issue = "91399")]
pub const fn next_up(self) -> Self {
pub const extern "C" fn next_up(self) -> Self {
// Some targets violate Rust's assumption of IEEE semantics, e.g. by flushing
// denormals to zero. This is in general unsound and unsupported, but here
// we do our best to still produce the correct result on such targets.
Expand Down Expand Up @@ -835,7 +810,7 @@ impl f64 {
#[inline]
#[unstable(feature = "float_next_up_down", issue = "91399")]
#[rustc_const_unstable(feature = "float_next_up_down", issue = "91399")]
pub const fn next_down(self) -> Self {
pub const extern "C" fn next_down(self) -> Self {
// Some targets violate Rust's assumption of IEEE semantics, e.g. by flushing
// denormals to zero. This is in general unsound and unsupported, but here
// we do our best to still produce the correct result on such targets.
Expand Down Expand Up @@ -1155,7 +1130,7 @@ impl f64 {
#[rustc_const_unstable(feature = "const_float_bits_conv", issue = "72447")]
#[must_use]
#[inline]
pub const fn from_bits(v: u64) -> Self {
pub const extern "C" fn from_bits(v: u64) -> Self {
// It turns out the safety issues with sNaN were overblown! Hooray!
// SAFETY: `u64` is a plain old datatype so we can always transmute from it.
unsafe { mem::transmute(v) }
Expand Down
10 changes: 5 additions & 5 deletions library/std/src/f32/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,11 +348,11 @@ fn test_is_sign_negative() {

#[test]
fn test_next_up() {
let tiny = f32::from_bits(TINY_BITS);
let tiny_up = f32::from_bits(TINY_UP_BITS);
let max_down = f32::from_bits(MAX_DOWN_BITS);
let largest_subnormal = f32::from_bits(LARGEST_SUBNORMAL_BITS);
let smallest_normal = f32::from_bits(SMALLEST_NORMAL_BITS);
let tiny = crate::hint::black_box(f32::from_bits(TINY_BITS));
let tiny_up = crate::hint::black_box(f32::from_bits(TINY_UP_BITS));
let max_down = crate::hint::black_box(f32::from_bits(MAX_DOWN_BITS));
let largest_subnormal = crate::hint::black_box(f32::from_bits(LARGEST_SUBNORMAL_BITS));
let smallest_normal = crate::hint::black_box(f32::from_bits(SMALLEST_NORMAL_BITS));
assert_f32_biteq!(f32::NEG_INFINITY.next_up(), f32::MIN);
assert_f32_biteq!(f32::MIN.next_up(), -max_down);
assert_f32_biteq!((-1.0 - f32::EPSILON).next_up(), -1.0);
Expand Down

0 comments on commit 386cae8

Please sign in to comment.