user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix NaN handling of simd float min and max operations

Browse source
This commit is contained in:
bjorn3 2022-03-25 20:24:47 +01:00
parent f3d97cce27
commit 3c030e2425
7 changed files with 83 additions and 41 deletions
Download patch file Download diff file Expand all files Collapse all files

53
example/float-minmax-pass.rs Normal file
View file

@ -0,0 +1,53 @@
// Copied from https://github.com/rust-lang/rust/blob/3fe3b89cd57229343eeca753fdd8c63d9b03c65c/src/test/ui/simd/intrinsic/float-minmax-pass.rs
// run-pass
// ignore-emscripten
// Test that the simd_f{min,max} intrinsics produce the correct results.
#![feature(repr_simd, platform_intrinsics)]
#![allow(non_camel_case_types)]
#[repr(simd)]
#[derive(Copy, Clone, PartialEq, Debug)]
struct f32x4(pub f32, pub f32, pub f32, pub f32);
extern "platform-intrinsic" {
fn simd_fmin<T>(x: T, y: T) -> T;
fn simd_fmax<T>(x: T, y: T) -> T;
}
fn main() {
let x = f32x4(1.0, 2.0, 3.0, 4.0);
let y = f32x4(2.0, 1.0, 4.0, 3.0);
#[cfg(not(any(target_arch = "mips", target_arch = "mips64")))]
let nan = f32::NAN;
// MIPS hardware treats f32::NAN as SNAN. Clear the signaling bit.
// See https://github.com/rust-lang/rust/issues/52746.
#[cfg(any(target_arch = "mips", target_arch = "mips64"))]
let nan = f32::from_bits(f32::NAN.to_bits() - 1);
let n = f32x4(nan, nan, nan, nan);
unsafe {
let min0 = simd_fmin(x, y);
let min1 = simd_fmin(y, x);
assert_eq!(min0, min1);
let e = f32x4(1.0, 1.0, 3.0, 3.0);
assert_eq!(min0, e);
let minn = simd_fmin(x, n);
assert_eq!(minn, x);
let minn = simd_fmin(y, n);
assert_eq!(minn, y);
let max0 = simd_fmax(x, y);
let max1 = simd_fmax(y, x);
assert_eq!(max0, max1);
let e = f32x4(2.0, 2.0, 4.0, 4.0);
assert_eq!(max0, e);
let maxn = simd_fmax(x, n);
assert_eq!(maxn, x);
let maxn = simd_fmax(y, n);
assert_eq!(maxn, y);
}
}

16
patches/0001-portable-simd-Disable-unsupported-tests.patch
View file

@ -106,22 +106,6 @@ diff --git a/crates/core_simd/tests/ops_macros.rs b/crates/core_simd/tests/ops_m
index 31b7ee2..bd04b3c 100644
--- a/crates/core_simd/tests/ops_macros.rs
+++ b/crates/core_simd/tests/ops_macros.rs
@@ -567,6 +567,7 @@ macro_rules! impl_float_tests {
});
}
+ /*
fn horizontal_max<const LANES: usize>() {
test_helpers::test_1(&|x| {
let vmax = Vector::<LANES>::from_array(x).horizontal_max();
@@ -590,6 +591,7 @@ macro_rules! impl_float_tests {
Ok(())
});
}
+ */
}
#[cfg(feature = "std")]
@@ -604,6 +606,7 @@ macro_rules! impl_float_tests {
)
}

1
scripts/test_rustc_tests.sh
View file

@ -79,7 +79,6 @@ rm src/test/ui/abi/stack-protector.rs # requires stack protector support
# giving different but possibly correct results
# =============================================
rm src/test/ui/simd/intrinsic/float-minmax-pass.rs # same
rm src/test/ui/mir/mir_misc_casts.rs # depends on deduplication of constants
rm src/test/ui/mir/mir_raw_fat_ptr.rs # same
rm src/test/ui/consts/issue-33537.rs # same

4
scripts/tests.sh
View file

