Fix NaN handling of simd float min and max operations

7 files changed, 83 insertions, 41 deletions

diff --git a/example/float-minmax-pass.rs b/example/float-minmax-pass.rs
new file mode 100644
index 00000000000..b8f901d1ba1
--- /dev/null
+++ b/example/float-minmax-pass.rs
@@ -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);
+    }
+}
diff --git a/patches/0001-portable-simd-Disable-unsupported-tests.patch b/patches/0001-portable-simd-Disable-unsupported-tests.patch
index c1325908691..b952b21e6e6 100644
--- a/patches/0001-portable-simd-Disable-unsupported-tests.patch
+++ b/patches/0001-portable-simd-Disable-unsupported-tests.patch
@@ -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 {
                          )
                      }
diff --git a/scripts/test_rustc_tests.sh b/scripts/test_rustc_tests.sh
index 5f47e7204ea..3d6bd887459 100755
--- a/scripts/test_rustc_tests.sh
+++ b/scripts/test_rustc_tests.sh
@@ -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
diff --git a/scripts/tests.sh b/scripts/tests.sh
index fee1012c8f1..aae626081f6 100755
--- a/scripts/tests.sh
+++ b/scripts/tests.sh
@@ -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
diff --git a/src/intrinsics/mod.rs b/src/intrinsics/mod.rs
index 310d27c6dec..d76dfca7960 100644
--- a/src/intrinsics/mod.rs
+++ b/src/intrinsics/mod.rs
@@ -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);
         };
diff --git a/src/intrinsics/simd.rs b/src/intrinsics/simd.rs
index bc21d736166..25755f8c7ec 100644
--- a/src/intrinsics/simd.rs
+++ b/src/intrinsics/simd.rs
@@ -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)
diff --git a/src/num.rs b/src/num.rs
index 545d390e269..4ce8adb182e 100644
--- a/src/num.rs
+++ b/src/num.rs
@@ -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)
+}