From aed1d6597fc39e8cab8112166d4b5860ca03c25c Mon Sep 17 00:00:00 2001 From: Gael Guennebaud Date: Thu, 1 Dec 2016 21:55:10 +0100 Subject: [PATCH 1/6] Clean up SparseCore module regarding ReverseInnerIterator (grafted from 27873008d431a307bed9c200a12622a361af4d14 ) --- Eigen/src/SparseCore/SparseBlock.h | 3 -- Eigen/src/SparseCore/SparseCompressedBase.h | 1 - Eigen/src/SparseCore/SparseCwiseBinaryOp.h | 6 --- Eigen/src/SparseCore/SparseCwiseUnaryOp.h | 54 --------------------- Eigen/src/SparseCore/SparseTranspose.h | 12 ----- 5 files changed, 76 deletions(-) diff --git a/Eigen/src/SparseCore/SparseBlock.h b/Eigen/src/SparseCore/SparseBlock.h index acaf933f4..cb8d9d2e2 100644 --- a/Eigen/src/SparseCore/SparseBlock.h +++ b/Eigen/src/SparseCore/SparseBlock.h @@ -432,7 +432,6 @@ public: protected: // friend class internal::GenericSparseBlockInnerIteratorImpl; - friend class ReverseInnerIterator; friend struct internal::unary_evaluator, internal::IteratorBased, Scalar >; Index nonZeros() const { return Dynamic; } @@ -468,8 +467,6 @@ struct unary_evaluator, IteratorBa typedef typename XprType::StorageIndex StorageIndex; typedef typename XprType::Scalar Scalar; - class ReverseInnerIterator; - enum { IsRowMajor = XprType::IsRowMajor, diff --git a/Eigen/src/SparseCore/SparseCompressedBase.h b/Eigen/src/SparseCore/SparseCompressedBase.h index 710eb0156..e0850795c 100644 --- a/Eigen/src/SparseCore/SparseCompressedBase.h +++ b/Eigen/src/SparseCore/SparseCompressedBase.h @@ -273,7 +273,6 @@ struct evaluator > { typedef typename Derived::Scalar Scalar; typedef typename Derived::InnerIterator InnerIterator; - typedef typename Derived::ReverseInnerIterator ReverseInnerIterator; enum { CoeffReadCost = NumTraits::ReadCost, diff --git a/Eigen/src/SparseCore/SparseCwiseBinaryOp.h b/Eigen/src/SparseCore/SparseCwiseBinaryOp.h index 4ba4d631d..0a9bdeac2 100644 --- a/Eigen/src/SparseCore/SparseCwiseBinaryOp.h +++ b/Eigen/src/SparseCore/SparseCwiseBinaryOp.h @@ -68,7 +68,6 @@ protected: typedef typename XprType::StorageIndex StorageIndex; public: - class ReverseInnerIterator; class InnerIterator { public: @@ -161,7 +160,6 @@ protected: typedef typename XprType::StorageIndex StorageIndex; public: - class ReverseInnerIterator; class InnerIterator { enum { IsRowMajor = (int(Rhs::Flags)&RowMajorBit)==RowMajorBit }; @@ -249,7 +247,6 @@ protected: typedef typename XprType::StorageIndex StorageIndex; public: - class ReverseInnerIterator; class InnerIterator { enum { IsRowMajor = (int(Lhs::Flags)&RowMajorBit)==RowMajorBit }; @@ -402,7 +399,6 @@ protected: typedef typename traits::Scalar Scalar; public: - class ReverseInnerIterator; class InnerIterator { public: @@ -487,7 +483,6 @@ protected: typedef typename traits::Scalar Scalar; public: - class ReverseInnerIterator; class InnerIterator { enum { IsRowMajor = (int(RhsArg::Flags)&RowMajorBit)==RowMajorBit }; @@ -561,7 +556,6 @@ protected: typedef typename traits::Scalar Scalar; public: - class ReverseInnerIterator; class InnerIterator { enum { IsRowMajor = (int(LhsArg::Flags)&RowMajorBit)==RowMajorBit }; diff --git a/Eigen/src/SparseCore/SparseCwiseUnaryOp.h b/Eigen/src/SparseCore/SparseCwiseUnaryOp.h index 9143a4c82..28f221437 100644 --- a/Eigen/src/SparseCore/SparseCwiseUnaryOp.h +++ b/Eigen/src/SparseCore/SparseCwiseUnaryOp.h @@ -22,7 +22,6 @@ struct unary_evaluator, IteratorBased> typedef CwiseUnaryOp XprType; class InnerIterator; - class ReverseInnerIterator; enum { CoeffReadCost = evaluator::CoeffReadCost + functor_traits::Cost, @@ -41,7 +40,6 @@ struct unary_evaluator, IteratorBased> protected: typedef typename evaluator::InnerIterator EvalIterator; -// typedef typename evaluator::ReverseInnerIterator EvalReverseIterator; const UnaryOp m_functor; evaluator m_argImpl; @@ -70,33 +68,6 @@ class unary_evaluator, IteratorBased>::InnerIterat Scalar& valueRef(); }; -// template -// class unary_evaluator, IteratorBased>::ReverseInnerIterator -// : public unary_evaluator, IteratorBased>::EvalReverseIterator -// { -// typedef typename XprType::Scalar Scalar; -// typedef typename unary_evaluator, IteratorBased>::EvalReverseIterator Base; -// public: -// -// EIGEN_STRONG_INLINE ReverseInnerIterator(const XprType& unaryOp, typename XprType::Index outer) -// : Base(unaryOp.derived().nestedExpression(),outer), m_functor(unaryOp.derived().functor()) -// {} -// -// EIGEN_STRONG_INLINE ReverseInnerIterator& operator--() -// { Base::operator--(); return *this; } -// -// EIGEN_STRONG_INLINE Scalar value() const { return m_functor(Base::value()); } -// -// protected: -// const UnaryOp m_functor; -// private: -// Scalar& valueRef(); -// }; - - - - - template struct unary_evaluator, IteratorBased> : public evaluator_base > @@ -105,7 +76,6 @@ struct unary_evaluator, IteratorBased> typedef CwiseUnaryView XprType; class InnerIterator; - class ReverseInnerIterator; enum { CoeffReadCost = evaluator::CoeffReadCost + functor_traits::Cost, @@ -120,7 +90,6 @@ struct unary_evaluator, IteratorBased> protected: typedef typename evaluator::InnerIterator EvalIterator; -// typedef typename evaluator::ReverseInnerIterator EvalReverseIterator; const ViewOp m_functor; evaluator m_argImpl; @@ -148,29 +117,6 @@ class unary_evaluator, IteratorBased>::InnerItera const ViewOp m_functor; }; -// template -// class unary_evaluator, IteratorBased>::ReverseInnerIterator -// : public unary_evaluator, IteratorBased>::EvalReverseIterator -// { -// typedef typename XprType::Scalar Scalar; -// typedef typename unary_evaluator, IteratorBased>::EvalReverseIterator Base; -// public: -// -// EIGEN_STRONG_INLINE ReverseInnerIterator(const XprType& unaryOp, typename XprType::Index outer) -// : Base(unaryOp.derived().nestedExpression(),outer), m_functor(unaryOp.derived().functor()) -// {} -// -// EIGEN_STRONG_INLINE ReverseInnerIterator& operator--() -// { Base::operator--(); return *this; } -// -// EIGEN_STRONG_INLINE Scalar value() const { return m_functor(Base::value()); } -// EIGEN_STRONG_INLINE Scalar& valueRef() { return m_functor(Base::valueRef()); } -// -// protected: -// const ViewOp m_functor; -// }; - - } // end namespace internal template diff --git a/Eigen/src/SparseCore/SparseTranspose.h b/Eigen/src/SparseCore/SparseTranspose.h index b6f180a41..3757d4c6b 100644 --- a/Eigen/src/SparseCore/SparseTranspose.h +++ b/Eigen/src/SparseCore/SparseTranspose.h @@ -56,7 +56,6 @@ struct unary_evaluator, IteratorBased> : public evaluator_base > { typedef typename evaluator::InnerIterator EvalIterator; - typedef typename evaluator::ReverseInnerIterator EvalReverseIterator; public: typedef Transpose XprType; @@ -75,17 +74,6 @@ struct unary_evaluator, IteratorBased> Index col() const { return EvalIterator::row(); } }; - class ReverseInnerIterator : public EvalReverseIterator - { - public: - EIGEN_STRONG_INLINE ReverseInnerIterator(const unary_evaluator& unaryOp, Index outer) - : EvalReverseIterator(unaryOp.m_argImpl,outer) - {} - - Index row() const { return EvalReverseIterator::col(); } - Index col() const { return EvalReverseIterator::row(); } - }; - enum { CoeffReadCost = evaluator::CoeffReadCost, Flags = XprType::Flags From a4c8701e9aa0ac14ffb626692a22995bda2026fb Mon Sep 17 00:00:00 2001 From: Gael Guennebaud Date: Mon, 5 Dec 2016 15:08:09 +0100 Subject: [PATCH 2/6] bug #1356: fix calls to evaluator::coeffRef(0,0) to get the address of the destination by adding a dstDataPtr() member to the kernel. This fixes undefined behavior if dst is empty (nullptr). (grafted from 0db6d5b3f434ae2e0c2e8b78402e062e67e86339 ) --- Eigen/src/Core/AssignEvaluator.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Eigen/src/Core/AssignEvaluator.h b/Eigen/src/Core/AssignEvaluator.h index 0d0189657..14400d246 100644 --- a/Eigen/src/Core/AssignEvaluator.h +++ b/Eigen/src/Core/AssignEvaluator.h @@ -407,7 +407,7 @@ struct dense_assignment_loop : int(Kernel::AssignmentTraits::DstAlignment), srcAlignment = Kernel::AssignmentTraits::JointAlignment }; - const Index alignedStart = dstIsAligned ? 0 : internal::first_aligned(&kernel.dstEvaluator().coeffRef(0), size); + const Index alignedStart = dstIsAligned ? 0 : internal::first_aligned(kernel.dstDataPtr(), size); const Index alignedEnd = alignedStart + ((size-alignedStart)/packetSize)*packetSize; unaligned_dense_assignment_loop::run(kernel, 0, alignedStart); @@ -527,7 +527,7 @@ struct dense_assignment_loop dstAlignment = alignable ? int(requestedAlignment) : int(Kernel::AssignmentTraits::DstAlignment) }; - const Scalar *dst_ptr = &kernel.dstEvaluator().coeffRef(0,0); + const Scalar *dst_ptr = kernel.dstDataPtr(); if((!bool(dstIsAligned)) && (UIntPtr(dst_ptr) % sizeof(Scalar))>0) { // the pointer is not aligend-on scalar, so alignment is not possible @@ -683,6 +683,11 @@ public: : int(DstEvaluatorType::Flags)&RowMajorBit ? inner : outer; } + + EIGEN_DEVICE_FUNC const Scalar* dstDataPtr() const + { + return m_dstExpr.data(); + } protected: DstEvaluatorType& m_dst; From ea56d2ff2ca8f102300ff4c34d8534f0ed55f1dd Mon Sep 17 00:00:00 2001 From: Gael Guennebaud Date: Mon, 5 Dec 2016 16:59:30 +0100 Subject: [PATCH 3/6] Fix memory leak in Ref (grafted from a6b971e291e9eb980eb94fa7d701f7b757dbcbd0 ) --- Eigen/src/SparseCore/SparseRef.h | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/Eigen/src/SparseCore/SparseRef.h b/Eigen/src/SparseCore/SparseRef.h index a558230e7..d91f38f97 100644 --- a/Eigen/src/SparseCore/SparseRef.h +++ b/Eigen/src/SparseCore/SparseRef.h @@ -185,20 +185,27 @@ class Ref, Options, StrideType EIGEN_SPARSE_PUBLIC_INTERFACE(Ref) template - inline Ref(const SparseMatrixBase& expr) + inline Ref(const SparseMatrixBase& expr) : m_hasCopy(false) { construct(expr.derived(), typename Traits::template match::type()); } - inline Ref(const Ref& other) : Base(other) { + inline Ref(const Ref& other) : Base(other), m_hasCopy(false) { // copy constructor shall not copy the m_object, to avoid unnecessary malloc and copy } template - inline Ref(const RefBase& other) { + inline Ref(const RefBase& other) : m_hasCopy(false) { construct(other.derived(), typename Traits::template match::type()); } + ~Ref() { + if(m_hasCopy) { + TPlainObjectType* obj = reinterpret_cast(m_object_bytes); + obj->~TPlainObjectType(); + } + } + protected: template @@ -208,6 +215,7 @@ class Ref, Options, StrideType { TPlainObjectType* obj = reinterpret_cast(m_object_bytes); ::new (obj) TPlainObjectType(expr); + m_hasCopy = true; Base::construct(*obj); } else @@ -221,11 +229,13 @@ class Ref, Options, StrideType { TPlainObjectType* obj = reinterpret_cast(m_object_bytes); ::new (obj) TPlainObjectType(expr); + m_hasCopy = true; Base::construct(*obj); } protected: char m_object_bytes[sizeof(TPlainObjectType)]; + bool m_hasCopy; }; @@ -293,20 +303,27 @@ class Ref, Options, StrideType EIGEN_SPARSE_PUBLIC_INTERFACE(Ref) template - inline Ref(const SparseMatrixBase& expr) + inline Ref(const SparseMatrixBase& expr) : m_hasCopy(false) { construct(expr.derived(), typename Traits::template match::type()); } - inline Ref(const Ref& other) : Base(other) { + inline Ref(const Ref& other) : Base(other), m_hasCopy(false) { // copy constructor shall not copy the m_object, to avoid unnecessary malloc and copy } template - inline Ref(const RefBase& other) { + inline Ref(const RefBase& other) : m_hasCopy(false) { construct(other.derived(), typename Traits::template match::type()); } + ~Ref() { + if(m_hasCopy) { + TPlainObjectType* obj = reinterpret_cast(m_object_bytes); + obj->~TPlainObjectType(); + } + } + protected: template @@ -320,11 +337,13 @@ class Ref, Options, StrideType { TPlainObjectType* obj = reinterpret_cast(m_object_bytes); ::new (obj) TPlainObjectType(expr); + m_hasCopy = true; Base::construct(*obj); } protected: char m_object_bytes[sizeof(TPlainObjectType)]; + bool m_hasCopy; }; namespace internal { From 0164f4c6828690a3e5c9fe80f1b670b2be2ec3c5 Mon Sep 17 00:00:00 2001 From: Silvio Traversaro Date: Mon, 5 Dec 2016 10:39:52 +0100 Subject: [PATCH 4/6] Make CMake config file relocatable (grafted from 18481b518fd05bb7007210949350d4104b70e7f5 ) --- CMakeLists.txt | 1 + cmake/Eigen3Config.cmake.in | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a7d3446d2..767f4d5ea 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -531,6 +531,7 @@ if (NOT CMAKE_VERSION VERSION_LESS 3.0) configure_package_config_file ( ${CMAKE_CURRENT_SOURCE_DIR}/cmake/Eigen3Config.cmake.in ${CMAKE_CURRENT_BINARY_DIR}/Eigen3Config.cmake + PATH_VARS EIGEN_INCLUDE_DIR EIGEN_ROOT_DIR INSTALL_DESTINATION ${CMAKEPACKAGE_INSTALL_DIR} NO_CHECK_REQUIRED_COMPONENTS_MACRO # Eigen does not provide components ) diff --git a/cmake/Eigen3Config.cmake.in b/cmake/Eigen3Config.cmake.in index 01937a3cc..c5c546887 100644 --- a/cmake/Eigen3Config.cmake.in +++ b/cmake/Eigen3Config.cmake.in @@ -11,9 +11,9 @@ set (EIGEN3_FOUND 1) set (EIGEN3_USE_FILE "${CMAKE_CURRENT_LIST_DIR}/UseEigen3.cmake") set (EIGEN3_DEFINITIONS "@EIGEN_DEFINITIONS@") -set (EIGEN3_INCLUDE_DIR "@EIGEN_INCLUDE_DIR@") -set (EIGEN3_INCLUDE_DIRS "@EIGEN_INCLUDE_DIRS@") -set (EIGEN3_ROOT_DIR "@EIGEN_ROOT_DIR@") +set (EIGEN3_INCLUDE_DIR "@PACKAGE_EIGEN_INCLUDE_DIR@") +set (EIGEN3_INCLUDE_DIRS "@PACKAGE_EIGEN_INCLUDE_DIR@") +set (EIGEN3_ROOT_DIR "@PACKAGE_EIGEN_ROOT_DIR@") set (EIGEN3_VERSION_STRING "@EIGEN_VERSION_STRING@") set (EIGEN3_VERSION_MAJOR "@EIGEN_VERSION_MAJOR@") From 75f0b8aae37b45cbcc3b1fa03ad938cf54cb8ede Mon Sep 17 00:00:00 2001 From: Silvio Traversaro Date: Tue, 6 Dec 2016 10:37:34 +0100 Subject: [PATCH 5/6] Added relocatable cmake support also for CMake before 3.0 and after 2.8.8 (grafted from e049a2a72a307cad9e078077148c6d3ee9552412 ) --- CMakeLists.txt | 29 ++++++++++++++++++++++++----- cmake/Eigen3ConfigLegacy.cmake.in | 8 +++++--- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 767f4d5ea..7237b54df 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -507,7 +507,6 @@ set ( EIGEN_VERSION_MINOR ${EIGEN_MAJOR_VERSION} ) set ( EIGEN_VERSION_PATCH ${EIGEN_MINOR_VERSION} ) set ( EIGEN_DEFINITIONS "") set ( EIGEN_INCLUDE_DIR "${CMAKE_INSTALL_PREFIX}/${INCLUDE_INSTALL_DIR}" ) -set ( EIGEN_INCLUDE_DIRS ${EIGEN_INCLUDE_DIR} ) set ( EIGEN_ROOT_DIR ${CMAKE_INSTALL_PREFIX} ) # Interface libraries require at least CMake 3.0 @@ -561,10 +560,30 @@ if (NOT CMAKE_VERSION VERSION_LESS 3.0) DESTINATION ${CMAKEPACKAGE_INSTALL_DIR}) else (NOT CMAKE_VERSION VERSION_LESS 3.0) # Fallback to legacy Eigen3Config.cmake without the imported target - configure_file ( ${CMAKE_CURRENT_SOURCE_DIR}/cmake/Eigen3ConfigLegacy.cmake.in - ${CMAKE_CURRENT_BINARY_DIR}/Eigen3Config.cmake - @ONLY ESCAPE_QUOTES - ) + + # If CMakePackageConfigHelpers module is available (CMake >= 2.8.8) + # create a relocatable Config file, otherwise leave the hardcoded paths + include(CMakePackageConfigHelpers OPTIONAL RESULT_VARIABLE CPCH_PATH) + + if(CPCH_PATH) + configure_package_config_file ( + ${CMAKE_CURRENT_SOURCE_DIR}/cmake/Eigen3ConfigLegacy.cmake.in + ${CMAKE_CURRENT_BINARY_DIR}/Eigen3Config.cmake + PATH_VARS EIGEN_INCLUDE_DIR EIGEN_ROOT_DIR + INSTALL_DESTINATION ${CMAKEPACKAGE_INSTALL_DIR} + NO_CHECK_REQUIRED_COMPONENTS_MACRO # Eigen does not provide components + ) + else() + # The PACKAGE_* variables are defined by the configure_package_config_file + # but without it we define them manually to the hardcoded paths + set(PACKAGE_INIT "") + set(PACKAGE_EIGEN_INCLUDE_DIR ${EIGEN_INCLUDE_DIR}) + set(PACKAGE_EIGEN_ROOT_DIR ${EIGEN_ROOT_DIR}) + configure_file ( ${CMAKE_CURRENT_SOURCE_DIR}/cmake/Eigen3ConfigLegacy.cmake.in + ${CMAKE_CURRENT_BINARY_DIR}/Eigen3Config.cmake + @ONLY ESCAPE_QUOTES + ) + endif() install ( FILES ${CMAKE_CURRENT_SOURCE_DIR}/cmake/UseEigen3.cmake ${CMAKE_CURRENT_BINARY_DIR}/Eigen3Config.cmake diff --git a/cmake/Eigen3ConfigLegacy.cmake.in b/cmake/Eigen3ConfigLegacy.cmake.in index 04e7886ce..62d722469 100644 --- a/cmake/Eigen3ConfigLegacy.cmake.in +++ b/cmake/Eigen3ConfigLegacy.cmake.in @@ -14,13 +14,15 @@ # EIGEN3_VERSION_MINOR - The minor version of Eigen # EIGEN3_VERSION_PATCH - The patch version of Eigen +@PACKAGE_INIT@ + set ( EIGEN3_FOUND 1 ) set ( EIGEN3_USE_FILE "${CMAKE_CURRENT_LIST_DIR}/UseEigen3.cmake" ) set ( EIGEN3_DEFINITIONS "@EIGEN_DEFINITIONS@" ) -set ( EIGEN3_INCLUDE_DIR "@EIGEN_INCLUDE_DIR@" ) -set ( EIGEN3_INCLUDE_DIRS "@EIGEN_INCLUDE_DIRS@" ) -set ( EIGEN3_ROOT_DIR "@EIGEN_ROOT_DIR@" ) +set ( EIGEN3_INCLUDE_DIR "@PACKAGE_EIGEN_INCLUDE_DIR@" ) +set ( EIGEN3_INCLUDE_DIRS "@PACKAGE_EIGEN_INCLUDE_DIR@" ) +set ( EIGEN3_ROOT_DIR "@PACKAGE_EIGEN_ROOT_DIR@" ) set ( EIGEN3_VERSION_STRING "@EIGEN_VERSION_STRING@" ) set ( EIGEN3_VERSION_MAJOR "@EIGEN_VERSION_MAJOR@" ) From 487a6e65152b1b1043321359e9f277c3a8a8d8fb Mon Sep 17 00:00:00 2001 From: Gael Guennebaud Date: Tue, 6 Dec 2016 11:34:06 +0100 Subject: [PATCH 6/6] Explain how to choose your favorite Eigen version (grafted from 0c4d05b0091c84687e2470822743e8f89d2a1ebb ) --- doc/TopicCMakeGuide.dox | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/doc/TopicCMakeGuide.dox b/doc/TopicCMakeGuide.dox index a540db68e..896cfa831 100644 --- a/doc/TopicCMakeGuide.dox +++ b/doc/TopicCMakeGuide.dox @@ -32,6 +32,11 @@ which requires at least version 3.3 of %Eigen. Here, `path-to-example-directory` is the path to the directory that contains both `CMakeLists.txt` and `example.cpp`. +If you have multiple installed version of %Eigen, you can pick your favorite one by setting the \c Eigen3_DIR cmake's variable to the respective path containing the \c Eigen3*.cmake files. For instance: +\code +cmake path-to-example-directory -DEigen3_DIR=$HOME/mypackages/share/eigen3/cmake/ +\endcode + If the `REQUIRED` option is omitted when locating %Eigen using `find_package`, one can check whether the package was found as follows: \code{.cmake} @@ -45,5 +50,3 @@ endif (TARGET Eigen3::Eigen) */ } - -// vim: set ft=cpp.doxygen