From 892d73643c3422d1590ed2c299c3fcc3f15bc3c1 Mon Sep 17 00:00:00 2001 From: Peng Liu Date: Mon, 23 Dec 2024 15:34:15 -0500 Subject: [PATCH 1/3] Optimize ranges::move{,_backward} for vector::iterator --- libcxx/docs/ReleaseNotes/21.rst | 4 +- libcxx/include/__algorithm/move.h | 10 ++ libcxx/include/__algorithm/move_backward.h | 10 ++ libcxx/include/__bit_reference | 16 --- .../test/benchmarks/algorithms/move.bench.cpp | 55 ++++++++ .../algorithms/move_backward.bench.cpp | 55 ++++++++ .../alg.move/move.pass.cpp | 53 ++++++-- .../alg.move/move_backward.pass.cpp | 60 ++++++--- .../alg.move/ranges.move.pass.cpp | 107 +++++++++++----- .../alg.move/ranges.move_backward.pass.cpp | 118 +++++++++++++----- 10 files changed, 376 insertions(+), 112 deletions(-) create mode 100644 libcxx/test/benchmarks/algorithms/move.bench.cpp create mode 100644 libcxx/test/benchmarks/algorithms/move_backward.bench.cpp diff --git a/libcxx/docs/ReleaseNotes/21.rst b/libcxx/docs/ReleaseNotes/21.rst index 2439360797023..88a0666611a9a 100644 --- a/libcxx/docs/ReleaseNotes/21.rst +++ b/libcxx/docs/ReleaseNotes/21.rst @@ -43,8 +43,8 @@ Implemented Papers Improvements and New Features ----------------------------- -- The ``std::ranges::{copy, copy_n, copy_backward}`` algorithms have been optimized for ``std::vector::iterator``\s, - resulting in a performance improvement of up to 2000x. +- The ``std::ranges::{copy, copy_n, copy_backward, move, move_backward}`` algorithms have been optimized for + ``std::vector::iterator``, resulting in a performance improvement of up to 2000x. - Updated formatting library to Unicode 16.0.0. diff --git a/libcxx/include/__algorithm/move.h b/libcxx/include/__algorithm/move.h index 6f3b0eb5d2927..a3320e9f1985d 100644 --- a/libcxx/include/__algorithm/move.h +++ b/libcxx/include/__algorithm/move.h @@ -9,11 +9,13 @@ #ifndef _LIBCPP___ALGORITHM_MOVE_H #define _LIBCPP___ALGORITHM_MOVE_H +#include <__algorithm/copy.h> #include <__algorithm/copy_move_common.h> #include <__algorithm/for_each_segment.h> #include <__algorithm/iterator_operations.h> #include <__algorithm/min.h> #include <__config> +#include <__fwd/bit_reference.h> #include <__iterator/iterator_traits.h> #include <__iterator/segmented_iterator.h> #include <__type_traits/common_type.h> @@ -98,6 +100,14 @@ struct __move_impl { } } + template + _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 pair<__bit_iterator<_Cp, _IsConst>, __bit_iterator<_Cp, false> > + operator()(__bit_iterator<_Cp, _IsConst> __first, + __bit_iterator<_Cp, _IsConst> __last, + __bit_iterator<_Cp, false> __result) { + return std::__copy(__first, __last, __result); + } + // At this point, the iterators have been unwrapped so any `contiguous_iterator` has been unwrapped to a pointer. template ::value, int> = 0> _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 pair<_In*, _Out*> diff --git a/libcxx/include/__algorithm/move_backward.h b/libcxx/include/__algorithm/move_backward.h index 24a8d9b24527a..14482fee18114 100644 --- a/libcxx/include/__algorithm/move_backward.h +++ b/libcxx/include/__algorithm/move_backward.h @@ -9,10 +9,12 @@ #ifndef _LIBCPP___ALGORITHM_MOVE_BACKWARD_H #define _LIBCPP___ALGORITHM_MOVE_BACKWARD_H +#include <__algorithm/copy_backward.h> #include <__algorithm/copy_move_common.h> #include <__algorithm/iterator_operations.h> #include <__algorithm/min.h> #include <__config> +#include <__fwd/bit_reference.h> #include <__iterator/iterator_traits.h> #include <__iterator/segmented_iterator.h> #include <__type_traits/common_type.h> @@ -107,6 +109,14 @@ struct __move_backward_impl { } } + template + _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 pair<__bit_iterator<_Cp, _IsConst>, __bit_iterator<_Cp, false> > + operator()(__bit_iterator<_Cp, _IsConst> __first, + __bit_iterator<_Cp, _IsConst> __last, + __bit_iterator<_Cp, false> __result) { + return std::__copy_backward<_ClassicAlgPolicy>(__first, __last, __result); + } + // At this point, the iterators have been unwrapped so any `contiguous_iterator` has been unwrapped to a pointer. template ::value, int> = 0> _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 pair<_In*, _Out*> diff --git a/libcxx/include/__bit_reference b/libcxx/include/__bit_reference index aad470394732c..377f5fed12266 100644 --- a/libcxx/include/__bit_reference +++ b/libcxx/include/__bit_reference @@ -210,22 +210,6 @@ private: __mask_(__m) {} }; -// move - -template -inline _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> -move(__bit_iterator<_Cp, _IsConst> __first, __bit_iterator<_Cp, _IsConst> __last, __bit_iterator<_Cp, false> __result) { - return std::copy(__first, __last, __result); -} - -// move_backward - -template -inline _LIBCPP_HIDE_FROM_ABI __bit_iterator<_Cp, false> move_backward( - __bit_iterator<_Cp, _IsConst> __first, __bit_iterator<_Cp, _IsConst> __last, __bit_iterator<_Cp, false> __result) { - return std::copy_backward(__first, __last, __result); -} - // swap_ranges template diff --git a/libcxx/test/benchmarks/algorithms/move.bench.cpp b/libcxx/test/benchmarks/algorithms/move.bench.cpp new file mode 100644 index 0000000000000..909c0c4f1b4c5 --- /dev/null +++ b/libcxx/test/benchmarks/algorithms/move.bench.cpp @@ -0,0 +1,55 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20 + +#include +#include +#include + +static void bm_ranges_move_vb(benchmark::State& state, bool aligned) { + auto n = state.range(); + std::vector in(n, true); + std::vector out(aligned ? n : n + 8); + benchmark::DoNotOptimize(&in); + auto dst = aligned ? out.begin() : out.begin() + 4; + for (auto _ : state) { + benchmark::DoNotOptimize(std::ranges::move(in, dst)); + benchmark::DoNotOptimize(&out); + } +} + +static void bm_move_vb(benchmark::State& state, bool aligned) { + auto n = state.range(); + std::vector in(n, true); + std::vector out(aligned ? n : n + 8); + benchmark::DoNotOptimize(&in); + auto beg = in.begin(); + auto end = in.end(); + auto dst = aligned ? out.begin() : out.begin() + 4; + for (auto _ : state) { + benchmark::DoNotOptimize(std::move(beg, end, dst)); + benchmark::DoNotOptimize(&out); + } +} + +static void bm_ranges_move_vb_aligned(benchmark::State& state) { bm_ranges_move_vb(state, true); } +static void bm_ranges_move_vb_unaligned(benchmark::State& state) { bm_ranges_move_vb(state, false); } + +static void bm_move_vb_aligned(benchmark::State& state) { bm_move_vb(state, true); } +static void bm_move_vb_unaligned(benchmark::State& state) { bm_move_vb(state, false); } + +// Test std::ranges::move for vector::iterator +BENCHMARK(bm_ranges_move_vb_aligned)->Range(8, 1 << 16)->DenseRange(102400, 204800, 4096); +BENCHMARK(bm_ranges_move_vb_unaligned)->Range(8, 1 << 20); + +// Test std::move for vector::iterator +BENCHMARK(bm_move_vb_aligned)->Range(8, 1 << 20); +BENCHMARK(bm_move_vb_unaligned)->Range(8, 1 << 20); + +BENCHMARK_MAIN(); diff --git a/libcxx/test/benchmarks/algorithms/move_backward.bench.cpp b/libcxx/test/benchmarks/algorithms/move_backward.bench.cpp new file mode 100644 index 0000000000000..48b1a776bf4dd --- /dev/null +++ b/libcxx/test/benchmarks/algorithms/move_backward.bench.cpp @@ -0,0 +1,55 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20 + +#include +#include +#include + +static void bm_ranges_move_backward_vb(benchmark::State& state, bool aligned) { + auto n = state.range(); + std::vector in(n, true); + std::vector out(aligned ? n : n + 8); + benchmark::DoNotOptimize(&in); + auto dst = aligned ? out.end() : out.end() - 4; + for (auto _ : state) { + benchmark::DoNotOptimize(std::ranges::move_backward(in, dst)); + benchmark::DoNotOptimize(&out); + } +} + +static void bm_move_backward(benchmark::State& state, bool aligned) { + auto n = state.range(); + std::vector in(n, true); + std::vector out(aligned ? n : n + 8); + benchmark::DoNotOptimize(&in); + auto beg = in.begin(); + auto end = in.end(); + auto dst = aligned ? out.end() : out.end() - 4; + for (auto _ : state) { + benchmark::DoNotOptimize(std::move_backward(beg, end, dst)); + benchmark::DoNotOptimize(&out); + } +} + +static void bm_ranges_move_backward_vb_aligned(benchmark::State& state) { bm_ranges_move_backward_vb(state, true); } +static void bm_ranges_move_backward_vb_unaligned(benchmark::State& state) { bm_ranges_move_backward_vb(state, false); } + +static void bm_move_backward_vb_aligned(benchmark::State& state) { bm_move_backward(state, true); } +static void bm_move_backward_vb_unaligned(benchmark::State& state) { bm_move_backward(state, false); } + +// Test std::ranges::move_backward for vector::iterator +BENCHMARK(bm_ranges_move_backward_vb_aligned)->Range(8, 1 << 16)->DenseRange(102400, 204800, 4096); +BENCHMARK(bm_ranges_move_backward_vb_unaligned)->Range(8, 1 << 20); + +// Test std::move_backward for vector::iterator +BENCHMARK(bm_move_backward_vb_aligned)->Range(8, 1 << 20); +BENCHMARK(bm_move_backward_vb_unaligned)->Range(8, 1 << 20); + +BENCHMARK_MAIN(); diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/move.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/move.pass.cpp index b1ad6873bc5e5..8b414b061105f 100644 --- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/move.pass.cpp +++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/move.pass.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include "MoveOnly.h" #include "test_iterators.h" @@ -45,15 +46,15 @@ struct Test { template TEST_CONSTEXPR_CXX20 void operator()() { const unsigned N = 1000; - int ia[N] = {}; + int ia[N] = {}; for (unsigned i = 0; i < N; ++i) - ia[i] = i; + ia[i] = i; int ib[N] = {0}; - OutIter r = std::move(InIter(ia), InIter(ia+N), OutIter(ib)); - assert(base(r) == ib+N); + OutIter r = std::move(InIter(ia), InIter(ia + N), OutIter(ib)); + assert(base(r) == ib + N); for (unsigned i = 0; i < N; ++i) - assert(ia[i] == ib[i]); + assert(ia[i] == ib[i]); } }; @@ -73,13 +74,13 @@ struct Test1 { const unsigned N = 100; std::unique_ptr ia[N]; for (unsigned i = 0; i < N; ++i) - ia[i].reset(new int(i)); + ia[i].reset(new int(i)); std::unique_ptr ib[N]; - OutIter r = std::move(InIter(ia), InIter(ia+N), OutIter(ib)); - assert(base(r) == ib+N); + OutIter r = std::move(InIter(ia), InIter(ia + N), OutIter(ib)); + assert(base(r) == ib + N); for (unsigned i = 0; i < N; ++i) - assert(*ib[i] == static_cast(i)); + assert(*ib[i] == static_cast(i)); } }; @@ -92,6 +93,26 @@ struct Test1OutIters { } }; +TEST_CONSTEXPR_CXX20 bool test_vector_bool(std::size_t N) { + std::vector in(N, false); + for (std::size_t i = 0; i < N; i += 2) + in[i] = true; + + { // Test move with aligned bytes + std::vector out(N); + std::move(in.begin(), in.end(), out.begin()); + assert(in == out); + } + { // Test move with unaligned bytes + std::vector out(N + 8); + std::move(in.begin(), in.end(), out.begin() + 4); + for (std::size_t i = 0; i < N; ++i) + assert(out[i + 4] == in[i]); + } + + return true; +} + TEST_CONSTEXPR_CXX20 bool test() { types::for_each(types::cpp17_input_iterator_list(), TestOutIters()); if (TEST_STD_AT_LEAST_23_OR_RUNTIME_EVALUATED) @@ -118,7 +139,7 @@ TEST_CONSTEXPR_CXX20 bool test() { // When non-trivial { MoveOnly from[3] = {1, 2, 3}; - MoveOnly to[3] = {}; + MoveOnly to[3] = {}; std::move(std::begin(from), std::end(from), std::begin(to)); assert(to[0] == MoveOnly(1)); assert(to[1] == MoveOnly(2)); @@ -127,7 +148,7 @@ TEST_CONSTEXPR_CXX20 bool test() { // When trivial { TrivialMoveOnly from[3] = {1, 2, 3}; - TrivialMoveOnly to[3] = {}; + TrivialMoveOnly to[3] = {}; std::move(std::begin(from), std::end(from), std::begin(to)); assert(to[0] == TrivialMoveOnly(1)); assert(to[1] == TrivialMoveOnly(2)); @@ -135,6 +156,16 @@ TEST_CONSTEXPR_CXX20 bool test() { } } + { // Test vector::iterator optimization + assert(test_vector_bool(8)); + assert(test_vector_bool(19)); + assert(test_vector_bool(32)); + assert(test_vector_bool(49)); + assert(test_vector_bool(64)); + assert(test_vector_bool(199)); + assert(test_vector_bool(256)); + } + return true; } diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/move_backward.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/move_backward.pass.cpp index 61dea47b51071..dfee9de2fa768 100644 --- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/move_backward.pass.cpp +++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/move_backward.pass.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include "MoveOnly.h" #include "test_iterators.h" @@ -44,24 +45,22 @@ struct Test { template TEST_CONSTEXPR_CXX20 void operator()() { const unsigned N = 1000; - int ia[N] = {}; + int ia[N] = {}; for (unsigned i = 0; i < N; ++i) - ia[i] = i; + ia[i] = i; int ib[N] = {0}; - OutIter r = std::move_backward(InIter(ia), InIter(ia+N), OutIter(ib+N)); + OutIter r = std::move_backward(InIter(ia), InIter(ia + N), OutIter(ib + N)); assert(base(r) == ib); for (unsigned i = 0; i < N; ++i) - assert(ia[i] == ib[i]); + assert(ia[i] == ib[i]); } }; struct TestOutIters { template TEST_CONSTEXPR_CXX20 void operator()() { - types::for_each( - types::concatenate_t >(), - Test()); + types::for_each(types::concatenate_t >(), Test()); } }; @@ -72,24 +71,44 @@ struct Test1 { const unsigned N = 100; std::unique_ptr ia[N]; for (unsigned i = 0; i < N; ++i) - ia[i].reset(new int(i)); + ia[i].reset(new int(i)); std::unique_ptr ib[N]; - OutIter r = std::move_backward(InIter(ia), InIter(ia+N), OutIter(ib+N)); + OutIter r = std::move_backward(InIter(ia), InIter(ia + N), OutIter(ib + N)); assert(base(r) == ib); for (unsigned i = 0; i < N; ++i) - assert(*ib[i] == static_cast(i)); + assert(*ib[i] == static_cast(i)); } }; struct Test1OutIters { template TEST_CONSTEXPR_CXX23 void operator()() { - types::for_each(types::concatenate_t*> >(), - Test1()); + types::for_each( + types::concatenate_t*> >(), Test1()); } }; +TEST_CONSTEXPR_CXX20 bool test_vector_bool(std::size_t N) { + std::vector in(N, false); + for (std::size_t i = 0; i < N; i += 2) + in[i] = true; + + { // Test move_backward with aligned bytes + std::vector out(N); + std::move_backward(in.begin(), in.end(), out.end()); + assert(in == out); + } + { // Test move_backward with unaligned bytes + std::vector out(N + 8); + std::move_backward(in.begin(), in.end(), out.end() - 4); + for (std::size_t i = 0; i < N; ++i) + assert(out[i + 4] == in[i]); + } + + return true; +} + TEST_CONSTEXPR_CXX20 bool test() { types::for_each(types::bidirectional_iterator_list(), TestOutIters()); if (TEST_STD_AT_LEAST_23_OR_RUNTIME_EVALUATED) @@ -117,7 +136,7 @@ TEST_CONSTEXPR_CXX20 bool test() { // When non-trivial { MoveOnly from[3] = {1, 2, 3}; - MoveOnly to[3] = {}; + MoveOnly to[3] = {}; std::move_backward(std::begin(from), std::end(from), std::end(to)); assert(to[0] == MoveOnly(1)); assert(to[1] == MoveOnly(2)); @@ -126,7 +145,7 @@ TEST_CONSTEXPR_CXX20 bool test() { // When trivial { TrivialMoveOnly from[3] = {1, 2, 3}; - TrivialMoveOnly to[3] = {}; + TrivialMoveOnly to[3] = {}; std::move_backward(std::begin(from), std::end(from), std::end(to)); assert(to[0] == TrivialMoveOnly(1)); assert(to[1] == TrivialMoveOnly(2)); @@ -134,11 +153,20 @@ TEST_CONSTEXPR_CXX20 bool test() { } } + { // Test vector::iterator optimization + assert(test_vector_bool(8)); + assert(test_vector_bool(19)); + assert(test_vector_bool(32)); + assert(test_vector_bool(49)); + assert(test_vector_bool(64)); + assert(test_vector_bool(199)); + assert(test_vector_bool(256)); + } + return true; } -int main(int, char**) -{ +int main(int, char**) { test(); #if TEST_STD_VER >= 20 static_assert(test()); diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move.pass.cpp index a0d1473360a14..664631aea826b 100644 --- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move.pass.cpp +++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move.pass.cpp @@ -31,6 +31,7 @@ #include "almost_satisfies_types.h" #include "MoveOnly.h" #include "test_iterators.h" +#include "test_macros.h" template > concept HasMoveIt = requires(In in, Sent sent, Out out) { std::ranges::move(in, sent, out); }; @@ -65,7 +66,7 @@ constexpr void test(std::array in) { { std::array out; std::same_as> decltype(auto) ret = - std::ranges::move(In(in.data()), Sent(In(in.data() + in.size())), Out(out.data())); + std::ranges::move(In(in.data()), Sent(In(in.data() + in.size())), Out(out.data())); assert(in == out); assert(base(ret.in) == in.data() + in.size()); assert(base(ret.out) == out.data() + out.size()); @@ -73,8 +74,7 @@ constexpr void test(std::array in) { { std::array out; auto range = std::ranges::subrange(In(in.data()), Sent(In(in.data() + in.size()))); - std::same_as> decltype(auto) ret = - std::ranges::move(range, Out(out.data())); + std::same_as> decltype(auto) ret = std::ranges::move(range, Out(out.data())); assert(in == out); assert(base(ret.in) == in.data() + in.size()); assert(base(ret.out) == out.data() + out.size()); @@ -84,16 +84,16 @@ constexpr void test(std::array in) { template constexpr void test_containers() { { - InContainer in {1, 2, 3, 4}; + InContainer in{1, 2, 3, 4}; OutContainer out(4); std::same_as> auto ret = - std::ranges::move(In(in.begin()), Sent(In(in.end())), Out(out.begin())); + std::ranges::move(In(in.begin()), Sent(In(in.end())), Out(out.begin())); assert(std::ranges::equal(in, out)); assert(base(ret.in) == in.end()); assert(base(ret.out) == out.end()); } { - InContainer in {1, 2, 3, 4}; + InContainer in{1, 2, 3, 4}; OutContainer out(4); auto range = std::ranges::subrange(In(in.begin()), Sent(In(in.end()))); std::same_as> auto ret = std::ranges::move(range, Out(out.begin())); @@ -165,22 +165,51 @@ constexpr void test_proxy_in_iterators() { } struct IteratorWithMoveIter { - using value_type = int; - using difference_type = int; + using value_type = int; + using difference_type = int; explicit IteratorWithMoveIter() = default; int* ptr; constexpr IteratorWithMoveIter(int* ptr_) : ptr(ptr_) {} constexpr int& operator*() const; // iterator with iter_move should not be dereferenced - constexpr IteratorWithMoveIter& operator++() { ++ptr; return *this; } - constexpr IteratorWithMoveIter operator++(int) { auto ret = *this; ++*this; return ret; } + constexpr IteratorWithMoveIter& operator++() { + ++ptr; + return *this; + } + constexpr IteratorWithMoveIter operator++(int) { + auto ret = *this; + ++*this; + return ret; + } friend constexpr int iter_move(const IteratorWithMoveIter&) { return 42; } constexpr bool operator==(const IteratorWithMoveIter& other) const = default; }; +#if TEST_STD_VER >= 23 +constexpr bool test_vector_bool(std::size_t N) { + std::vector in(N, false); + for (std::size_t i = 0; i < N; i += 2) + in[i] = true; + + { // Test move with aligned bytes + std::vector out(N); + std::ranges::move(in, out.begin()); + assert(in == out); + } + { // Test move with unaligned bytes + std::vector out(N + 8); + std::ranges::move(in, out.begin() + 4); + for (std::size_t i = 0; i < N; ++i) + assert(out[i + 4] == in[i]); + } + + return true; +} +#endif + // cpp17_intput_iterator has a defaulted template argument template using Cpp17InIter = cpp17_input_iterator; @@ -267,13 +296,13 @@ constexpr bool test() { { // check that ranges::dangling is returned std::array out; std::same_as> decltype(auto) ret = - std::ranges::move(std::array {1, 2, 3, 4}, out.data()); + std::ranges::move(std::array{1, 2, 3, 4}, out.data()); assert(ret.out == out.data() + 4); assert((out == std::array{1, 2, 3, 4})); } { // check that an iterator is returned with a borrowing range - std::array in {1, 2, 3, 4}; + std::array in{1, 2, 3, 4}; std::array out; std::same_as::iterator, int*>> decltype(auto) ret = std::ranges::move(std::views::all(in), out.data()); @@ -284,8 +313,8 @@ constexpr bool test() { { // check that every element is moved exactly once struct MoveOnce { - bool moved = false; - constexpr MoveOnce() = default; + bool moved = false; + constexpr MoveOnce() = default; constexpr MoveOnce(const MoveOnce& other) = delete; constexpr MoveOnce& operator=(MoveOnce&& other) { assert(!other.moved); @@ -294,16 +323,16 @@ constexpr bool test() { } }; { - std::array in {}; - std::array out {}; + std::array in{}; + std::array out{}; auto ret = std::ranges::move(in.begin(), in.end(), out.begin()); assert(ret.in == in.end()); assert(ret.out == out.end()); assert(std::all_of(out.begin(), out.end(), [](const auto& e) { return e.moved; })); } { - std::array in {}; - std::array out {}; + std::array in{}; + std::array out{}; auto ret = std::ranges::move(in, out.begin()); assert(ret.in == in.end()); assert(ret.out == out.end()); @@ -314,8 +343,8 @@ constexpr bool test() { { // check that the range is moved forwards struct OnlyForwardsMovable { OnlyForwardsMovable* next = nullptr; - bool canMove = false; - OnlyForwardsMovable() = default; + bool canMove = false; + OnlyForwardsMovable() = default; constexpr OnlyForwardsMovable& operator=(OnlyForwardsMovable&&) { assert(canMove); if (next != nullptr) @@ -324,12 +353,12 @@ constexpr bool test() { } }; { - std::array in {}; - std::array out {}; - out[0].next = &out[1]; - out[1].next = &out[2]; + std::array in{}; + std::array out{}; + out[0].next = &out[1]; + out[1].next = &out[2]; out[0].canMove = true; - auto ret = std::ranges::move(in.begin(), in.end(), out.begin()); + auto ret = std::ranges::move(in.begin(), in.end(), out.begin()); assert(ret.in == in.end()); assert(ret.out == out.end()); assert(out[0].canMove); @@ -337,12 +366,12 @@ constexpr bool test() { assert(out[2].canMove); } { - std::array in {}; - std::array out {}; - out[0].next = &out[1]; - out[1].next = &out[2]; + std::array in{}; + std::array out{}; + out[0].next = &out[1]; + out[1].next = &out[2]; out[0].canMove = true; - auto ret = std::ranges::move(in, out.begin()); + auto ret = std::ranges::move(in, out.begin()); assert(ret.in == in.end()); assert(ret.out == out.end()); assert(out[0].canMove); @@ -358,19 +387,31 @@ constexpr bool test() { auto ret = std::ranges::move(IteratorWithMoveIter(a), IteratorWithMoveIter(a + 4), b.data()); assert(ret.in == a + 4); assert(ret.out == b.data() + 4); - assert((b == std::array {42, 42, 42, 42})); + assert((b == std::array{42, 42, 42, 42})); } { int a[] = {1, 2, 3, 4}; std::array b; auto range = std::ranges::subrange(IteratorWithMoveIter(a), IteratorWithMoveIter(a + 4)); - auto ret = std::ranges::move(range, b.data()); + auto ret = std::ranges::move(range, b.data()); assert(ret.in == a + 4); assert(ret.out == b.data() + 4); - assert((b == std::array {42, 42, 42, 42})); + assert((b == std::array{42, 42, 42, 42})); } } +#if TEST_STD_VER >= 23 + { // Test vector::iterator optimization + assert(test_vector_bool(8)); + assert(test_vector_bool(19)); + assert(test_vector_bool(32)); + assert(test_vector_bool(49)); + assert(test_vector_bool(64)); + assert(test_vector_bool(199)); + assert(test_vector_bool(256)); + } +#endif + return true; } diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move_backward.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move_backward.pass.cpp index 47cf178636ad1..a12ea8665f793 100644 --- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move_backward.pass.cpp +++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move_backward.pass.cpp @@ -31,6 +31,7 @@ #include "almost_satisfies_types.h" #include "MoveOnly.h" #include "test_iterators.h" +#include "test_macros.h" template > concept HasMoveBackwardIt = requires(In in, Sent sent, Out out) { std::ranges::move_backward(in, sent, out); }; @@ -65,7 +66,7 @@ constexpr void test(std::array in) { { std::array out; std::same_as> decltype(auto) ret = - std::ranges::move_backward(In(in.data()), Sent(In(in.data() + in.size())), Out(out.data() + out.size())); + std::ranges::move_backward(In(in.data()), Sent(In(in.data() + in.size())), Out(out.data() + out.size())); assert(in == out); assert(base(ret.in) == in.data() + in.size()); assert(base(ret.out) == out.data()); @@ -92,16 +93,16 @@ constexpr void test_iterators() { template constexpr void test_containers() { { - InContainer in {1, 2, 3, 4}; + InContainer in{1, 2, 3, 4}; OutContainer out(4); std::same_as> auto ret = - std::ranges::move_backward(In(in.begin()), Sent(In(in.end())), Out(out.end())); + std::ranges::move_backward(In(in.begin()), Sent(In(in.end())), Out(out.end())); assert(std::ranges::equal(in, out)); assert(base(ret.in) == in.end()); assert(base(ret.out) == out.begin()); } { - InContainer in {1, 2, 3, 4}; + InContainer in{1, 2, 3, 4}; OutContainer out(4); auto range = std::ranges::subrange(In(in.begin()), Sent(In(in.end()))); std::same_as> auto ret = std::ranges::move_backward(range, Out(out.end())); @@ -159,25 +160,61 @@ constexpr void test_proxy_in_iterators() { } struct IteratorWithMoveIter { - using value_type = int; - using difference_type = int; + using value_type = int; + using difference_type = int; explicit IteratorWithMoveIter() = default; int* ptr; constexpr IteratorWithMoveIter(int* ptr_) : ptr(ptr_) {} constexpr int& operator*() const; // iterator with iter_move should not be dereferenced - constexpr IteratorWithMoveIter& operator++() { ++ptr; return *this; } - constexpr IteratorWithMoveIter operator++(int) { auto ret = *this; ++*this; return ret; } + constexpr IteratorWithMoveIter& operator++() { + ++ptr; + return *this; + } + constexpr IteratorWithMoveIter operator++(int) { + auto ret = *this; + ++*this; + return ret; + } - constexpr IteratorWithMoveIter& operator--() { --ptr; return *this; } - constexpr IteratorWithMoveIter operator--(int) { auto ret = *this; --*this; return ret; } + constexpr IteratorWithMoveIter& operator--() { + --ptr; + return *this; + } + constexpr IteratorWithMoveIter operator--(int) { + auto ret = *this; + --*this; + return ret; + } friend constexpr int iter_move(const IteratorWithMoveIter&) { return 42; } constexpr bool operator==(const IteratorWithMoveIter& other) const = default; }; +#if TEST_STD_VER >= 23 +constexpr bool test_vector_bool(std::size_t N) { + std::vector in(N, false); + for (std::size_t i = 0; i < N; i += 2) + in[i] = true; + + { // Test move_backward with aligned bytes + std::vector out(N); + std::ranges::move_backward(in, out.end()); + assert(in == out); + } + { // Test move_backward with unaligned bytes + std::vector out(N + 8); + std::ranges::move_backward(in, out.end() - 4); + for (std::size_t i = 0; i < N; ++i) + assert(out[i + 4] == in[i]); + } + + return true; +} +#endif + constexpr bool test() { test_in_iterators(); test_in_iterators(); @@ -243,7 +280,8 @@ constexpr bool test() { MoveOnly b[3]; ProxyRange proxyA{a}; ProxyRange proxyB{b}; - std::ranges::move_backward(std::begin(proxyA), std::end(proxyA), std::ranges::next(proxyB.begin(), std::end(proxyB))); + std::ranges::move_backward( + std::begin(proxyA), std::end(proxyA), std::ranges::next(proxyB.begin(), std::end(proxyB))); assert(b[0].get() == 1); assert(b[1].get() == 2); assert(b[2].get() == 3); @@ -253,13 +291,13 @@ constexpr bool test() { { // check that ranges::dangling is returned std::array out; std::same_as> auto ret = - std::ranges::move_backward(std::array {1, 2, 3, 4}, out.data() + out.size()); + std::ranges::move_backward(std::array{1, 2, 3, 4}, out.data() + out.size()); assert(ret.out == out.data()); assert((out == std::array{1, 2, 3, 4})); } { // check that an iterator is returned with a borrowing range - std::array in {1, 2, 3, 4}; + std::array in{1, 2, 3, 4}; std::array out; std::same_as::iterator, int*>> auto ret = std::ranges::move_backward(std::views::all(in), out.data() + out.size()); @@ -270,8 +308,8 @@ constexpr bool test() { { // check that every element is moved exactly once struct MoveOnce { - bool moved = false; - constexpr MoveOnce() = default; + bool moved = false; + constexpr MoveOnce() = default; constexpr MoveOnce(const MoveOnce& other) = delete; constexpr MoveOnce& operator=(const MoveOnce& other) { assert(!other.moved); @@ -280,16 +318,16 @@ constexpr bool test() { } }; { - std::array in {}; - std::array out {}; + std::array in{}; + std::array out{}; auto ret = std::ranges::move_backward(in.begin(), in.end(), out.end()); assert(ret.in == in.end()); assert(ret.out == out.begin()); assert(std::all_of(out.begin(), out.end(), [](const auto& e) { return e.moved; })); } { - std::array in {}; - std::array out {}; + std::array in{}; + std::array out{}; auto ret = std::ranges::move_backward(in, out.end()); assert(ret.in == in.end()); assert(ret.out == out.begin()); @@ -300,8 +338,8 @@ constexpr bool test() { { // check that the range is moved backwards struct OnlyBackwardsMovable { OnlyBackwardsMovable* next = nullptr; - bool canMove = false; - OnlyBackwardsMovable() = default; + bool canMove = false; + OnlyBackwardsMovable() = default; constexpr OnlyBackwardsMovable& operator=(const OnlyBackwardsMovable&) { assert(canMove); if (next != nullptr) @@ -310,12 +348,12 @@ constexpr bool test() { } }; { - std::array in {}; - std::array out {}; - out[1].next = &out[0]; - out[2].next = &out[1]; + std::array in{}; + std::array out{}; + out[1].next = &out[0]; + out[2].next = &out[1]; out[2].canMove = true; - auto ret = std::ranges::move_backward(in, out.end()); + auto ret = std::ranges::move_backward(in, out.end()); assert(ret.in == in.end()); assert(ret.out == out.begin()); assert(out[0].canMove); @@ -323,12 +361,12 @@ constexpr bool test() { assert(out[2].canMove); } { - std::array in {}; - std::array out {}; - out[1].next = &out[0]; - out[2].next = &out[1]; + std::array in{}; + std::array out{}; + out[1].next = &out[0]; + out[2].next = &out[1]; out[2].canMove = true; - auto ret = std::ranges::move_backward(in.begin(), in.end(), out.end()); + auto ret = std::ranges::move_backward(in.begin(), in.end(), out.end()); assert(ret.in == in.end()); assert(ret.out == out.begin()); assert(out[0].canMove); @@ -344,19 +382,31 @@ constexpr bool test() { auto ret = std::ranges::move_backward(IteratorWithMoveIter(a), IteratorWithMoveIter(a + 4), b.data() + b.size()); assert(ret.in == a + 4); assert(ret.out == b.data()); - assert((b == std::array {42, 42, 42, 42})); + assert((b == std::array{42, 42, 42, 42})); } { int a[] = {1, 2, 3, 4}; std::array b; auto range = std::ranges::subrange(IteratorWithMoveIter(a), IteratorWithMoveIter(a + 4)); - auto ret = std::ranges::move_backward(range, b.data() + b.size()); + auto ret = std::ranges::move_backward(range, b.data() + b.size()); assert(ret.in == a + 4); assert(ret.out == b.data()); - assert((b == std::array {42, 42, 42, 42})); + assert((b == std::array{42, 42, 42, 42})); } } +#if TEST_STD_VER >= 23 + { // Test vector::iterator optimization + assert(test_vector_bool(8)); + assert(test_vector_bool(19)); + assert(test_vector_bool(32)); + assert(test_vector_bool(49)); + assert(test_vector_bool(64)); + assert(test_vector_bool(199)); + assert(test_vector_bool(256)); + } +#endif + return true; } From f5074662e013ba1bdd03b78dd9ea6d8ef477f4d4 Mon Sep 17 00:00:00 2001 From: Peng Liu Date: Thu, 6 Feb 2025 09:37:08 -0500 Subject: [PATCH 2/3] Rewrite benchmarks and tests to avoid moving/reading a already-moved vector --- .../test/benchmarks/algorithms/move.bench.cpp | 71 +++++++++++-------- .../algorithms/move_backward.bench.cpp | 71 +++++++++++-------- .../alg.move/move.pass.cpp | 16 +++-- .../alg.move/move_backward.pass.cpp | 16 +++-- .../alg.move/ranges.move.pass.cpp | 15 ++-- .../alg.move/ranges.move_backward.pass.cpp | 15 ++-- 6 files changed, 120 insertions(+), 84 deletions(-) diff --git a/libcxx/test/benchmarks/algorithms/move.bench.cpp b/libcxx/test/benchmarks/algorithms/move.bench.cpp index 909c0c4f1b4c5..10853057abb49 100644 --- a/libcxx/test/benchmarks/algorithms/move.bench.cpp +++ b/libcxx/test/benchmarks/algorithms/move.bench.cpp @@ -10,46 +10,61 @@ #include #include +#include #include -static void bm_ranges_move_vb(benchmark::State& state, bool aligned) { +template +void bm_ranges_move_vb(benchmark::State& state) { auto n = state.range(); - std::vector in(n, true); - std::vector out(aligned ? n : n + 8); - benchmark::DoNotOptimize(&in); - auto dst = aligned ? out.begin() : out.begin() + 4; + std::vector v1(n, true); + std::vector v2(n, false); + benchmark::DoNotOptimize(v1); + benchmark::DoNotOptimize(v2); + std::vector* in = &v1; + std::vector* out = &v2; for (auto _ : state) { - benchmark::DoNotOptimize(std::ranges::move(in, dst)); - benchmark::DoNotOptimize(&out); + if constexpr (aligned) { + benchmark::DoNotOptimize(std::ranges::move(*in, std::ranges::begin(*out))); + } else { + benchmark::DoNotOptimize(std::ranges::move(*in | std::views::drop(4), std::ranges::begin(*out))); + } + std::swap(in, out); + benchmark::DoNotOptimize(in); + benchmark::DoNotOptimize(out); } } -static void bm_move_vb(benchmark::State& state, bool aligned) { +template +void bm_move_vb(benchmark::State& state) { auto n = state.range(); - std::vector in(n, true); - std::vector out(aligned ? n : n + 8); - benchmark::DoNotOptimize(&in); - auto beg = in.begin(); - auto end = in.end(); - auto dst = aligned ? out.begin() : out.begin() + 4; + std::vector v1(n, true); + std::vector v2(n, false); + benchmark::DoNotOptimize(v1); + benchmark::DoNotOptimize(v2); + std::vector* in = &v1; + std::vector* out = &v2; for (auto _ : state) { - benchmark::DoNotOptimize(std::move(beg, end, dst)); - benchmark::DoNotOptimize(&out); + auto first1 = in->begin(); + auto last1 = in->end(); + auto first2 = out->begin(); + if constexpr (aligned) { + benchmark::DoNotOptimize(std::move(first1, last1, first2)); + } else { + benchmark::DoNotOptimize(std::move(first1 + 4, last1, first2)); + } + std::swap(in, out); + benchmark::DoNotOptimize(in); + benchmark::DoNotOptimize(out); } } -static void bm_ranges_move_vb_aligned(benchmark::State& state) { bm_ranges_move_vb(state, true); } -static void bm_ranges_move_vb_unaligned(benchmark::State& state) { bm_ranges_move_vb(state, false); } +BENCHMARK(bm_ranges_move_vb) + ->Name("bm_ranges_move_vb_aligned") + ->Range(8, 1 << 16) + ->DenseRange(102400, 204800, 4096); +BENCHMARK(bm_ranges_move_vb)->Name("bm_ranges_move_vb_unaligned")->Range(8, 1 << 20); -static void bm_move_vb_aligned(benchmark::State& state) { bm_move_vb(state, true); } -static void bm_move_vb_unaligned(benchmark::State& state) { bm_move_vb(state, false); } - -// Test std::ranges::move for vector::iterator -BENCHMARK(bm_ranges_move_vb_aligned)->Range(8, 1 << 16)->DenseRange(102400, 204800, 4096); -BENCHMARK(bm_ranges_move_vb_unaligned)->Range(8, 1 << 20); - -// Test std::move for vector::iterator -BENCHMARK(bm_move_vb_aligned)->Range(8, 1 << 20); -BENCHMARK(bm_move_vb_unaligned)->Range(8, 1 << 20); +BENCHMARK(bm_move_vb)->Name("bm_move_vb_aligned")->Range(8, 1 << 20); +BENCHMARK(bm_move_vb)->Name("bm_move_vb_unaligned")->Range(8, 1 << 20); BENCHMARK_MAIN(); diff --git a/libcxx/test/benchmarks/algorithms/move_backward.bench.cpp b/libcxx/test/benchmarks/algorithms/move_backward.bench.cpp index 48b1a776bf4dd..138d2c685e1c8 100644 --- a/libcxx/test/benchmarks/algorithms/move_backward.bench.cpp +++ b/libcxx/test/benchmarks/algorithms/move_backward.bench.cpp @@ -10,46 +10,61 @@ #include #include +#include #include -static void bm_ranges_move_backward_vb(benchmark::State& state, bool aligned) { +template +void bm_ranges_move_backward_vb(benchmark::State& state) { auto n = state.range(); - std::vector in(n, true); - std::vector out(aligned ? n : n + 8); - benchmark::DoNotOptimize(&in); - auto dst = aligned ? out.end() : out.end() - 4; + std::vector v1(n, true); + std::vector v2(n, false); + benchmark::DoNotOptimize(v1); + benchmark::DoNotOptimize(v2); + std::vector* in = &v1; + std::vector* out = &v2; for (auto _ : state) { - benchmark::DoNotOptimize(std::ranges::move_backward(in, dst)); - benchmark::DoNotOptimize(&out); + if constexpr (aligned) { + benchmark::DoNotOptimize(std::ranges::move_backward(*in, std::ranges::end(*out))); + } else { + benchmark::DoNotOptimize(std::ranges::move_backward(*in | std::views::take(n - 4), std::ranges::end(*out))); + } + std::swap(in, out); + benchmark::DoNotOptimize(in); + benchmark::DoNotOptimize(out); } } -static void bm_move_backward(benchmark::State& state, bool aligned) { +template +void bm_move_backward_vb(benchmark::State& state) { auto n = state.range(); - std::vector in(n, true); - std::vector out(aligned ? n : n + 8); - benchmark::DoNotOptimize(&in); - auto beg = in.begin(); - auto end = in.end(); - auto dst = aligned ? out.end() : out.end() - 4; + std::vector v1(n, true); + std::vector v2(n, false); + benchmark::DoNotOptimize(v1); + benchmark::DoNotOptimize(v2); + std::vector* in = &v1; + std::vector* out = &v2; for (auto _ : state) { - benchmark::DoNotOptimize(std::move_backward(beg, end, dst)); - benchmark::DoNotOptimize(&out); + auto first1 = in->begin(); + auto last1 = in->end(); + auto last2 = out->end(); + if constexpr (aligned) { + benchmark::DoNotOptimize(std::move_backward(first1, last1, last2)); + } else { + benchmark::DoNotOptimize(std::move_backward(first1, last1 - 4, last2)); + } + std::swap(in, out); + benchmark::DoNotOptimize(in); + benchmark::DoNotOptimize(out); } } -static void bm_ranges_move_backward_vb_aligned(benchmark::State& state) { bm_ranges_move_backward_vb(state, true); } -static void bm_ranges_move_backward_vb_unaligned(benchmark::State& state) { bm_ranges_move_backward_vb(state, false); } +BENCHMARK(bm_ranges_move_backward_vb) + ->Name("bm_ranges_move_backward_vb_aligned") + ->Range(8, 1 << 16) + ->DenseRange(102400, 204800, 4096); +BENCHMARK(bm_ranges_move_backward_vb)->Name("bm_ranges_move_backward_vb_unaligned")->Range(8, 1 << 20); -static void bm_move_backward_vb_aligned(benchmark::State& state) { bm_move_backward(state, true); } -static void bm_move_backward_vb_unaligned(benchmark::State& state) { bm_move_backward(state, false); } - -// Test std::ranges::move_backward for vector::iterator -BENCHMARK(bm_ranges_move_backward_vb_aligned)->Range(8, 1 << 16)->DenseRange(102400, 204800, 4096); -BENCHMARK(bm_ranges_move_backward_vb_unaligned)->Range(8, 1 << 20); - -// Test std::move_backward for vector::iterator -BENCHMARK(bm_move_backward_vb_aligned)->Range(8, 1 << 20); -BENCHMARK(bm_move_backward_vb_unaligned)->Range(8, 1 << 20); +BENCHMARK(bm_move_backward_vb)->Name("bm_move_backward_vb_aligned")->Range(8, 1 << 20); +BENCHMARK(bm_move_backward_vb)->Name("bm_move_backward_vb_unaligned")->Range(8, 1 << 20); BENCHMARK_MAIN(); diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/move.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/move.pass.cpp index 8b414b061105f..1afaa1a7e6da1 100644 --- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/move.pass.cpp +++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/move.pass.cpp @@ -94,20 +94,22 @@ struct Test1OutIters { }; TEST_CONSTEXPR_CXX20 bool test_vector_bool(std::size_t N) { - std::vector in(N, false); + std::vector v(N, false); for (std::size_t i = 0; i < N; i += 2) - in[i] = true; + v[i] = true; { // Test move with aligned bytes + std::vector in(v); std::vector out(N); std::move(in.begin(), in.end(), out.begin()); - assert(in == out); + assert(out == v); } { // Test move with unaligned bytes - std::vector out(N + 8); - std::move(in.begin(), in.end(), out.begin() + 4); - for (std::size_t i = 0; i < N; ++i) - assert(out[i + 4] == in[i]); + std::vector in(v); + std::vector out(N); + std::move(in.begin() + 4, in.end(), out.begin()); + for (std::size_t i = 0; i < N - 4; ++i) + assert(v[i + 4] == out[i]); } return true; diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/move_backward.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/move_backward.pass.cpp index dfee9de2fa768..3c0fcadb2d036 100644 --- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/move_backward.pass.cpp +++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/move_backward.pass.cpp @@ -90,20 +90,22 @@ struct Test1OutIters { }; TEST_CONSTEXPR_CXX20 bool test_vector_bool(std::size_t N) { - std::vector in(N, false); + std::vector v(N, false); for (std::size_t i = 0; i < N; i += 2) - in[i] = true; + v[i] = true; { // Test move_backward with aligned bytes + std::vector in(v); std::vector out(N); std::move_backward(in.begin(), in.end(), out.end()); - assert(in == out); + assert(out == v); } { // Test move_backward with unaligned bytes - std::vector out(N + 8); - std::move_backward(in.begin(), in.end(), out.end() - 4); - for (std::size_t i = 0; i < N; ++i) - assert(out[i + 4] == in[i]); + std::vector in(v); + std::vector out(N); + std::move_backward(in.begin(), in.end() - 4, out.end()); + for (std::size_t i = 0; i < N - 4; ++i) + assert(out[i + 4] == v[i]); } return true; diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move.pass.cpp index 664631aea826b..4a0d90abd7f1f 100644 --- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move.pass.cpp +++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move.pass.cpp @@ -190,20 +190,21 @@ struct IteratorWithMoveIter { #if TEST_STD_VER >= 23 constexpr bool test_vector_bool(std::size_t N) { - std::vector in(N, false); + std::vector v(N, false); for (std::size_t i = 0; i < N; i += 2) - in[i] = true; + v[i] = true; { // Test move with aligned bytes + std::vector in{v}; std::vector out(N); std::ranges::move(in, out.begin()); - assert(in == out); + assert(out == v); } { // Test move with unaligned bytes - std::vector out(N + 8); - std::ranges::move(in, out.begin() + 4); - for (std::size_t i = 0; i < N; ++i) - assert(out[i + 4] == in[i]); + std::vector in{v}; + std::vector out(N); + std::ranges::move(in | std::views::drop(4), out.begin()); + assert(std::ranges::equal(v | std::views::drop(4), out | std::views::take(N - 4))); } return true; diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move_backward.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move_backward.pass.cpp index a12ea8665f793..e89430254a281 100644 --- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move_backward.pass.cpp +++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move_backward.pass.cpp @@ -195,20 +195,21 @@ struct IteratorWithMoveIter { #if TEST_STD_VER >= 23 constexpr bool test_vector_bool(std::size_t N) { - std::vector in(N, false); + std::vector v(N, false); for (std::size_t i = 0; i < N; i += 2) - in[i] = true; + v[i] = true; { // Test move_backward with aligned bytes + std::vector in{v}; std::vector out(N); std::ranges::move_backward(in, out.end()); - assert(in == out); + assert(out == v); } { // Test move_backward with unaligned bytes - std::vector out(N + 8); - std::ranges::move_backward(in, out.end() - 4); - for (std::size_t i = 0; i < N; ++i) - assert(out[i + 4] == in[i]); + std::vector in{v}; + std::vector out(N); + std::ranges::move_backward(in | std::views::take(N - 4), out.end()); + assert(std::ranges::equal(v | std::views::take(N - 4), out | std::views::drop(4))); } return true; From f472c55b06cf31a058bcfee401a52afb963efef2 Mon Sep 17 00:00:00 2001 From: Peng Liu Date: Tue, 11 Feb 2025 15:47:17 -0500 Subject: [PATCH 3/3] Use subrange instead of drop_view --- libcxx/test/benchmarks/algorithms/move.bench.cpp | 3 ++- libcxx/test/benchmarks/algorithms/move_backward.bench.cpp | 3 ++- .../alg.modifying.operations/alg.move/ranges.move.pass.cpp | 2 +- .../alg.move/ranges.move_backward.pass.cpp | 2 +- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/libcxx/test/benchmarks/algorithms/move.bench.cpp b/libcxx/test/benchmarks/algorithms/move.bench.cpp index 10853057abb49..73f36f0c129de 100644 --- a/libcxx/test/benchmarks/algorithms/move.bench.cpp +++ b/libcxx/test/benchmarks/algorithms/move.bench.cpp @@ -26,7 +26,8 @@ void bm_ranges_move_vb(benchmark::State& state) { if constexpr (aligned) { benchmark::DoNotOptimize(std::ranges::move(*in, std::ranges::begin(*out))); } else { - benchmark::DoNotOptimize(std::ranges::move(*in | std::views::drop(4), std::ranges::begin(*out))); + benchmark::DoNotOptimize( + std::ranges::move(std::views::counted(in->begin() + 4, n - 4), std::ranges::begin(*out))); } std::swap(in, out); benchmark::DoNotOptimize(in); diff --git a/libcxx/test/benchmarks/algorithms/move_backward.bench.cpp b/libcxx/test/benchmarks/algorithms/move_backward.bench.cpp index 138d2c685e1c8..23d7395198419 100644 --- a/libcxx/test/benchmarks/algorithms/move_backward.bench.cpp +++ b/libcxx/test/benchmarks/algorithms/move_backward.bench.cpp @@ -26,7 +26,8 @@ void bm_ranges_move_backward_vb(benchmark::State& state) { if constexpr (aligned) { benchmark::DoNotOptimize(std::ranges::move_backward(*in, std::ranges::end(*out))); } else { - benchmark::DoNotOptimize(std::ranges::move_backward(*in | std::views::take(n - 4), std::ranges::end(*out))); + benchmark::DoNotOptimize( + std::ranges::move_backward(std::views::counted(in->begin(), n - 4), std::ranges::end(*out))); } std::swap(in, out); benchmark::DoNotOptimize(in); diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move.pass.cpp index 4a0d90abd7f1f..1a89408865892 100644 --- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move.pass.cpp +++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move.pass.cpp @@ -203,7 +203,7 @@ constexpr bool test_vector_bool(std::size_t N) { { // Test move with unaligned bytes std::vector in{v}; std::vector out(N); - std::ranges::move(in | std::views::drop(4), out.begin()); + std::ranges::move(std::views::counted(in.begin() + 4, N - 4), out.begin()); assert(std::ranges::equal(v | std::views::drop(4), out | std::views::take(N - 4))); } diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move_backward.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move_backward.pass.cpp index e89430254a281..923b4c790dd1d 100644 --- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move_backward.pass.cpp +++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/ranges.move_backward.pass.cpp @@ -208,7 +208,7 @@ constexpr bool test_vector_bool(std::size_t N) { { // Test move_backward with unaligned bytes std::vector in{v}; std::vector out(N); - std::ranges::move_backward(in | std::views::take(N - 4), out.end()); + std::ranges::move_backward(std::views::counted(in.begin(), N - 4), out.end()); assert(std::ranges::equal(v | std::views::take(N - 4), out | std::views::drop(4))); }