Skip to content

Commit abe63f6

Browse files
RalfJungRalfJung
committed
simplify float::classify logic
1 parent 5997b68 commit abe63f6

File tree

4 files changed

+31
-109
lines changed

4 files changed

+31
-109
lines changed

‎core/src/num/f128.rs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -439,10 +439,6 @@ impl f128 {
439439
#[unstable(feature = "f128", issue = "116909")]
440440
#[rustc_const_unstable(feature = "const_float_classify", issue = "72505")]
441441
pub const fn classify(self) -> FpCategory {
442-
// Other float types suffer from various platform bugs that violate the usual IEEE semantics
443-
// and also make bitwise classification not always work reliably. However, `f128` cannot fit
444-
// into any other float types so this is not a concern, and we can rely on bit patterns.
445-
446442
let bits = self.to_bits();
447443
match (bits & Self::MAN_MASK, bits & Self::EXP_MASK) {
448444
(0, Self::EXP_MASK) => FpCategory::Infinite,

‎core/src/num/f16.rs‎

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

‎core/src/num/f32.rs‎

Lines changed: 12 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -652,42 +652,18 @@ impl f32 {
652652
#[stable(feature = "rust1", since = "1.0.0")]
653653
#[rustc_const_unstable(feature = "const_float_classify", issue = "72505")]
654654
pub const fn classify(self) -> FpCategory {
655-
// A previous implementation tried to only use bitmask-based checks,
656-
// using f32::to_bits to transmute the float to its bit repr and match on that.
657-
// If we only cared about being "technically" correct, that's an entirely legit
658-
// implementation.
659-
//
660-
// Unfortunately, there is hardware out there that does not correctly implement the IEEE
661-
// float semantics Rust relies on: x87 uses a too-large mantissa and exponent, and some
662-
// hardware flushes subnormals to zero. These are platforms bugs, and Rust will misbehave on
663-
// such hardware, but we can at least try to make things seem as sane as possible by being
664-
// careful here.
665-
// see also https://github.com/rust-lang/rust/issues/114479
666-
if self.is_infinite() {
667-
// A value may compare unequal to infinity, despite having a "full" exponent mask.
668-
FpCategory::Infinite
669-
} else if self.is_nan() {
670-
// And it may not be NaN, as it can simply be an "overextended" finite value.
671-
FpCategory::Nan
672-
} else {
673-
// However, std can't simply compare to zero to check for zero, either,
674-
// as correctness requires avoiding equality tests that may be Subnormal == -0.0
675-
// because it may be wrong under "denormals are zero" and "flush to zero" modes.
676-
// Most of std's targets don't use those, but they are used for thumbv7neon.
677-
// So, this does use bitpattern matching for the rest. On x87, due to the incorrect
678-
// float codegen on this hardware, this doesn't actually return a right answer for NaN
679-
// because it cannot correctly discern between a floating point NaN, and some normal
680-
// floating point numbers truncated from an x87 FPU -- but we took care of NaN above, so
681-
// we are fine.
682-
// FIXME(jubilee): This probably could at least answer things correctly for Infinity,
683-
// like the f64 version does, but I need to run more checks on how things go on x86.
684-
// I fear losing mantissa data that would have answered that differently.
685-
let b = self.to_bits();
686-
match (b & Self::MAN_MASK, b & Self::EXP_MASK) {
687-
(0, 0) => FpCategory::Zero,
688-
(_, 0) => FpCategory::Subnormal,
689-
_ => FpCategory::Normal,
690-
}
655+
// We used to have complicated logic here that avoids the simple bit-based tests to work
656+
// around buggy codegen for x87 targets (see
657+
// https://github.com/rust-lang/rust/issues/114479). However, some LLVM versions later, none
658+
// of our tests is able to find any difference between the complicated and the naive
659+
// version, so now we are back to the naive version.
660+
let b = self.to_bits();
661+
match (b & Self::MAN_MASK, b & Self::EXP_MASK) {
662+
(0, Self::EXP_MASK) => FpCategory::Infinite,
663+
(_, Self::EXP_MASK) => FpCategory::Nan,
664+
(0, 0) => FpCategory::Zero,
665+
(_, 0) => FpCategory::Subnormal,
666+
_ => FpCategory::Normal,
691667
}
692668
}
693669

‎core/src/num/f64.rs‎

Lines changed: 12 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -651,38 +651,18 @@ impl f64 {
651651
#[stable(feature = "rust1", since = "1.0.0")]
652652
#[rustc_const_unstable(feature = "const_float_classify", issue = "72505")]
653653
pub const fn classify(self) -> FpCategory {
654-
// A previous implementation tried to only use bitmask-based checks,
655-
// using f64::to_bits to transmute the float to its bit repr and match on that.
656-
// If we only cared about being "technically" correct, that's an entirely legit
657-
// implementation.
658-
//
659-
// Unfortunately, there is hardware out there that does not correctly implement the IEEE
660-
// float semantics Rust relies on: x87 uses a too-large exponent, and some hardware flushes
661-
// subnormals to zero. These are platforms bugs, and Rust will misbehave on such hardware,
662-
// but we can at least try to make things seem as sane as possible by being careful here.
663-
// see also https://github.com/rust-lang/rust/issues/114479
664-
//
665-
// Thus, a value may compare unequal to infinity, despite having a "full" exponent mask.
666-
// And it may not be NaN, as it can simply be an "overextended" finite value.
667-
if self.is_nan() {
668-
FpCategory::Nan
669-
} else {
670-
// However, std can't simply compare to zero to check for zero, either,
671-
// as correctness requires avoiding equality tests that may be Subnormal == -0.0
672-
// because it may be wrong under "denormals are zero" and "flush to zero" modes.
673-
// Most of std's targets don't use those, but they are used for thumbv7neon.
674-
// So, this does use bitpattern matching for the rest. On x87, due to the incorrect
675-
// float codegen on this hardware, this doesn't actually return a right answer for NaN
676-
// because it cannot correctly discern between a floating point NaN, and some normal
677-
// floating point numbers truncated from an x87 FPU -- but we took care of NaN above, so
678-
// we are fine.
679-
let b = self.to_bits();
680-
match (b & Self::MAN_MASK, b & Self::EXP_MASK) {
681-
(0, Self::EXP_MASK) => FpCategory::Infinite,
682-
(0, 0) => FpCategory::Zero,
683-
(_, 0) => FpCategory::Subnormal,
684-
_ => FpCategory::Normal,
685-
}
654+
// We used to have complicated logic here that avoids the simple bit-based tests to work
655+
// around buggy codegen for x87 targets (see
656+
// https://github.com/rust-lang/rust/issues/114479). However, some LLVM versions later, none
657+
// of our tests is able to find any difference between the complicated and the naive
658+
// version, so now we are back to the naive version.
659+
let b = self.to_bits();
660+
match (b & Self::MAN_MASK, b & Self::EXP_MASK) {
661+
(0, Self::EXP_MASK) => FpCategory::Infinite,
662+
(_, Self::EXP_MASK) => FpCategory::Nan,
663+
(0, 0) => FpCategory::Zero,
664+
(_, 0) => FpCategory::Subnormal,
665+
_ => FpCategory::Normal,
686666
}
687667
}
688668

0 commit comments

Comments
 (0)