Fix float min and max operations in presence of NaN

Cranelift's fmin and fmax instructions propagate NaN, while Rust's min
and max don't.

Fixes #1049
This commit is contained in:
bjorn3 2021-07-29 15:20:37 +02:00
parent e0b9f3b3cc
commit c6564f814e
5 changed files with 23 additions and 60 deletions

View File

@ -124,22 +124,6 @@ index cb39e73..fc0ebe1 100644
fn sqrt<const LANES: usize>() {
test_helpers::test_unary_elementwise(
@@ -491,6 +493,7 @@ macro_rules! impl_float_tests {
)
}
+ /*
fn min<const LANES: usize>() {
// Regular conditions (both values aren't zero)
test_helpers::test_binary_elementwise(
@@ -536,6 +539,7 @@ macro_rules! impl_float_tests {
assert!(p_zero.max(n_zero).to_array().iter().all(|x| *x == 0.));
assert!(n_zero.max(p_zero).to_array().iter().all(|x| *x == 0.));
}
+ */
fn clamp<const LANES: usize>() {
test_helpers::test_3(&|value: [Scalar; LANES], mut min: [Scalar; LANES], mut max: [Scalar; LANES]| {
@@ -581,6 +585,7 @@ macro_rules! impl_float_tests {
});
}

View File

@ -46,45 +46,5 @@ index 4bc44e9..8e3c7a4 100644
#[test]
fn cell_allows_array_cycle() {
diff --git a/library/core/tests/num/mod.rs b/library/core/tests/num/mod.rs
index a17c094..5bb11d2 100644
--- a/library/core/tests/num/mod.rs
+++ b/library/core/tests/num/mod.rs
@@ -651,11 +651,12 @@ macro_rules! test_float {
assert_eq!((9.0 as $fty).min($neginf), $neginf);
assert_eq!(($neginf as $fty).min(-9.0), $neginf);
assert_eq!((-9.0 as $fty).min($neginf), $neginf);
- assert_eq!(($nan as $fty).min(9.0), 9.0);
- assert_eq!(($nan as $fty).min(-9.0), -9.0);
- assert_eq!((9.0 as $fty).min($nan), 9.0);
- assert_eq!((-9.0 as $fty).min($nan), -9.0);
- assert!(($nan as $fty).min($nan).is_nan());
+ // Cranelift fmin has NaN propagation
+ //assert_eq!(($nan as $fty).min(9.0), 9.0);
+ //assert_eq!(($nan as $fty).min(-9.0), -9.0);
+ //assert_eq!((9.0 as $fty).min($nan), 9.0);
+ //assert_eq!((-9.0 as $fty).min($nan), -9.0);
+ //assert!(($nan as $fty).min($nan).is_nan());
}
#[test]
fn max() {
@@ -673,11 +674,12 @@ macro_rules! test_float {
assert_eq!((9.0 as $fty).max($neginf), 9.0);
assert_eq!(($neginf as $fty).max(-9.0), -9.0);
assert_eq!((-9.0 as $fty).max($neginf), -9.0);
- assert_eq!(($nan as $fty).max(9.0), 9.0);
- assert_eq!(($nan as $fty).max(-9.0), -9.0);
- assert_eq!((9.0 as $fty).max($nan), 9.0);
- assert_eq!((-9.0 as $fty).max($nan), -9.0);
- assert!(($nan as $fty).max($nan).is_nan());
+ // Cranelift fmax has NaN propagation
+ //assert_eq!(($nan as $fty).max(9.0), 9.0);
+ //assert_eq!(($nan as $fty).max(-9.0), -9.0);
+ //assert_eq!((9.0 as $fty).max($nan), 9.0);
+ //assert_eq!((-9.0 as $fty).max($nan), -9.0);
+ //assert!(($nan as $fty).max($nan).is_nan());
}
#[test]
fn rem_euclid() {
--
2.21.0 (Apple Git-122)

View File

@ -139,6 +139,7 @@ function extended_sysroot_tests() {
pushd stdsimd
echo "[TEST] rust-lang/stdsimd"
../build/cargo clean
../build/cargo build --all-targets --target $TARGET_TRIPLE
if [[ "$HOST_TRIPLE" = "$TARGET_TRIPLE" ]]; then
../build/cargo test -q

View File

@ -1029,23 +1029,39 @@ pub(crate) fn codegen_intrinsic_call<'tcx>(
ret.write_cvalue(fx, old);
};
// In Rust floating point min and max don't propagate NaN. In Cranelift they do however.
// For this reason it is necessary to use `a.is_nan() ? b : (a >= b ? b : a)` for `minnumf*`
// and `a.is_nan() ? b : (a <= b ? b : a)` for `maxnumf*`. NaN checks are done by comparing
// a float against itself. Only in case of NaN is it not equal to itself.
minnumf32, (v a, v b) {
let val = fx.bcx.ins().fmin(a, b);
let a_is_nan = fx.bcx.ins().fcmp(FloatCC::NotEqual, a, a);
let a_ge_b = fx.bcx.ins().fcmp(FloatCC::GreaterThanOrEqual, a, b);
let temp = fx.bcx.ins().select(a_ge_b, b, a);
let val = fx.bcx.ins().select(a_is_nan, b, temp);
let val = CValue::by_val(val, fx.layout_of(fx.tcx.types.f32));
ret.write_cvalue(fx, val);
};
minnumf64, (v a, v b) {
let val = fx.bcx.ins().fmin(a, b);
let a_is_nan = fx.bcx.ins().fcmp(FloatCC::NotEqual, a, a);
let a_ge_b = fx.bcx.ins().fcmp(FloatCC::GreaterThanOrEqual, a, b);
let temp = fx.bcx.ins().select(a_ge_b, b, a);
let val = fx.bcx.ins().select(a_is_nan, b, temp);
let val = CValue::by_val(val, fx.layout_of(fx.tcx.types.f64));
ret.write_cvalue(fx, val);
};
maxnumf32, (v a, v b) {
let val = fx.bcx.ins().fmax(a, b);
let a_is_nan = fx.bcx.ins().fcmp(FloatCC::NotEqual, a, a);
let a_le_b = fx.bcx.ins().fcmp(FloatCC::LessThanOrEqual, a, b);
let temp = fx.bcx.ins().select(a_le_b, b, a);
let val = fx.bcx.ins().select(a_is_nan, b, temp);
let val = CValue::by_val(val, fx.layout_of(fx.tcx.types.f32));
ret.write_cvalue(fx, val);
};
maxnumf64, (v a, v b) {
let val = fx.bcx.ins().fmax(a, b);
let a_is_nan = fx.bcx.ins().fcmp(FloatCC::NotEqual, a, a);
let a_le_b = fx.bcx.ins().fcmp(FloatCC::LessThanOrEqual, a, b);
let temp = fx.bcx.ins().select(a_le_b, b, a);
let val = fx.bcx.ins().select(a_is_nan, b, temp);
let val = CValue::by_val(val, fx.layout_of(fx.tcx.types.f64));
ret.write_cvalue(fx, val);
};

View File

@ -378,6 +378,7 @@ pub(super) fn codegen_simd_intrinsic_call<'tcx>(
};
simd_reduce_min, (c v) {
// FIXME support floats
validate_simd_type!(fx, intrinsic, span, v.layout().ty);
simd_reduce(fx, v, None, ret, |fx, layout, a, b| {
let lt = fx.bcx.ins().icmp(if layout.ty.is_signed() {
@ -390,6 +391,7 @@ pub(super) fn codegen_simd_intrinsic_call<'tcx>(
};
simd_reduce_max, (c v) {
// FIXME support floats
validate_simd_type!(fx, intrinsic, span, v.layout().ty);
simd_reduce(fx, v, None, ret, |fx, layout, a, b| {
let gt = fx.bcx.ins().icmp(if layout.ty.is_signed() {