From 70b1ce11c6b32cfe4ed1cedcef5e96a43659d15a Mon Sep 17 00:00:00 2001 From: Gael Guennebaud Date: Mon, 19 Jul 2010 23:31:08 +0200 Subject: [PATCH] * fix SelfCwiseBinaryOp traits and handling of mixed types * improve compilation error in case of type mismatch --- Eigen/src/Core/ArrayBase.h | 8 +++--- Eigen/src/Core/Assign.h | 19 +++++++++++-- Eigen/src/Core/CwiseBinaryOp.h | 32 +++++++++++---------- Eigen/src/Core/SelfCwiseBinaryOp.h | 34 ++++++++++++----------- Eigen/src/Core/Transpose.h | 6 ++-- Eigen/src/Core/util/BlasUtil.h | 2 +- Eigen/src/Core/util/Constants.h | 4 ++- Eigen/src/Core/util/ForwardDeclarations.h | 2 +- 8 files changed, 64 insertions(+), 43 deletions(-) diff --git a/Eigen/src/Core/ArrayBase.h b/Eigen/src/Core/ArrayBase.h index 1a54b2335..941e39938 100644 --- a/Eigen/src/Core/ArrayBase.h +++ b/Eigen/src/Core/ArrayBase.h @@ -178,7 +178,7 @@ template EIGEN_STRONG_INLINE Derived & ArrayBase::operator-=(const ArrayBase &other) { - SelfCwiseBinaryOp, Derived> tmp(derived()); + SelfCwiseBinaryOp, Derived, OtherDerived> tmp(derived()); tmp = other; return derived(); } @@ -192,7 +192,7 @@ template EIGEN_STRONG_INLINE Derived & ArrayBase::operator+=(const ArrayBase& other) { - SelfCwiseBinaryOp, Derived> tmp(derived()); + SelfCwiseBinaryOp, Derived, OtherDerived> tmp(derived()); tmp = other.derived(); return derived(); } @@ -206,7 +206,7 @@ template EIGEN_STRONG_INLINE Derived & ArrayBase::operator*=(const ArrayBase& other) { - SelfCwiseBinaryOp, Derived> tmp(derived()); + SelfCwiseBinaryOp, Derived, OtherDerived> tmp(derived()); tmp = other.derived(); return derived(); } @@ -220,7 +220,7 @@ template EIGEN_STRONG_INLINE Derived & ArrayBase::operator/=(const ArrayBase& other) { - SelfCwiseBinaryOp, Derived> tmp(derived()); + SelfCwiseBinaryOp, Derived, OtherDerived> tmp(derived()); tmp = other.derived(); return derived(); } diff --git a/Eigen/src/Core/Assign.h b/Eigen/src/Core/Assign.h index 6b114ac30..335a888f6 100644 --- a/Eigen/src/Core/Assign.h +++ b/Eigen/src/Core/Assign.h @@ -256,6 +256,12 @@ struct ei_assign_impl; *** Default traversal *** ************************/ +template +struct ei_assign_impl +{ + inline static void run(Derived1 &, const Derived2 &) { } +}; + template struct ei_assign_impl { @@ -486,14 +492,21 @@ template EIGEN_STRONG_INLINE Derived& DenseBase ::lazyAssign(const DenseBase& other) { + enum{ + SameType = ei_is_same_type::ret + }; + EIGEN_STATIC_ASSERT_SAME_MATRIX_SIZE(Derived,OtherDerived) - EIGEN_STATIC_ASSERT((ei_is_same_type::ret), - YOU_MIXED_DIFFERENT_NUMERIC_TYPES__YOU_NEED_TO_USE_THE_CAST_METHOD_OF_MATRIXBASE_TO_CAST_NUMERIC_TYPES_EXPLICITLY) + EIGEN_STATIC_ASSERT(SameType,YOU_MIXED_DIFFERENT_NUMERIC_TYPES__YOU_NEED_TO_USE_THE_CAST_METHOD_OF_MATRIXBASE_TO_CAST_NUMERIC_TYPES_EXPLICITLY) + + + #ifdef EIGEN_DEBUG_ASSIGN ei_assign_traits::debug(); #endif ei_assert(rows() == other.rows() && cols() == other.cols()); - ei_assign_impl::run(derived(),other.derived()); + ei_assign_impl::Traversal) + : int(InvalidTraversal)>::run(derived(),other.derived()); #ifndef EIGEN_NO_DEBUG checkTransposeAliasing(other.derived()); #endif diff --git a/Eigen/src/Core/CwiseBinaryOp.h b/Eigen/src/Core/CwiseBinaryOp.h index 7f4587b53..171140c27 100644 --- a/Eigen/src/Core/CwiseBinaryOp.h +++ b/Eigen/src/Core/CwiseBinaryOp.h @@ -80,13 +80,14 @@ struct ei_traits > RhsCoeffReadCost = _RhsNested::CoeffReadCost, LhsFlags = _LhsNested::Flags, RhsFlags = _RhsNested::Flags, + SameType = ei_is_same_type::ret, StorageOrdersAgree = (int(Lhs::Flags)&RowMajorBit)==(int(Rhs::Flags)&RowMajorBit), Flags0 = (int(LhsFlags) | int(RhsFlags)) & ( HereditaryBits | (int(LhsFlags) & int(RhsFlags) & ( AlignedBit | (StorageOrdersAgree ? LinearAccessBit : 0) - | (ei_functor_traits::PacketAccess && StorageOrdersAgree ? PacketAccessBit : 0) + | (ei_functor_traits::PacketAccess && StorageOrdersAgree && SameType ? PacketAccessBit : 0) ) ) ), @@ -95,6 +96,19 @@ struct ei_traits > }; }; +// we require Lhs and Rhs to have the same scalar type. Currently there is no example of a binary functor +// that would take two operands of different types. If there were such an example, then this check should be +// moved to the BinaryOp functors, on a per-case basis. This would however require a change in the BinaryOp functors, as +// currently they take only one typename Scalar template parameter. +// It is tempting to always allow mixing different types but remember that this is often impossible in the vectorized paths. +// So allowing mixing different types gives very unexpected errors when enabling vectorization, when the user tries to +// add together a float matrix and a double matrix. +#define EIGEN_CHECK_BINARY_COMPATIBILIY(BINOP,LHS,RHS) \ + EIGEN_STATIC_ASSERT((ei_functor_allows_mixing_real_and_complex::ret \ + ? int(ei_is_same_type::Real, typename NumTraits::Real>::ret) \ + : int(ei_is_same_type::ret)), \ + YOU_MIXED_DIFFERENT_NUMERIC_TYPES__YOU_NEED_TO_USE_THE_CAST_METHOD_OF_MATRIXBASE_TO_CAST_NUMERIC_TYPES_EXPLICITLY) + template class CwiseBinaryOpImpl; @@ -121,17 +135,7 @@ class CwiseBinaryOp : ei_no_assignment_operator, EIGEN_STRONG_INLINE CwiseBinaryOp(const Lhs& lhs, const Rhs& rhs, const BinaryOp& func = BinaryOp()) : m_lhs(lhs), m_rhs(rhs), m_functor(func) { - // we require Lhs and Rhs to have the same scalar type. Currently there is no example of a binary functor - // that would take two operands of different types. If there were such an example, then this check should be - // moved to the BinaryOp functors, on a per-case basis. This would however require a change in the BinaryOp functors, as - // currently they take only one typename Scalar template parameter. - // It is tempting to always allow mixing different types but remember that this is often impossible in the vectorized paths. - // So allowing mixing different types gives very unexpected errors when enabling vectorization, when the user tries to - // add together a float matrix and a double matrix. - EIGEN_STATIC_ASSERT((ei_functor_allows_mixing_real_and_complex::ret - ? int(ei_is_same_type::ret) - : int(ei_is_same_type::ret)), - YOU_MIXED_DIFFERENT_NUMERIC_TYPES__YOU_NEED_TO_USE_THE_CAST_METHOD_OF_MATRIXBASE_TO_CAST_NUMERIC_TYPES_EXPLICITLY) + EIGEN_CHECK_BINARY_COMPATIBILIY(BinaryOp,typename Lhs::Scalar,typename Rhs::Scalar); // require the sizes to match EIGEN_STATIC_ASSERT_SAME_MATRIX_SIZE(Lhs, Rhs) ei_assert(lhs.rows() == rhs.rows() && lhs.cols() == rhs.cols()); @@ -211,7 +215,7 @@ template EIGEN_STRONG_INLINE Derived & MatrixBase::operator-=(const MatrixBase &other) { - SelfCwiseBinaryOp, Derived> tmp(derived()); + SelfCwiseBinaryOp, Derived, OtherDerived> tmp(derived()); tmp = other; return derived(); } @@ -225,7 +229,7 @@ template EIGEN_STRONG_INLINE Derived & MatrixBase::operator+=(const MatrixBase& other) { - SelfCwiseBinaryOp, Derived> tmp(derived()); + SelfCwiseBinaryOp, Derived, OtherDerived> tmp(derived()); tmp = other.derived(); return derived(); } diff --git a/Eigen/src/Core/SelfCwiseBinaryOp.h b/Eigen/src/Core/SelfCwiseBinaryOp.h index acdbb8658..de8c4e740 100644 --- a/Eigen/src/Core/SelfCwiseBinaryOp.h +++ b/Eigen/src/Core/SelfCwiseBinaryOp.h @@ -39,14 +39,19 @@ * * \sa class SwapWrapper for a similar trick. */ -template -struct ei_traits > : ei_traits +template +struct ei_traits > + : ei_traits > { - + enum { + Flags = ei_traits >::Flags | (Lhs::Flags&DirectAccessBit), + OuterStrideAtCompileTime = Lhs::OuterStrideAtCompileTime, + InnerStrideAtCompileTime = Lhs::InnerStrideAtCompileTime + }; }; -template class SelfCwiseBinaryOp - : public ei_dense_xpr_base< SelfCwiseBinaryOp >::type +template class SelfCwiseBinaryOp + : public ei_dense_xpr_base< SelfCwiseBinaryOp >::type { public: @@ -57,7 +62,7 @@ template class SelfCwiseBinaryOp using Base::operator=; - inline SelfCwiseBinaryOp(MatrixType& xpr, const BinaryOp& func = BinaryOp()) : m_matrix(xpr), m_functor(func) {} + inline SelfCwiseBinaryOp(Lhs& xpr, const BinaryOp& func = BinaryOp()) : m_matrix(xpr), m_functor(func) {} inline Index rows() const { return m_matrix.rows(); } inline Index cols() const { return m_matrix.cols(); } @@ -122,12 +127,8 @@ template class SelfCwiseBinaryOp template EIGEN_STRONG_INLINE SelfCwiseBinaryOp& lazyAssign(const DenseBase& rhs) { - EIGEN_STATIC_ASSERT_SAME_MATRIX_SIZE(MatrixType,RhsDerived) - - EIGEN_STATIC_ASSERT((ei_functor_allows_mixing_real_and_complex::ret - ? int(ei_is_same_type::ret) - : int(ei_is_same_type::ret)), - YOU_MIXED_DIFFERENT_NUMERIC_TYPES__YOU_NEED_TO_USE_THE_CAST_METHOD_OF_MATRIXBASE_TO_CAST_NUMERIC_TYPES_EXPLICITLY) + EIGEN_STATIC_ASSERT_SAME_MATRIX_SIZE(Lhs,RhsDerived) + EIGEN_CHECK_BINARY_COMPATIBILIY(BinaryOp,typename Lhs::Scalar,typename RhsDerived::Scalar); #ifdef EIGEN_DEBUG_ASSIGN ei_assign_traits::debug(); @@ -141,7 +142,7 @@ template class SelfCwiseBinaryOp } protected: - MatrixType& m_matrix; + Lhs& m_matrix; const BinaryOp& m_functor; private: @@ -151,8 +152,8 @@ template class SelfCwiseBinaryOp template inline Derived& DenseBase::operator*=(const Scalar& other) { - SelfCwiseBinaryOp, Derived> tmp(derived()); typedef typename Derived::PlainObject PlainObject; + SelfCwiseBinaryOp, Derived, typename PlainObject::ConstantReturnType> tmp(derived()); tmp = PlainObject::Constant(rows(),cols(),other); return derived(); } @@ -160,10 +161,11 @@ inline Derived& DenseBase::operator*=(const Scalar& other) template inline Derived& DenseBase::operator/=(const Scalar& other) { - SelfCwiseBinaryOp::IsInteger, + typedef typename ei_meta_if::IsInteger, ei_scalar_quotient_op, - ei_scalar_product_op >::ret, Derived> tmp(derived()); + ei_scalar_product_op >::ret BinOp; typedef typename Derived::PlainObject PlainObject; + SelfCwiseBinaryOp tmp(derived()); tmp = PlainObject::Constant(rows(),cols(), NumTraits::IsInteger ? other : Scalar(1)/other); return derived(); } diff --git a/Eigen/src/Core/Transpose.h b/Eigen/src/Core/Transpose.h index 3a84b84ff..de8ae0a5b 100644 --- a/Eigen/src/Core/Transpose.h +++ b/Eigen/src/Core/Transpose.h @@ -303,11 +303,11 @@ inline void MatrixBase::adjointInPlace() // The following is to detect aliasing problems in most common cases. -template -struct ei_blas_traits > +template +struct ei_blas_traits > : ei_blas_traits { - typedef SelfCwiseBinaryOp XprType; + typedef SelfCwiseBinaryOp XprType; static inline const XprType extract(const XprType& x) { return x; } }; diff --git a/Eigen/src/Core/util/BlasUtil.h b/Eigen/src/Core/util/BlasUtil.h index 59b6d3827..972814dc9 100644 --- a/Eigen/src/Core/util/BlasUtil.h +++ b/Eigen/src/Core/util/BlasUtil.h @@ -35,7 +35,7 @@ struct ei_gebp_kernel; template struct ei_gemm_pack_rhs; -template +template struct ei_gemm_pack_lhs; template< diff --git a/Eigen/src/Core/util/Constants.h b/Eigen/src/Core/util/Constants.h index d758c2a0b..eb70981e4 100644 --- a/Eigen/src/Core/util/Constants.h +++ b/Eigen/src/Core/util/Constants.h @@ -176,7 +176,9 @@ enum { LinearVectorizedTraversal, /** \internal Generic vectorization path using one vectorized loop per row/column with some * scalar loops to handle the unaligned boundaries */ - SliceVectorizedTraversal + SliceVectorizedTraversal, + /** \internal Special case to properly handle incompatible scalar types or other defecting cases*/ + InvalidTraversal }; enum { diff --git a/Eigen/src/Core/util/ForwardDeclarations.h b/Eigen/src/Core/util/ForwardDeclarations.h index 72223c9e0..2c0e9c3e2 100644 --- a/Eigen/src/Core/util/ForwardDeclarations.h +++ b/Eigen/src/Core/util/ForwardDeclarations.h @@ -67,7 +67,7 @@ template class CwiseNullaryOp; template class CwiseUnaryOp; template class CwiseUnaryView; template class CwiseBinaryOp; -template class SelfCwiseBinaryOp; +template class SelfCwiseBinaryOp; template class ProductBase; template class GeneralProduct; template class CoeffBasedProduct;