From 8763a05898820ec484ea86c3d5241bf96ec9f60e Mon Sep 17 00:00:00 2001 From: Florian Maurin <5298202-florian360@users.noreply.gitlab.com> Date: Fri, 21 Aug 2026 16:13:09 +0000 Subject: [PATCH] Core: Avoid NaN seeds in RVV min/max reductions libeigen/eigen!2890 --- Eigen/src/Core/arch/RVV10/PacketMath.h | 26 ++++----------- Eigen/src/Core/arch/RVV10/PacketMath2.h | 30 ++++++----------- Eigen/src/Core/arch/RVV10/PacketMath4.h | 30 ++++++----------- Eigen/src/Core/arch/RVV10/PacketMathFP16.h | 36 ++++++-------------- test/packetmath.cpp | 26 +++++++++++++++ test/packetmath_fastmath.cpp | 38 ++++++++++++++++++++++ 6 files changed, 100 insertions(+), 86 deletions(-) diff --git a/Eigen/src/Core/arch/RVV10/PacketMath.h b/Eigen/src/Core/arch/RVV10/PacketMath.h index 0acbaec3a..edd2694ac 100644 --- a/Eigen/src/Core/arch/RVV10/PacketMath.h +++ b/Eigen/src/Core/arch/RVV10/PacketMath.h @@ -642,22 +642,16 @@ EIGEN_STRONG_INLINE float predux_mul(const Packet1Xf& a) { return pfirst(prod); } +// Reusing the first lane is exact for an idempotent reduction and avoids a NaN seed that becomes poison under +// finite fast-math. template <> EIGEN_STRONG_INLINE float predux_min(const Packet1Xf& a) { - return (std::min)( - __riscv_vfmv_f(__riscv_vfredmin_vs_f32m1_f32m1( - a, __riscv_vfmv_v_f_f32m1((std::numeric_limits::quiet_NaN)(), unpacket_traits::size), - unpacket_traits::size)), - (std::numeric_limits::max)()); + return __riscv_vfmv_f(__riscv_vfredmin_vs_f32m1_f32m1(a, a, unpacket_traits::size)); } template <> EIGEN_STRONG_INLINE float predux_max(const Packet1Xf& a) { - return (std::max)( - __riscv_vfmv_f(__riscv_vfredmax_vs_f32m1_f32m1( - a, __riscv_vfmv_v_f_f32m1((std::numeric_limits::quiet_NaN)(), unpacket_traits::size), - unpacket_traits::size)), - -(std::numeric_limits::max)()); + return __riscv_vfmv_f(__riscv_vfredmax_vs_f32m1_f32m1(a, a, unpacket_traits::size)); } template @@ -1340,20 +1334,12 @@ EIGEN_STRONG_INLINE double predux_mul(const Packet1Xd& a) { template <> EIGEN_STRONG_INLINE double predux_min(const Packet1Xd& a) { - return (std::min)( - __riscv_vfmv_f(__riscv_vfredmin_vs_f64m1_f64m1( - a, __riscv_vfmv_v_f_f64m1((std::numeric_limits::quiet_NaN)(), unpacket_traits::size), - unpacket_traits::size)), - (std::numeric_limits::max)()); + return __riscv_vfmv_f(__riscv_vfredmin_vs_f64m1_f64m1(a, a, unpacket_traits::size)); } template <> EIGEN_STRONG_INLINE double predux_max(const Packet1Xd& a) { - return (std::max)( - __riscv_vfmv_f(__riscv_vfredmax_vs_f64m1_f64m1( - a, __riscv_vfmv_v_f_f64m1((std::numeric_limits::quiet_NaN)(), unpacket_traits::size), - unpacket_traits::size)), - -(std::numeric_limits::max)()); + return __riscv_vfmv_f(__riscv_vfredmax_vs_f64m1_f64m1(a, a, unpacket_traits::size)); } template diff --git a/Eigen/src/Core/arch/RVV10/PacketMath2.h b/Eigen/src/Core/arch/RVV10/PacketMath2.h index 2f8a436c3..59312dcb6 100644 --- a/Eigen/src/Core/arch/RVV10/PacketMath2.h +++ b/Eigen/src/Core/arch/RVV10/PacketMath2.h @@ -616,22 +616,18 @@ EIGEN_STRONG_INLINE float predux_mul(const Packet2Xf& a) { __riscv_vget_v_f32m2_f32m1(a, 0), __riscv_vget_v_f32m2_f32m1(a, 1), unpacket_traits::size)); } +// Reusing the first lane is exact for an idempotent reduction and avoids a NaN seed that becomes poison under +// finite fast-math. template <> EIGEN_STRONG_INLINE float predux_min(const Packet2Xf& a) { - return (std::min)( - __riscv_vfmv_f(__riscv_vfredmin_vs_f32m2_f32m1( - a, __riscv_vfmv_v_f_f32m1((std::numeric_limits::quiet_NaN)(), unpacket_traits::size / 2), - unpacket_traits::size)), - (std::numeric_limits::max)()); + return __riscv_vfmv_f( + __riscv_vfredmin_vs_f32m2_f32m1(a, __riscv_vget_v_f32m2_f32m1(a, 0), unpacket_traits::size)); } template <> EIGEN_STRONG_INLINE float predux_max(const Packet2Xf& a) { - return (std::max)( - __riscv_vfmv_f(__riscv_vfredmax_vs_f32m2_f32m1( - a, __riscv_vfmv_v_f_f32m1((std::numeric_limits::quiet_NaN)(), unpacket_traits::size / 2), - unpacket_traits::size)), - -(std::numeric_limits::max)()); + return __riscv_vfmv_f( + __riscv_vfredmax_vs_f32m2_f32m1(a, __riscv_vget_v_f32m2_f32m1(a, 0), unpacket_traits::size)); } template <> @@ -1266,20 +1262,14 @@ EIGEN_STRONG_INLINE double predux_mul(const Packet2Xd& a) { template <> EIGEN_STRONG_INLINE double predux_min(const Packet2Xd& a) { - return (std::min)( - __riscv_vfmv_f(__riscv_vfredmin_vs_f64m2_f64m1( - a, __riscv_vfmv_v_f_f64m1((std::numeric_limits::quiet_NaN)(), unpacket_traits::size / 2), - unpacket_traits::size)), - (std::numeric_limits::max)()); + return __riscv_vfmv_f( + __riscv_vfredmin_vs_f64m2_f64m1(a, __riscv_vget_v_f64m2_f64m1(a, 0), unpacket_traits::size)); } template <> EIGEN_STRONG_INLINE double predux_max(const Packet2Xd& a) { - return (std::max)( - __riscv_vfmv_f(__riscv_vfredmax_vs_f64m2_f64m1( - a, __riscv_vfmv_v_f_f64m1((std::numeric_limits::quiet_NaN)(), unpacket_traits::size / 2), - unpacket_traits::size)), - -(std::numeric_limits::max)()); + return __riscv_vfmv_f( + __riscv_vfredmax_vs_f64m2_f64m1(a, __riscv_vget_v_f64m2_f64m1(a, 0), unpacket_traits::size)); } #if __riscv_v_min_vlen >= 256 diff --git a/Eigen/src/Core/arch/RVV10/PacketMath4.h b/Eigen/src/Core/arch/RVV10/PacketMath4.h index adf834a69..8064a6c8a 100644 --- a/Eigen/src/Core/arch/RVV10/PacketMath4.h +++ b/Eigen/src/Core/arch/RVV10/PacketMath4.h @@ -619,22 +619,18 @@ EIGEN_STRONG_INLINE float predux_mul(const Packet4Xf& a) { return predux_mul(__riscv_vfmul_vv_f32m1(half1, half2, unpacket_traits::size)); } +// Reusing the first lane is exact for an idempotent reduction and avoids a NaN seed that becomes poison under +// finite fast-math. template <> EIGEN_STRONG_INLINE float predux_min(const Packet4Xf& a) { - return (std::min)( - __riscv_vfmv_f(__riscv_vfredmin_vs_f32m4_f32m1( - a, __riscv_vfmv_v_f_f32m1((std::numeric_limits::quiet_NaN)(), unpacket_traits::size / 4), - unpacket_traits::size)), - (std::numeric_limits::max)()); + return __riscv_vfmv_f( + __riscv_vfredmin_vs_f32m4_f32m1(a, __riscv_vget_v_f32m4_f32m1(a, 0), unpacket_traits::size)); } template <> EIGEN_STRONG_INLINE float predux_max(const Packet4Xf& a) { - return (std::max)( - __riscv_vfmv_f(__riscv_vfredmax_vs_f32m4_f32m1( - a, __riscv_vfmv_v_f_f32m1((std::numeric_limits::quiet_NaN)(), unpacket_traits::size / 4), - unpacket_traits::size)), - -(std::numeric_limits::max)()); + return __riscv_vfmv_f( + __riscv_vfredmax_vs_f32m4_f32m1(a, __riscv_vget_v_f32m4_f32m1(a, 0), unpacket_traits::size)); } template <> @@ -1271,20 +1267,14 @@ EIGEN_STRONG_INLINE double predux_mul(const Packet4Xd& a) { template <> EIGEN_STRONG_INLINE double predux_min(const Packet4Xd& a) { - return (std::min)( - __riscv_vfmv_f(__riscv_vfredmin_vs_f64m4_f64m1( - a, __riscv_vfmv_v_f_f64m1((std::numeric_limits::quiet_NaN)(), unpacket_traits::size / 4), - unpacket_traits::size)), - (std::numeric_limits::max)()); + return __riscv_vfmv_f( + __riscv_vfredmin_vs_f64m4_f64m1(a, __riscv_vget_v_f64m4_f64m1(a, 0), unpacket_traits::size)); } template <> EIGEN_STRONG_INLINE double predux_max(const Packet4Xd& a) { - return (std::max)( - __riscv_vfmv_f(__riscv_vfredmax_vs_f64m4_f64m1( - a, __riscv_vfmv_v_f_f64m1((std::numeric_limits::quiet_NaN)(), unpacket_traits::size / 4), - unpacket_traits::size)), - -(std::numeric_limits::max)()); + return __riscv_vfmv_f( + __riscv_vfredmax_vs_f64m4_f64m1(a, __riscv_vget_v_f64m4_f64m1(a, 0), unpacket_traits::size)); } template <> diff --git a/Eigen/src/Core/arch/RVV10/PacketMathFP16.h b/Eigen/src/Core/arch/RVV10/PacketMathFP16.h index fe7d60220..5e3851789 100644 --- a/Eigen/src/Core/arch/RVV10/PacketMathFP16.h +++ b/Eigen/src/Core/arch/RVV10/PacketMathFP16.h @@ -421,24 +421,18 @@ EIGEN_STRONG_INLINE Eigen::half predux_mul(const Packet1Xh& a) { return pfirst(prod); } +// Reusing the first lane is exact for an idempotent reduction and avoids a NaN seed that becomes poison under +// finite fast-math. template <> EIGEN_STRONG_INLINE Eigen::half predux_min(const Packet1Xh& a) { - const Eigen::half max = (std::numeric_limits::max)(); - const Eigen::half nan = (std::numeric_limits::quiet_NaN)(); - return (std::min)(static_cast(__riscv_vfmv_f(__riscv_vfredmin_vs_f16m1_f16m1( - a, __riscv_vfmv_v_f_f16m1(numext::bit_cast<_Float16>(nan), unpacket_traits::size), - unpacket_traits::size))), - max); + return static_cast( + __riscv_vfmv_f(__riscv_vfredmin_vs_f16m1_f16m1(a, a, unpacket_traits::size))); } template <> EIGEN_STRONG_INLINE Eigen::half predux_max(const Packet1Xh& a) { - const Eigen::half min = -(std::numeric_limits::max)(); - const Eigen::half nan = (std::numeric_limits::quiet_NaN)(); - return (std::max)(static_cast(__riscv_vfmv_f(__riscv_vfredmax_vs_f16m1_f16m1( - a, __riscv_vfmv_v_f_f16m1(numext::bit_cast<_Float16>(nan), unpacket_traits::size), - unpacket_traits::size))), - min); + return static_cast( + __riscv_vfmv_f(__riscv_vfredmax_vs_f16m1_f16m1(a, a, unpacket_traits::size))); } template @@ -788,24 +782,14 @@ EIGEN_STRONG_INLINE Eigen::half predux_mul(const Packet2Xh& a) { template <> EIGEN_STRONG_INLINE Eigen::half predux_min(const Packet2Xh& a) { - const Eigen::half max = (std::numeric_limits::max)(); - const Eigen::half nan = (std::numeric_limits::quiet_NaN)(); - return (std::min)( - static_cast(__riscv_vfmv_f(__riscv_vfredmin_vs_f16m2_f16m1( - a, __riscv_vfmv_v_f_f16m1(numext::bit_cast<_Float16>(nan), unpacket_traits::size / 2), - unpacket_traits::size))), - max); + return static_cast(__riscv_vfmv_f( + __riscv_vfredmin_vs_f16m2_f16m1(a, __riscv_vget_v_f16m2_f16m1(a, 0), unpacket_traits::size))); } template <> EIGEN_STRONG_INLINE Eigen::half predux_max(const Packet2Xh& a) { - const Eigen::half min = -(std::numeric_limits::max)(); - const Eigen::half nan = (std::numeric_limits::quiet_NaN)(); - return (std::max)( - static_cast(__riscv_vfmv_f(__riscv_vfredmax_vs_f16m2_f16m1( - a, __riscv_vfmv_v_f_f16m1(numext::bit_cast<_Float16>(nan), unpacket_traits::size / 2), - unpacket_traits::size))), - min); + return static_cast(__riscv_vfmv_f( + __riscv_vfredmax_vs_f16m2_f16m1(a, __riscv_vget_v_f16m2_f16m1(a, 0), unpacket_traits::size))); } template diff --git a/test/packetmath.cpp b/test/packetmath.cpp index 38791d828..65ef80bb3 100644 --- a/test/packetmath.cpp +++ b/test/packetmath.cpp @@ -1686,6 +1686,18 @@ std::enable_if_t::IsComplex, void> packetmath_ieee_special_va run_ieee_cases(atanh_fun()); } +template +void packetmath_redux_infinities() { + const Scalar infinity = NumTraits::infinity(); + const Packet positive_infinity = internal::pset1(infinity); + VERIFY_IS_EQUAL(internal::predux_min(positive_infinity), infinity); + VERIFY_IS_EQUAL(internal::predux_max(positive_infinity), infinity); + + const Packet negative_infinity = internal::pset1(-infinity); + VERIFY_IS_EQUAL(internal::predux_min(negative_infinity), -infinity); + VERIFY_IS_EQUAL(internal::predux_max(negative_infinity), -infinity); +} + template void packetmath_notcomplex() { packetmath_ieee_special_values(); @@ -2259,4 +2271,18 @@ EIGEN_DECLARE_TEST(packetmath) { CALL_SUBTEST_15(test::runner::run()); g_first_pass = false; } + +#if defined(EIGEN_VECTORIZE_RVV10) + CALL_SUBTEST_1((packetmath_redux_infinities())); + CALL_SUBTEST_1((packetmath_redux_infinities())); + CALL_SUBTEST_1((packetmath_redux_infinities())); + CALL_SUBTEST_2((packetmath_redux_infinities())); + CALL_SUBTEST_2((packetmath_redux_infinities())); + CALL_SUBTEST_2((packetmath_redux_infinities())); +#endif + +#if defined(EIGEN_VECTORIZE_RVV10FP16) + CALL_SUBTEST_13((packetmath_redux_infinities())); + CALL_SUBTEST_13((packetmath_redux_infinities())); +#endif } diff --git a/test/packetmath_fastmath.cpp b/test/packetmath_fastmath.cpp index 26aba463a..94d0ed64d 100644 --- a/test/packetmath_fastmath.cpp +++ b/test/packetmath_fastmath.cpp @@ -26,6 +26,28 @@ EIGEN_DONT_INLINE bool mask_any(const Scalar* mask) { return Eigen::internal::predux_any(Eigen::internal::ploadu(mask)); } +// Keep the finite inputs opaque while compiling the reduction itself with fast-math optimizations. +template +EIGEN_DONT_INLINE void reduce_minmax(const Scalar* input, Scalar* output) { + const Packet packet = Eigen::internal::ploadu(input); + output[0] = Eigen::internal::predux_min(packet); + output[1] = Eigen::internal::predux_max(packet); +} + +template +void verify_minmax_reduction() { + constexpr int packet_size = Eigen::internal::unpacket_traits::size; + Scalar input[packet_size]; + for (int i = 0; i < packet_size; ++i) { + input[i] = Scalar(i + 1); + } + + Scalar output[2]; + reduce_minmax(input, output); + VERIFY_IS_EQUAL(output[0], Scalar(1)); + VERIFY_IS_EQUAL(output[1], Scalar(packet_size)); +} + // Complementary to the opaque-mask calls: the all-ones mask flows straight // from ptrue into the packet op, the way comparison-mask producers feed it, // which gives constant folding under -ffinite-math-only a chance to see the @@ -126,6 +148,8 @@ struct packetmath_fastmath_runner { } VERIFY((mask_any(mask))); } + + verify_minmax_reduction(); } }; @@ -160,4 +184,18 @@ EIGEN_DECLARE_TEST(packetmath_fastmath) { CALL_SUBTEST(packetmath_fastmath_runner::run()); CALL_SUBTEST(packetmath_fastmath_runner::run()); CALL_SUBTEST(extended_scalar_constant_runner::run()); + +#if defined(EIGEN_VECTORIZE_RVV10) + CALL_SUBTEST((verify_minmax_reduction())); + CALL_SUBTEST((verify_minmax_reduction())); + CALL_SUBTEST((verify_minmax_reduction())); + CALL_SUBTEST((verify_minmax_reduction())); + CALL_SUBTEST((verify_minmax_reduction())); + CALL_SUBTEST((verify_minmax_reduction())); +#endif + +#if defined(EIGEN_VECTORIZE_RVV10FP16) + CALL_SUBTEST((verify_minmax_reduction())); + CALL_SUBTEST((verify_minmax_reduction())); +#endif }