From 386cae822aa4b4000347b0c36e1ebccb2b090221 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 1 Sep 2024 11:16:01 +0200 Subject: [PATCH] TEST ONLY: remove some x87 work-arounds and make some functions extern-C to try to reproduce an issue --- library/core/src/num/f128.rs | 4 --- library/core/src/num/f16.rs | 44 ++++++-------------------------- library/core/src/num/f32.rs | 49 ++++++++---------------------------- library/core/src/num/f64.rs | 45 ++++++++------------------------- library/std/src/f32/tests.rs | 10 ++++---- 5 files changed, 32 insertions(+), 120 deletions(-) diff --git a/library/core/src/num/f128.rs b/library/core/src/num/f128.rs index d4236e47bfe3b..8f5d8be39d5df 100644 --- a/library/core/src/num/f128.rs +++ b/library/core/src/num/f128.rs @@ -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, diff --git a/library/core/src/num/f16.rs b/library/core/src/num/f16.rs index dfa4740a9d856..dac11b484e1ae 100644 --- a/library/core/src/num/f16.rs +++ b/library/core/src/num/f16.rs @@ -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, } } diff --git a/library/core/src/num/f32.rs b/library/core/src/num/f32.rs index 90bfb073e0074..2172086e6dd95 100644 --- a/library/core/src/num/f32.rs +++ b/library/core/src/num/f32.rs @@ -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, } } @@ -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. @@ -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. @@ -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) } } diff --git a/library/core/src/num/f64.rs b/library/core/src/num/f64.rs index a9240e9bd99dd..036b603cd7158 100644 --- a/library/core/src/num/f64.rs +++ b/library/core/src/num/f64.rs @@ -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, } } @@ -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. @@ -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. @@ -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) } diff --git a/library/std/src/f32/tests.rs b/library/std/src/f32/tests.rs index 99cfcfb231dad..77a74215217fe 100644 --- a/library/std/src/f32/tests.rs +++ b/library/std/src/f32/tests.rs @@ -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);