@ -72,6 +72,10 @@ function base_sysroot_tests() {
$MY_RUSTC example/track-caller-attribute.rs --crate-type bin -Cpanic=abort --target "$TARGET_TRIPLE"
$RUN_WRAPPER ./target/out/track-caller-attribute
echo "[AOT] float-minmax-pass"
$MY_RUSTC example/float-minmax-pass.rs --crate-type bin -Cpanic=abort --target "$TARGET_TRIPLE"
$RUN_WRAPPER ./target/out/float-minmax-pass
echo "[AOT] mod_bench"
$MY_RUSTC example/mod_bench.rs --crate-type bin --target "$TARGET_TRIPLE"
$RUN_WRAPPER ./target/out/mod_bench

24
src/intrinsics/mod.rs
View file

@ -1019,39 +1019,23 @@ fn codegen_regular_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 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 = crate::num::codegen_float_min(fx, a, b);
let val = CValue::by_val(val, fx.layout_of(fx.tcx.types.f32));
ret.write_cvalue(fx, val);
};
minnumf64, (v a, v 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 = crate::num::codegen_float_min(fx, a, b);
let val = CValue::by_val(val, fx.layout_of(fx.tcx.types.f64));
ret.write_cvalue(fx, val);
};
maxnumf32, (v a, v 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 = crate::num::codegen_float_max(fx, a, b);
let val = CValue::by_val(val, fx.layout_of(fx.tcx.types.f32));
ret.write_cvalue(fx, val);
};
maxnumf64, (v a, v 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 = crate::num::codegen_float_max(fx, a, b);
let val = CValue::by_val(val, fx.layout_of(fx.tcx.types.f64));
ret.write_cvalue(fx, val);
};

8
src/intrinsics/simd.rs
View file

@ -354,8 +354,8 @@ pub(super) fn codegen_simd_intrinsic_call<'tcx>(
_ => unreachable!("{:?}", lane_ty),
}
match intrinsic {
sym::simd_fmin => fx.bcx.ins().fmin(x_lane, y_lane),
sym::simd_fmax => fx.bcx.ins().fmax(x_lane, y_lane),
sym::simd_fmin => crate::num::codegen_float_min(fx, x_lane, y_lane),
sym::simd_fmax => crate::num::codegen_float_max(fx, x_lane, y_lane),
_ => unreachable!(),
}
});
@ -495,7 +495,7 @@ pub(super) fn codegen_simd_intrinsic_call<'tcx>(
let lt = match ty.kind() {
ty::Int(_) => fx.bcx.ins().icmp(IntCC::SignedLessThan, a, b),
ty::Uint(_) => fx.bcx.ins().icmp(IntCC::UnsignedLessThan, a, b),
ty::Float(_) => fx.bcx.ins().fcmp(FloatCC::LessThan, a, b),
ty::Float(_) => return crate::num::codegen_float_min(fx, a, b),
_ => unreachable!(),
};
fx.bcx.ins().select(lt, a, b)
@ -512,7 +512,7 @@ pub(super) fn codegen_simd_intrinsic_call<'tcx>(
let gt = match ty.kind() {
ty::Int(_) => fx.bcx.ins().icmp(IntCC::SignedGreaterThan, a, b),
ty::Uint(_) => fx.bcx.ins().icmp(IntCC::UnsignedGreaterThan, a, b),
ty::Float(_) => fx.bcx.ins().fcmp(FloatCC::GreaterThan, a, b),
ty::Float(_) => return crate::num::codegen_float_max(fx, a, b),
_ => unreachable!(),
};
fx.bcx.ins().select(gt, a, b)

18
src/num.rs
View file

@ -420,3 +420,21 @@ pub(crate) fn codegen_ptr_binop<'tcx>(
CValue::by_val(fx.bcx.ins().bint(types::I8, res), fx.layout_of(fx.tcx.types.bool))
}
}
// 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.
pub(crate) fn codegen_float_min(fx: &mut FunctionCx<'_, '_, '_>, a: Value, b: Value) -> Value {
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);
fx.bcx.ins().select(a_is_nan, b, temp)
}
pub(crate) fn codegen_float_max(fx: &mut FunctionCx<'_, '_, '_>, a: Value, b: Value) -> Value {
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);
fx.bcx.ins().select(a_is_nan, b, temp)
}