From 38abf2be4289a8da5db2d5b1db759f26800ae1d3 Mon Sep 17 00:00:00 2001 From: Antonio Sanchez Date: Mon, 23 Nov 2020 14:13:59 -0800 Subject: [PATCH] Fix Half NaN definition and test. The `half_float` test was failing with `-mcpu=cortex-a55` (native `__fp16`) due to a bad NaN bit-pattern comparison (in the case of casting a float to `__fp16`, the signaling `NaN` is quieted). There was also an inconsistency between `numeric_limits::quiet_NaN()` and `NumTraits::quiet_NaN()`. Here we correct the inconsistency and compare NaNs according to the IEEE 754 definition. Also modified the `bfloat16_float` test to match. Tested with `cortex-a53` and `cortex-a55`. --- Eigen/src/Core/arch/Default/Half.h | 4 ++-- test/bfloat16_float.cpp | 20 +++++++++++++++++--- test/half_float.cpp | 22 ++++++++++++++++------ 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/Eigen/src/Core/arch/Default/Half.h b/Eigen/src/Core/arch/Default/Half.h index 4dde91365..d6ddff59c 100644 --- a/Eigen/src/Core/arch/Default/Half.h +++ b/Eigen/src/Core/arch/Default/Half.h @@ -200,7 +200,7 @@ struct numeric_limits { static Eigen::half round_error() { return Eigen::half(0.5); } static Eigen::half infinity() { return Eigen::half_impl::raw_uint16_to_half(0x7c00); } static Eigen::half quiet_NaN() { return Eigen::half_impl::raw_uint16_to_half(0x7e00); } - static Eigen::half signaling_NaN() { return Eigen::half_impl::raw_uint16_to_half(0x7e00); } + static Eigen::half signaling_NaN() { return Eigen::half_impl::raw_uint16_to_half(0x7d00); } static Eigen::half denorm_min() { return Eigen::half_impl::raw_uint16_to_half(0x1); } }; @@ -744,7 +744,7 @@ template<> struct NumTraits return half_impl::raw_uint16_to_half(0x7c00); } EIGEN_DEVICE_FUNC EIGEN_CONSTEXPR static EIGEN_STRONG_INLINE Eigen::half quiet_NaN() { - return half_impl::raw_uint16_to_half(0x7c01); + return half_impl::raw_uint16_to_half(0x7e00); } }; diff --git a/test/bfloat16_float.cpp b/test/bfloat16_float.cpp index fc648dfec..1df22f73e 100644 --- a/test/bfloat16_float.cpp +++ b/test/bfloat16_float.cpp @@ -274,9 +274,23 @@ void test_numtraits() VERIFY_IS_EQUAL( numext::bit_cast(std::numeric_limits::infinity()), numext::bit_cast(bfloat16(std::numeric_limits::infinity())) ); - VERIFY_IS_EQUAL( - numext::bit_cast(std::numeric_limits::quiet_NaN()), - numext::bit_cast(bfloat16(std::numeric_limits::quiet_NaN())) ); + // There is no guarantee that casting a 32-bit NaN to bfloat16 has a precise + // bit pattern. We test that it is in fact a NaN, then test the signaling + // bit (msb of significand is 1 for quiet, 0 for signaling). + const numext::uint16_t BFLOAT16_QUIET_BIT = 0x0040; + VERIFY( + (numext::isnan)(std::numeric_limits::quiet_NaN()) + && (numext::isnan)(bfloat16(std::numeric_limits::quiet_NaN())) + && ((numext::bit_cast(std::numeric_limits::quiet_NaN()) & BFLOAT16_QUIET_BIT) > 0) + && ((numext::bit_cast(bfloat16(std::numeric_limits::quiet_NaN())) & BFLOAT16_QUIET_BIT) > 0) ); + // After a cast to bfloat16, a signaling NaN may become non-signaling. Thus, + // we check that both are NaN, and that only the `numeric_limits` version is + // signaling. + VERIFY( + (numext::isnan)(std::numeric_limits::signaling_NaN()) + && (numext::isnan)(bfloat16(std::numeric_limits::signaling_NaN())) + && ((numext::bit_cast(std::numeric_limits::signaling_NaN()) & BFLOAT16_QUIET_BIT) == 0) ); + VERIFY( (std::numeric_limits::min)() > bfloat16(0.f) ); VERIFY( (std::numeric_limits::denorm_min)() > bfloat16(0.f) ); VERIFY_IS_EQUAL( (std::numeric_limits::denorm_min)()/bfloat16(2), bfloat16(0.f) ); diff --git a/test/half_float.cpp b/test/half_float.cpp index cf6df547a..b2c22197f 100644 --- a/test/half_float.cpp +++ b/test/half_float.cpp @@ -136,12 +136,22 @@ void test_numtraits() VERIFY_IS_EQUAL( numext::bit_cast(std::numeric_limits::infinity()), numext::bit_cast(half(std::numeric_limits::infinity())) ); - VERIFY_IS_EQUAL( - numext::bit_cast(std::numeric_limits::quiet_NaN()), - numext::bit_cast(half(std::numeric_limits::quiet_NaN())) ); - VERIFY_IS_EQUAL( - numext::bit_cast(std::numeric_limits::signaling_NaN()), - numext::bit_cast(half(std::numeric_limits::signaling_NaN())) ); + // There is no guarantee that casting a 32-bit NaN to 16-bit has a precise + // bit pattern. We test that it is in fact a NaN, then test the signaling + // bit (msb of significand is 1 for quiet, 0 for signaling). + const numext::uint16_t HALF_QUIET_BIT = 0x0200; + VERIFY( + (numext::isnan)(std::numeric_limits::quiet_NaN()) + && (numext::isnan)(half(std::numeric_limits::quiet_NaN())) + && ((numext::bit_cast(std::numeric_limits::quiet_NaN()) & HALF_QUIET_BIT) > 0) + && ((numext::bit_cast(half(std::numeric_limits::quiet_NaN())) & HALF_QUIET_BIT) > 0) ); + // After a cast to half, a signaling NaN may become non-signaling + // (e.g. in the case of casting float to native __fp16). Thus, we check that + // both are NaN, and that only the `numeric_limits` version is signaling. + VERIFY( + (numext::isnan)(std::numeric_limits::signaling_NaN()) + && (numext::isnan)(half(std::numeric_limits::signaling_NaN())) + && ((numext::bit_cast(std::numeric_limits::signaling_NaN()) & HALF_QUIET_BIT) == 0) ); VERIFY( (std::numeric_limits::min)() > half(0.f) ); VERIFY( (std::numeric_limits::denorm_min)() > half(0.f) );