diff --git a/Eigen/src/Core/GenericPacketMath.h b/Eigen/src/Core/GenericPacketMath.h index 99b73b917..b8b249f4a 100644 --- a/Eigen/src/Core/GenericPacketMath.h +++ b/Eigen/src/Core/GenericPacketMath.h @@ -1179,14 +1179,23 @@ EIGEN_DEVICE_FUNC inline Packet pdupimag(const Packet& a) { // fast-math flags clang turns into a poison constant that deletes any expression it flows // into. template ::type>::IsComplex, - bool IsScalar = is_scalar::value> + bool IsScalar = is_scalar::value, + bool IsInteger = NumTraits::type>::IsInteger> struct pisnan_impl { // Equivalent to !(a == a). static EIGEN_DEVICE_FUNC inline Packet run(const Packet& a) { return pcmp_lt_or_nan(a, a); } }; -template -struct pisnan_impl { +// Integer scalars have no NaN; the answer is the all-false mask. The generic path is unusable +// here: pcmp_lt_or_nan has no meaningful integer semantics (and its generic form does not even +// compile for integer SIMD packets). +template +struct pisnan_impl { + static EIGEN_DEVICE_FUNC inline Packet run(const Packet& a) { return pzero(a); } +}; + +template +struct pisnan_impl { static EIGEN_DEVICE_FUNC inline Packet run(const Packet& a) { using RealPacket = typename unpacket_traits::as_real; // A NaN in either the real or the imaginary lane marks the whole complex element. @@ -1197,8 +1206,8 @@ struct pisnan_impl { // Scalar complex arguments have no wrapped real packet; combine the per-component results in the // value domain, where the scalar mask convention is Scalar(1)/Scalar(0). -template -struct pisnan_impl { +template +struct pisnan_impl { static EIGEN_DEVICE_FUNC inline Scalar run(const Scalar& a) { using RealScalar = typename NumTraits::Real; const RealScalar nan_mask = @@ -1213,10 +1222,42 @@ EIGEN_DEVICE_FUNC inline Packet pisnan(const Packet& a) { return pisnan_impl::run(a); } +template ::type>::IsInteger> +struct pisinf_impl { + static EIGEN_DEVICE_FUNC inline Packet run(const Packet& a) { return pcmp_eq(pabs(a), pinf()); } +}; + +// Integer scalars have no infinity; the answer is the all-false mask. The generic path is wrong +// for them: pinf() synthesizes its bit pattern from numeric_limits digits, which for int32 yields +// 2^30, so |a| == 2^30 would read as "inf". +template +struct pisinf_impl { + static EIGEN_DEVICE_FUNC inline Packet run(const Packet& a) { return pzero(a); } +}; + /** \internal \returns isinf(a) */ template EIGEN_DEVICE_FUNC inline Packet pisinf(const Packet& a) { - return pcmp_eq(pabs(a), pinf()); + return pisinf_impl::run(a); +} + +template ::type>::IsInteger> +struct pisfinite_impl { + // |a| < inf is a single comparison that is false for both NaN and infinities. + static EIGEN_DEVICE_FUNC inline Packet run(const Packet& a) { return pcmp_lt(pabs(a), pinf()); } +}; + +// Integer scalars are always finite; the answer is the all-true mask (safe for integer packets, +// where all-ones is not a NaN bit pattern). +template +struct pisfinite_impl { + static EIGEN_DEVICE_FUNC inline Packet run(const Packet& a) { return ptrue(a); } +}; + +/** \internal \returns isfinite(a) */ +template +EIGEN_DEVICE_FUNC inline Packet pisfinite(const Packet& a) { + return pisfinite_impl::run(a); } /** \internal \returns the sine of \a a (coeff-wise) */ diff --git a/Eigen/src/Core/functors/UnaryFunctors.h b/Eigen/src/Core/functors/UnaryFunctors.h index 6da101db2..fc2220b86 100644 --- a/Eigen/src/Core/functors/UnaryFunctors.h +++ b/Eigen/src/Core/functors/UnaryFunctors.h @@ -183,7 +183,11 @@ struct scalar_carg_op { template struct functor_traits> { using RealScalar = typename NumTraits::Real; - enum { Cost = functor_traits>::Cost, PacketAccess = packet_traits::HasATan }; + enum { + Cost = functor_traits>::Cost, + // The generic pcarg lowers to patan2, whose quotient-based reduction needs pdiv. + PacketAccess = packet_traits::HasATan && packet_traits::HasDiv + }; }; /** \internal @@ -1130,7 +1134,7 @@ struct scalar_isfinite_op { } template EIGEN_DEVICE_FUNC inline Packet packetOp(const Packet& a) const { - return pcmp_lt(pabs(a), pinf()); + return pisfinite(a); } }; template @@ -1359,7 +1363,9 @@ struct functor_traits> { Cost = scalar_div_cost::HasDiv>::value + (std::is_same::value ? NumTraits::AddCost * 15 + NumTraits::MulCost * 11 : NumTraits::AddCost * 2 + functor_traits>::Cost), + // Both packet paths branch with pcmp_*/pselect. PacketAccess = !NumTraits::IsComplex && packet_traits::HasAdd && packet_traits::HasDiv && + packet_traits::HasCmp && (std::is_same::value ? packet_traits::HasMul && packet_traits::HasMax && packet_traits::HasMin : packet_traits::HasNegate && packet_traits::HasExp) diff --git a/test/array_cwise.cpp b/test/array_cwise.cpp index 3d74a46ee..2371948ba 100644 --- a/test/array_cwise.cpp +++ b/test/array_cwise.cpp @@ -1350,6 +1350,30 @@ void typed_logicals_test(const ArrayType& m) { typed_logicals_test_impl::run(m); } +// Integer scalars are always finite, so isFiniteTyped() must be true (nonzero) everywhere. Only +// truthiness is checked: the exact nonzero value differs between the vectorized path (all-ones +// mask) and the scalar path (1). Regression: the vectorized path used to compare |x| against a +// synthesized "infinity" bit pattern, misclassifying |x| >= 2^(digits-1) for signed types and +// everything for unsigned ones. +template +void integer_typed_predicates_test(const ArrayType& m) { + typedef typename ArrayType::Scalar Scalar; + Index rows = m.rows(); + Index cols = m.cols(); + const Scalar values[] = {Scalar(0), + Scalar(1), + static_cast(-1), + Scalar(Scalar(1) << (std::numeric_limits::digits - 1)), + NumTraits::highest(), + NumTraits::lowest()}; + const Index num_values = sizeof(values) / sizeof(values[0]); + ArrayType m1(rows, cols); + for (Index i = 0; i < m1.size(); ++i) m1.coeffRef(i) = values[i % num_values]; + // Materialize so the vectorized assignment path engages. + ArrayType finite = m1.isFiniteTyped(); + VERIFY((finite != Scalar(0)).all()); +} + template struct cast_test_impl { using SrcArray = Array; @@ -1490,6 +1514,10 @@ EIGEN_DECLARE_TEST(array_cwise) { } for (int i = 0; i < g_repeat; i++) { CALL_SUBTEST_23(typed_logicals_test(ArrayX(internal::random(1, EIGEN_TEST_MAX_SIZE)))); + CALL_SUBTEST_23(integer_typed_predicates_test(ArrayX(internal::random(1, EIGEN_TEST_MAX_SIZE)))); + CALL_SUBTEST_23(integer_typed_predicates_test(ArrayXX(internal::random(1, EIGEN_TEST_MAX_SIZE), + internal::random(1, EIGEN_TEST_MAX_SIZE)))); + CALL_SUBTEST_23(integer_typed_predicates_test(ArrayX(internal::random(1, EIGEN_TEST_MAX_SIZE)))); CALL_SUBTEST_24(typed_logicals_test(ArrayX(internal::random(1, EIGEN_TEST_MAX_SIZE)))); CALL_SUBTEST_25(typed_logicals_test(ArrayX(internal::random(1, EIGEN_TEST_MAX_SIZE)))); CALL_SUBTEST_26(typed_logicals_test(ArrayX>(internal::random(1, EIGEN_TEST_MAX_SIZE)))); diff --git a/test/packetmath.cpp b/test/packetmath.cpp index 700dbb45c..8a2370330 100644 --- a/test/packetmath.cpp +++ b/test/packetmath.cpp @@ -520,6 +520,49 @@ struct packetmath_minus_zero_add_test +struct packetmath_integer_predicates_test { + static void run() {} +}; + +// Integer scalars have no NaN or infinity: pisnan/pisinf must be all-false and pisfinite +// all-true for every input, including |a| == 2^(digits-1), whose bit pattern matches the +// constant synthesized by pinf(). +template +struct packetmath_integer_predicates_test< + Scalar, Packet, std::enable_if_t::IsInteger && !std::is_same::value>> { + static void run() { + const int PacketSize = internal::unpacket_traits::size; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar data[PacketSize]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar res[PacketSize]; + // "True" is Scalar(1) in the scalar mask convention and all-ones bits in the packet one; + // ptrue of the tested Packet type yields the right one either way (the runner also + // instantiates Packet = Scalar). + const Scalar scalar_true = internal::ptrue(Scalar(0)); + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar lane_true[PacketSize]; + internal::pstore(lane_true, internal::ptrue(internal::pset1(Scalar(0)))); + const Scalar values[] = {Scalar(0), + Scalar(1), + static_cast(-1), + Scalar(Scalar(1) << (std::numeric_limits::digits - 1)), + NumTraits::highest(), + NumTraits::lowest()}; + const int num_values = sizeof(values) / sizeof(values[0]); + for (int i = 0; i < num_values; ++i) { + VERIFY(numext::is_exactly_zero(internal::pisnan(values[i])) && "scalar integer pisnan"); + VERIFY(numext::is_exactly_zero(internal::pisinf(values[i])) && "scalar integer pisinf"); + VERIFY(internal::pisfinite(values[i]) == scalar_true && "scalar integer pisfinite"); + } + for (int i = 0; i < PacketSize; ++i) data[i] = values[i % num_values]; + internal::pstore(res, internal::pisnan(internal::pload(data))); + for (int i = 0; i < PacketSize; ++i) VERIFY(numext::is_exactly_zero(res[i]) && "integer pisnan"); + internal::pstore(res, internal::pisinf(internal::pload(data))); + for (int i = 0; i < PacketSize; ++i) VERIFY(numext::is_exactly_zero(res[i]) && "integer pisinf"); + internal::pstore(res, internal::pisfinite(internal::pload(data))); + for (int i = 0; i < PacketSize; ++i) VERIFY(res[i] == lane_true[i] && "integer pisfinite"); + } +}; + // Ensure optimization barrier compiles and doesn't modify contents. // Only applies to raw types, so will not work for std::complex, Eigen::half // or Eigen::bfloat16. For those you would need to refer to an underlying @@ -827,6 +870,7 @@ void packetmath() { packetmath_boolean_mask_ops(); packetmath_pcast_ops_runner::run(); packetmath_minus_zero_add_test::run(); + packetmath_integer_predicates_test::run(); CHECK_CWISE3_IF(true, REF_MADD, internal::pmadd); if (!std::is_same::value && NumTraits::IsSigned) { diff --git a/unsupported/test/tensor_comparisons.cpp b/unsupported/test/tensor_comparisons.cpp index b7b3a8766..bf145c35a 100644 --- a/unsupported/test/tensor_comparisons.cpp +++ b/unsupported/test/tensor_comparisons.cpp @@ -184,10 +184,27 @@ static void test_isfinite() { } } +// Integer scalars have no NaN or infinity; every value must classify as finite, including +// |x| >= 2^30, whose bit pattern the vectorized predicates used to misread as infinity. +static void test_predicates_integer() { + Tensor mat(8); + mat.setValues({0, 1, -1, 1 << 30, -(1 << 30), 2147483647, -2147483647 - 1, 42}); + Tensor nan(8), inf(8), finite(8); + nan = (mat.isnan)(); + inf = (mat.isinf)(); + finite = (mat.isfinite)(); + for (int i = 0; i < 8; ++i) { + VERIFY(!nan(i)); + VERIFY(!inf(i)); + VERIFY(finite(i)); + } +} + EIGEN_DECLARE_TEST(tensor_comparisons) { CALL_SUBTEST(test_orderings()); CALL_SUBTEST(test_equality()); CALL_SUBTEST(test_isnan()); CALL_SUBTEST(test_isinf()); CALL_SUBTEST(test_isfinite()); + CALL_SUBTEST(test_predicates_integer()); }