Core: Add move support for DiagonalMatrix libeigen/eigen!3106 Co-authored-by: Rasmus Munk Larsen <rmlarsen@gmail.com>
diff --git a/Eigen/src/Core/Diagonal.h b/Eigen/src/Core/Diagonal.h index e716375..2d35569 100644 --- a/Eigen/src/Core/Diagonal.h +++ b/Eigen/src/Core/Diagonal.h
@@ -56,8 +56,8 @@ MatrixType::MaxColsAtCompileTime - plain_enum_max(DiagIndex, 0))), MaxColsAtCompileTime = 1, MaskLvalueBit = is_lvalue<MatrixType>::value ? LvalueBit : 0, - Flags = (unsigned int)MatrixTypeNested_::Flags & (RowMajorBit | MaskLvalueBit | DirectAccessBit) & - ~RowMajorBit, // FIXME DirectAccessBit should not be handled by expressions + Flags = (unsigned int)MatrixTypeNested_::Flags & + (MaskLvalueBit | DirectAccessBit), // FIXME DirectAccessBit should not be handled by expressions MatrixTypeOuterStride = outer_stride_at_compile_time<MatrixType>::value, OuterStrideAtCompileTime = 0 };
diff --git a/Eigen/src/Core/DiagonalMatrix.h b/Eigen/src/Core/DiagonalMatrix.h index 18f1697..989cbba 100644 --- a/Eigen/src/Core/DiagonalMatrix.h +++ b/Eigen/src/Core/DiagonalMatrix.h
@@ -235,6 +235,9 @@ inline DiagonalMatrix(const DiagonalMatrix& other) : m_diagonal(other.diagonal()) {} #endif + /** Move constructor. Moves the stored diagonal vector. */ + EIGEN_DEVICE_FUNC constexpr DiagonalMatrix(DiagonalMatrix&&) = default; + /** generic constructor from expression of the diagonal coefficients */ template <typename OtherDerived> EIGEN_DEVICE_FUNC constexpr explicit inline DiagonalMatrix(const MatrixBase<OtherDerived>& other) @@ -257,6 +260,19 @@ } #endif + /** Move assignment operator. Transfers dynamic storage and copies inline storage. */ + EIGEN_DEVICE_FUNC constexpr DiagonalMatrix& operator=(DiagonalMatrix&& other) noexcept( + DiagonalVectorType::MaxSizeAtCompileTime == Dynamic && + std::is_nothrow_move_assignable<DiagonalVectorType>::value) { + EIGEN_IF_CONSTEXPR (DiagonalVectorType::MaxSizeAtCompileTime == Dynamic) { + m_diagonal = std::move(other.m_diagonal); + } else { + // Preserve the vectorized assignment path for inline storage. + m_diagonal = other.m_diagonal; + } + return *this; + } + using InitializeReturnType = DiagonalWrapper<const CwiseNullaryOp<internal::scalar_constant_op<Scalar>, DiagonalVectorType>>;
diff --git a/benchmarks/Core/CMakeLists.txt b/benchmarks/Core/CMakeLists.txt index 139278c..67db020 100644 --- a/benchmarks/Core/CMakeLists.txt +++ b/benchmarks/Core/CMakeLists.txt
@@ -35,6 +35,7 @@ eigen_add_benchmark(bench_map bench_map.cpp) eigen_add_benchmark(bench_reshaped bench_reshaped.cpp) eigen_add_benchmark(bench_diagonal bench_diagonal.cpp) +eigen_add_benchmark(bench_diagonal_move bench_diagonal_move.cpp) eigen_add_benchmark(bench_triangular_product bench_triangular_product.cpp) eigen_add_benchmark(bench_selfadjoint_product bench_selfadjoint_product.cpp) eigen_add_benchmark(bench_symv bench_symv.cpp)
diff --git a/benchmarks/Core/bench_diagonal_move.cpp b/benchmarks/Core/bench_diagonal_move.cpp new file mode 100644 index 0000000..9e79c09 --- /dev/null +++ b/benchmarks/Core/bench_diagonal_move.cpp
@@ -0,0 +1,70 @@ +// SPDX-FileCopyrightText: The Eigen Authors +// SPDX-License-Identifier: MPL-2.0 + +#include <benchmark/benchmark.h> +#include <Eigen/Core> +#include <type_traits> + +template <typename DiagonalType, bool Move> +static void BM_DiagonalConstruct(benchmark::State& state) { + using Argument = std::conditional_t<Move, DiagonalType&&, const DiagonalType&>; + DiagonalType source(state.range(0)); + source.diagonal().setRandom(); + const DiagonalType expected(source); + for (auto _ : state) { + DiagonalType temporary(static_cast<Argument>(source)); + benchmark::DoNotOptimize(temporary); + benchmark::ClobberMemory(); + source = static_cast<Argument>(temporary); + benchmark::DoNotOptimize(source); + benchmark::ClobberMemory(); + } + if (source.rows() != expected.rows() || source.diagonal() != expected.diagonal()) { + state.SkipWithError("Diagonal construction changed coefficients"); + } + state.SetItemsProcessed(state.iterations()); +} + +template <typename DiagonalType, bool Move> +static void BM_DiagonalAssign(benchmark::State& state) { + using Argument = std::conditional_t<Move, DiagonalType&&, const DiagonalType&>; + DiagonalType source(state.range(0)), destination(state.range(0)); + source.diagonal().setRandom(); + destination.diagonal().setZero(); + const DiagonalType expected(source); + for (auto _ : state) { + destination = static_cast<Argument>(source); + benchmark::DoNotOptimize(destination); + benchmark::ClobberMemory(); + source = static_cast<Argument>(destination); + benchmark::DoNotOptimize(source); + benchmark::ClobberMemory(); + } + if (source.rows() != expected.rows() || source.diagonal() != expected.diagonal()) { + state.SkipWithError("Diagonal assignment changed coefficients"); + } + state.SetItemsProcessed(2 * state.iterations()); +} + +using DynamicFloat = Eigen::DiagonalMatrix<float, Eigen::Dynamic>; +using DynamicDouble = Eigen::DiagonalMatrix<double, Eigen::Dynamic>; +using DynamicComplex = Eigen::DiagonalMatrix<std::complex<double>, Eigen::Dynamic>; +using Fixed4 = Eigen::DiagonalMatrix<double, 4>; +using Fixed16 = Eigen::DiagonalMatrix<double, 16>; +using Bounded16 = Eigen::DiagonalMatrix<double, Eigen::Dynamic, 16>; + +// Construct measures construction plus assignment back; Assign measures two assignments. +#define DIAGONAL_MOVE_BENCHMARKS(Type, ...) \ + BENCHMARK_TEMPLATE(BM_DiagonalConstruct, Type, true)->ArgNames({"size"})->ArgsProduct({{__VA_ARGS__}}); \ + BENCHMARK_TEMPLATE(BM_DiagonalConstruct, Type, false)->ArgNames({"size"})->ArgsProduct({{__VA_ARGS__}}); \ + BENCHMARK_TEMPLATE(BM_DiagonalAssign, Type, true)->ArgNames({"size"})->ArgsProduct({{__VA_ARGS__}}); \ + BENCHMARK_TEMPLATE(BM_DiagonalAssign, Type, false)->ArgNames({"size"})->ArgsProduct({{__VA_ARGS__}}) + +DIAGONAL_MOVE_BENCHMARKS(DynamicFloat, 0, 1, 4, 16, 256, 4096, 65536); +DIAGONAL_MOVE_BENCHMARKS(DynamicDouble, 0, 1, 4, 16, 256, 4096, 65536); +DIAGONAL_MOVE_BENCHMARKS(DynamicComplex, 0, 1, 4, 16, 256, 4096, 65536); +DIAGONAL_MOVE_BENCHMARKS(Fixed4, 4); +DIAGONAL_MOVE_BENCHMARKS(Fixed16, 16); +DIAGONAL_MOVE_BENCHMARKS(Bounded16, 0, 1, 4, 16); + +#undef DIAGONAL_MOVE_BENCHMARKS
diff --git a/test/diagonalmatrices.cpp b/test/diagonalmatrices.cpp index 5271c8a..12e4e78 100644 --- a/test/diagonalmatrices.cpp +++ b/test/diagonalmatrices.cpp
@@ -14,6 +14,8 @@ #define EIGEN_RUNTIME_NO_MALLOC #include "main.h" +#include <Eigen/Core> +#include "MovableScalar.h" #include "random_for_arithmetic.h" using namespace std; template <typename MatrixType> @@ -510,6 +512,74 @@ } } +template <typename DiagonalType> +void diagonal_matrix_move(Index size) { + using Scalar = typename DiagonalType::Scalar; + using VectorType = typename DiagonalType::DiagonalVectorType; + STATIC_CHECK(std::is_nothrow_move_constructible<DiagonalType>::value == + std::is_nothrow_move_constructible<VectorType>::value); + STATIC_CHECK(VectorType::MaxSizeAtCompileTime != Dynamic || std::is_nothrow_move_assignable<DiagonalType>::value == + std::is_nothrow_move_assignable<VectorType>::value); + + DiagonalType source; + source.resize(size); + for (Index i = 0; i < size; ++i) { + const Scalar value = Scalar(i + 1); + source.diagonal()(i) = value; + } + const DiagonalType expected(source); + DiagonalType copied; + copied = source; + const Scalar* original_data = source.diagonal().data(); + + internal::set_is_malloc_allowed(false); + DiagonalType constructed(std::move(source)); + internal::set_is_malloc_allowed(true); + VERIFY_IS_EQUAL(constructed.rows(), size); + if (VectorType::MaxSizeAtCompileTime == Dynamic) VERIFY(constructed.diagonal().data() == original_data); + for (Index i = 0; i < size; ++i) { + VERIFY_IS_EQUAL(constructed.diagonal()(i), expected.diagonal()(i)); + VERIFY_IS_EQUAL(copied.diagonal()(i), expected.diagonal()(i)); + } + + source = expected; + DiagonalType assigned; + internal::set_is_malloc_allowed(false); + assigned = std::move(constructed); + internal::set_is_malloc_allowed(true); + VERIFY_IS_EQUAL(assigned.rows(), size); + if (VectorType::MaxSizeAtCompileTime == Dynamic) VERIFY(assigned.diagonal().data() == original_data); + for (Index i = 0; i < size; ++i) VERIFY_IS_EQUAL(assigned.diagonal()(i), expected.diagonal()(i)); + + constructed = expected; + // Also move into an existing allocation, with a different size when storage is dynamic. + if (VectorType::SizeAtCompileTime == Dynamic) assigned.resize(size == 0 ? 1 : size - 1); + original_data = constructed.diagonal().data(); + internal::set_is_malloc_allowed(false); + assigned = std::move(constructed); + internal::set_is_malloc_allowed(true); + VERIFY_IS_EQUAL(assigned.rows(), size); + if (VectorType::MaxSizeAtCompileTime == Dynamic) VERIFY(assigned.diagonal().data() == original_data); + for (Index i = 0; i < size; ++i) VERIFY_IS_EQUAL(assigned.diagonal()(i), expected.diagonal()(i)); + + constructed = expected; + for (Index i = 0; i < size; ++i) { + VERIFY_IS_EQUAL(source.diagonal()(i), expected.diagonal()(i)); + VERIFY_IS_EQUAL(constructed.diagonal()(i), expected.diagonal()(i)); + } +} + +template <typename Scalar> +void diagonal_matrix_moves() { + diagonal_matrix_move<DiagonalMatrix<Scalar, 0>>(0); + diagonal_matrix_move<DiagonalMatrix<Scalar, 1>>(1); + diagonal_matrix_move<DiagonalMatrix<Scalar, 4>>(4); + for (Index size : {0, 1, 4, 17}) { + diagonal_matrix_move<DiagonalMatrix<Scalar, Dynamic>>(size); + diagonal_matrix_move<DiagonalMatrix<Scalar, Dynamic, 17>>(size); + } +} + EIGEN_DECLARE_TEST(diagonalmatrices) { for (int i = 0; i < g_repeat; i++) { CALL_SUBTEST_1(diagonalmatrices(Matrix<float, 1, 1>())); @@ -536,4 +606,9 @@ CALL_SUBTEST_10(selfadjoint_diagonal_products<0>()); CALL_SUBTEST_10(selfadjoint_diagonal_products_block_path<0>()); CALL_SUBTEST_10(structured_diagonal_aliasing<0>()); + CALL_SUBTEST_11(diagonal_matrix_moves<float>()); + CALL_SUBTEST_11(diagonal_matrix_moves<double>()); + CALL_SUBTEST_11(diagonal_matrix_moves<int>()); + CALL_SUBTEST_11(diagonal_matrix_moves<std::complex<double>>()); + CALL_SUBTEST_11(diagonal_matrix_moves<MovableScalar<float>>()); }