From 3e95609cd4f036ccb647f9a590062cdb250e9760 Mon Sep 17 00:00:00 2001 From: Benoit Jacob Date: Tue, 1 Jun 2010 09:01:39 -0400 Subject: [PATCH] Backed out changeset 641d968a9a7ed57a3b8a3f45dea43c5ee6717f97 --- Eigen/src/Core/NoAlias.h | 10 ++-------- Eigen/src/Core/ReturnByValue.h | 11 ++++------- Eigen/src/LU/Inverse.h | 19 ++++++++++++++++--- test/prec_inverse_4x4.cpp | 4 +--- 4 files changed, 23 insertions(+), 21 deletions(-) diff --git a/Eigen/src/Core/NoAlias.h b/Eigen/src/Core/NoAlias.h index 0542571e2..30ddbeb3c 100644 --- a/Eigen/src/Core/NoAlias.h +++ b/Eigen/src/Core/NoAlias.h @@ -45,18 +45,12 @@ class NoAlias public: NoAlias(ExpressionType& expression) : m_expression(expression) {} - /* \sa MatrixBase::lazyAssign() */ + /** Behaves like MatrixBase::lazyAssign(other) + * \sa MatrixBase::lazyAssign() */ template EIGEN_STRONG_INLINE ExpressionType& operator=(const StorageBase& other) { return m_expression.lazyAssign(other.derived()); } - template - EIGEN_STRONG_INLINE ExpressionType& operator=(const ReturnByValue& other) - { - other.evalTo(m_expression); - return m_expression; - } - /** \sa MatrixBase::operator+= */ template EIGEN_STRONG_INLINE ExpressionType& operator+=(const StorageBase& other) diff --git a/Eigen/src/Core/ReturnByValue.h b/Eigen/src/Core/ReturnByValue.h index b2e581c70..986bab54d 100644 --- a/Eigen/src/Core/ReturnByValue.h +++ b/Eigen/src/Core/ReturnByValue.h @@ -36,9 +36,9 @@ struct ei_traits > enum { // We're disabling the DirectAccess because e.g. the constructor of // the Block-with-DirectAccess expression requires to have a coeffRef method. - // FIXME this should be fixed so we can have DirectAccessBit here. + // Also, we don't want to have to implement the stride stuff. Flags = (ei_traits::ReturnType>::Flags - | EvalBeforeNestingBit | EvalBeforeAssigningBit) & ~DirectAccessBit + | EvalBeforeNestingBit) & ~DirectAccessBit }; }; @@ -83,11 +83,8 @@ template template Derived& DenseBase::operator=(const ReturnByValue& other) { - // since we're by-passing the mechanisms in Assign.h, we implement here the EvalBeforeAssigningBit. - // override by using .noalias(), see corresponding operator= in NoAlias. - typename Derived::PlainObject result(rows(), cols()); - other.evalTo(result); - return (derived() = result); + other.evalTo(derived()); + return derived(); } #endif // EIGEN_RETURNBYVALUE_H diff --git a/Eigen/src/LU/Inverse.h b/Eigen/src/LU/Inverse.h index 1e9d69a22..9e0c46094 100644 --- a/Eigen/src/LU/Inverse.h +++ b/Eigen/src/LU/Inverse.h @@ -281,9 +281,15 @@ struct ei_traits > template struct ei_inverse_impl : public ReturnByValue > { - typedef typename MatrixType::Nested MatrixTypeNested; + // for 2x2, it's worth giving a chance to avoid evaluating. + // for larger sizes, evaluating has negligible cost, limits code size, + // and allows for vectorized paths. + typedef typename ei_meta_if< + MatrixType::RowsAtCompileTime == 2, + typename ei_nested::type, + typename ei_eval::type + >::ret MatrixTypeNested; typedef typename ei_cleantype::type MatrixTypeNestedCleaned; - const MatrixTypeNested m_matrix; ei_inverse_impl(const MatrixType& matrix) @@ -353,7 +359,14 @@ inline void MatrixBase::computeInverseAndDetWithCheck( { // i'd love to put some static assertions there, but SFINAE means that they have no effect... ei_assert(rows() == cols()); - ei_compute_inverse_and_det_with_check::run + // for 2x2, it's worth giving a chance to avoid evaluating. + // for larger sizes, evaluating has negligible cost and limits code size. + typedef typename ei_meta_if< + RowsAtCompileTime == 2, + typename ei_cleantype::type>::type, + PlainObject + >::ret MatrixType; + ei_compute_inverse_and_det_with_check::run (derived(), absDeterminantThreshold, inverse, determinant, invertible); } diff --git a/test/prec_inverse_4x4.cpp b/test/prec_inverse_4x4.cpp index 05dbee7b5..4150caec2 100644 --- a/test/prec_inverse_4x4.cpp +++ b/test/prec_inverse_4x4.cpp @@ -64,9 +64,7 @@ template void inverse_general_4x4(int repeat) double error_avg = error_sum / repeat; EIGEN_DEBUG_VAR(error_avg); EIGEN_DEBUG_VAR(error_max); - // FIXME that 1.3 used to be a 1.0 until the NumTraits changes on 28 April 2010, and then a 1.2 until the ReturnByValue/Inverse changes - // on 30 May 2010, what's going wrong (if anything) ?? - VERIFY(error_avg < (NumTraits::IsComplex ? 8.0 : 1.3)); + VERIFY(error_avg < (NumTraits::IsComplex ? 8.0 : 1.2)); // FIXME that 1.2 used to be a 1.0 until the NumTraits changes on 28 April 2010, what's going wrong?? VERIFY(error_max < (NumTraits::IsComplex ? 64.0 : 20.0)); }