Skip to content

Commit 2867841

Browse files
Fix uninitialized_meow memcpy optimization (#1548)
Co-authored-by: Stephan T. Lavavej <[email protected]>
1 parent 188c35a commit 2867841

File tree

7 files changed

+423
-82
lines changed

7 files changed

+423
-82
lines changed

stl/inc/memory

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,9 @@ namespace ranges {
149149
auto _OFirst = _Get_unwrapped(_STD move(_First2));
150150
const auto _OLast = _Get_unwrapped(_STD move(_Last2));
151151
if constexpr (_Ptr_copy_cat<_It, _Out>::_Really_trivial) {
152-
_OFirst = _Copy_memcpy_common(_IFirst, _IFirst + _Count, _OFirst, _OLast);
152+
auto _UResult = _Copy_memcpy_common(_IFirst, _IFirst + _Count, _OFirst, _OLast);
153+
_IFirst = _UResult.in;
154+
_OFirst = _UResult.out;
153155
} else {
154156
_Uninitialized_backout _Backout{_STD move(_OFirst)};
155157

@@ -283,7 +285,9 @@ namespace ranges {
283285
auto _OFirst = _Get_unwrapped(_STD move(_First2));
284286
const auto _OLast = _Get_unwrapped(_STD move(_Last2));
285287
if constexpr (_Ptr_move_cat<_It, _Out>::_Really_trivial) {
286-
_OFirst = _Copy_memcpy_common(_IFirst, _IFirst + _Count, _OFirst, _OLast);
288+
auto _UResult = _Copy_memcpy_common(_IFirst, _IFirst + _Count, _OFirst, _OLast);
289+
_IFirst = _UResult.in;
290+
_OFirst = _UResult.out;
287291
} else {
288292
_Uninitialized_backout _Backout{_STD move(_OFirst)};
289293

stl/inc/xmemory

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1551,6 +1551,18 @@ namespace ranges {
15511551
&& _No_throw_forward_iterator<iterator_t<_Rng>>;
15521552
// clang-format on
15531553

1554+
template <class _InIt, class _OutIt>
1555+
in_out_result<_InIt, _OutIt> _Copy_memcpy_common(
1556+
_InIt _IFirst, _InIt _ILast, _OutIt _OFirst, _OutIt _OLast) noexcept {
1557+
const auto _IFirst_ch = const_cast<char*>(reinterpret_cast<const volatile char*>(_IFirst));
1558+
const auto _ILast_ch = const_cast<const char*>(reinterpret_cast<const volatile char*>(_ILast));
1559+
const auto _OFirst_ch = const_cast<char*>(reinterpret_cast<volatile char*>(_OFirst));
1560+
const auto _OLast_ch = const_cast<const char*>(reinterpret_cast<const volatile char*>(_OLast));
1561+
const auto _Count = static_cast<size_t>((_STD min)(_ILast_ch - _IFirst_ch, _OLast_ch - _OFirst_ch));
1562+
_CSTD memcpy(_OFirst_ch, _IFirst_ch, _Count);
1563+
return {reinterpret_cast<_InIt>(_IFirst_ch + _Count), reinterpret_cast<_OutIt>(_OFirst_ch + _Count)};
1564+
}
1565+
15541566
// ALIAS TEMPLATE uninitialized_move_result
15551567
template <class _In, class _Out>
15561568
using uninitialized_move_result = in_out_result<_In, _Out>;

stl/inc/xutility

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4102,17 +4102,6 @@ _OutIt _Copy_memmove(move_iterator<_InIt> _First, move_iterator<_InIt> _Last, _O
41024102
return _Copy_memmove(_First.base(), _Last.base(), _Dest);
41034103
}
41044104

4105-
template <class _InIt, class _OutIt>
4106-
_OutIt _Copy_memcpy_common(_InIt _IFirst, _InIt _ILast, _OutIt _OFirst, _OutIt _OLast) noexcept {
4107-
const auto _IFirst_ch = const_cast<const char*>(reinterpret_cast<const volatile char*>(_IFirst));
4108-
const auto _ILast_ch = const_cast<const char*>(reinterpret_cast<const volatile char*>(_ILast));
4109-
const auto _OFirst_ch = const_cast<char*>(reinterpret_cast<volatile char*>(_OFirst));
4110-
const auto _OLast_ch = const_cast<const char*>(reinterpret_cast<const volatile char*>(_OLast));
4111-
const auto _Count = static_cast<size_t>((_STD min)(_ILast_ch - _IFirst_ch, _OLast_ch - _OFirst_ch));
4112-
_CSTD memcpy(_OFirst_ch, _IFirst_ch, _Count);
4113-
return reinterpret_cast<_OutIt>(_OFirst_ch + _Count);
4114-
}
4115-
41164105
// VARIABLE TEMPLATE _Is_vb_iterator
41174106
template <class _It, bool _RequiresMutable = false>
41184107
_INLINE_VAR constexpr bool _Is_vb_iterator = false;

tests/std/tests/P0896R4_ranges_alg_uninitialized_copy/test.cpp

Lines changed: 92 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -76,21 +76,16 @@ struct holder {
7676
}
7777
};
7878

79-
template <class R>
80-
void not_ranges_destroy(R&& r) { // TRANSITION, ranges::destroy
81-
for (auto& e : r) {
82-
destroy_at(&e);
83-
}
84-
}
85-
8679
struct instantiator {
87-
static constexpr int expected_output[] = {13, 55, 12345};
88-
static constexpr int expected_input[] = {13, 55, 12345};
80+
static constexpr int expected_output[] = {13, 55, 12345};
81+
static constexpr int expected_output_long[] = {13, 55, 12345, -1};
82+
static constexpr int expected_input[] = {13, 55, 12345};
83+
static constexpr int expected_input_long[] = {13, 55, 12345, 42};
8984

9085
template <ranges::input_range R, ranges::forward_range W>
9186
static void call() {
92-
using ranges::uninitialized_copy, ranges::uninitialized_copy_result, ranges::equal, ranges::equal_to,
93-
ranges::iterator_t;
87+
using ranges::destroy, ranges::uninitialized_copy, ranges::uninitialized_copy_result, ranges::equal,
88+
ranges::equal_to, ranges::iterator_t;
9489

9590
{ // Validate range overload
9691
int_wrapper input[3] = {13, 55, 12345};
@@ -107,7 +102,7 @@ struct instantiator {
107102
assert(result.out == wrapped_output.end());
108103
assert(equal(wrapped_output, expected_output, equal_to{}, &int_wrapper::val));
109104
assert(equal(input, expected_input, equal_to{}, &int_wrapper::val));
110-
not_ranges_destroy(wrapped_output);
105+
destroy(wrapped_output);
111106
assert(int_wrapper::constructions == 3);
112107
assert(int_wrapper::destructions == 3);
113108
}
@@ -127,10 +122,51 @@ struct instantiator {
127122
assert(result.out == wrapped_output.end());
128123
assert(equal(wrapped_output, expected_output, equal_to{}, &int_wrapper::val));
129124
assert(equal(input, expected_input, equal_to{}, &int_wrapper::val));
130-
not_ranges_destroy(wrapped_output);
125+
destroy(wrapped_output);
131126
assert(int_wrapper::constructions == 3);
132127
assert(int_wrapper::destructions == 3);
133128
}
129+
130+
{ // Validate range overload shorter output
131+
int_wrapper input[4] = {13, 55, 12345, 42};
132+
R wrapped_input{input};
133+
holder<int_wrapper, 3> mem;
134+
W wrapped_output{mem.as_span()};
135+
136+
int_wrapper::clear_counts();
137+
same_as<uninitialized_copy_result<iterator_t<R>, iterator_t<W>>> auto result =
138+
uninitialized_copy(wrapped_input, wrapped_output);
139+
assert(int_wrapper::constructions == 3);
140+
assert(int_wrapper::destructions == 0);
141+
assert(++result.in == wrapped_input.end());
142+
assert(result.out == wrapped_output.end());
143+
assert(equal(wrapped_output, expected_output, equal_to{}, &int_wrapper::val));
144+
assert(equal(input, expected_input_long, equal_to{}, &int_wrapper::val));
145+
destroy(wrapped_output);
146+
assert(int_wrapper::constructions == 3);
147+
assert(int_wrapper::destructions == 3);
148+
}
149+
150+
{ // Validate range overload shorter input
151+
int_wrapper input[3] = {13, 55, 12345};
152+
R wrapped_input{input};
153+
holder<int_wrapper, 4> mem;
154+
W wrapped_output{mem.as_span()};
155+
156+
int_wrapper::clear_counts();
157+
same_as<uninitialized_copy_result<iterator_t<R>, iterator_t<W>>> auto result =
158+
uninitialized_copy(wrapped_input, wrapped_output);
159+
assert(int_wrapper::constructions == 3);
160+
assert(int_wrapper::destructions == 0);
161+
assert(result.in == wrapped_input.end());
162+
construct_at(addressof(*result.out), -1); // Need to construct non written element for comparison
163+
assert(++result.out == wrapped_output.end());
164+
assert(equal(wrapped_output, expected_output_long, equal_to{}, &int_wrapper::val));
165+
assert(equal(input, expected_input, equal_to{}, &int_wrapper::val));
166+
destroy(wrapped_output);
167+
assert(int_wrapper::constructions == 4);
168+
assert(int_wrapper::destructions == 4);
169+
}
134170
}
135171
};
136172

@@ -160,6 +196,48 @@ struct throwing_test {
160196
}
161197
};
162198

199+
struct memcpy_test {
200+
static constexpr int expected_output[] = {13, 55, 12345};
201+
static constexpr int expected_output_long[] = {13, 55, 12345, -1};
202+
static constexpr int expected_input[] = {13, 55, 12345};
203+
static constexpr int expected_input_long[] = {13, 55, 12345, 42};
204+
205+
static void call() {
206+
{ // Validate only range overload
207+
int input[] = {13, 55, 12345};
208+
int output[] = {-1, -1, -1};
209+
210+
const auto result = ranges::uninitialized_copy(input, output);
211+
assert(result.in == end(input));
212+
assert(result.out == end(output));
213+
assert(ranges::equal(input, expected_input));
214+
assert(ranges::equal(output, expected_output));
215+
}
216+
217+
{ // Validate input shorter
218+
int input[] = {13, 55, 12345};
219+
int output[] = {-1, -1, -1, -1};
220+
221+
auto result = ranges::uninitialized_copy(input, output);
222+
assert(result.in == end(input));
223+
assert(++result.out == end(output));
224+
assert(ranges::equal(input, expected_input));
225+
assert(ranges::equal(output, expected_output_long));
226+
}
227+
228+
{ // Validate output shorter
229+
int input[] = {13, 55, 12345, 42};
230+
int output[] = {-1, -1, -1};
231+
232+
auto result = ranges::uninitialized_copy(input, output);
233+
assert(++result.in == end(input));
234+
assert(result.out == end(output));
235+
assert(ranges::equal(input, expected_input_long));
236+
assert(ranges::equal(output, expected_output));
237+
}
238+
}
239+
};
240+
163241
template <test::ProxyRef IsProxy>
164242
using test_input = test::range<test::input, int_wrapper, test::Sized::no, test::CanDifference::no, test::Common::no,
165243
test::CanCompare::yes, IsProxy>;
@@ -174,4 +252,5 @@ int main() {
174252
instantiator::call<test_input<test::ProxyRef::yes>, test_output>();
175253
throwing_test::call<test_input<test::ProxyRef::no>, test_output>();
176254
throwing_test::call<test_input<test::ProxyRef::yes>, test_output>();
255+
memcpy_test::call();
177256
}

tests/std/tests/P0896R4_ranges_alg_uninitialized_copy_n/test.cpp

Lines changed: 114 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <ranges>
1010
#include <span>
1111
#include <utility>
12+
#include <vector>
1213

1314
#include <range_algorithm_support.hpp>
1415

@@ -66,39 +67,77 @@ struct holder {
6667
}
6768
};
6869

69-
template <class R>
70-
void not_ranges_destroy(R&& r) { // TRANSITION, ranges::destroy
71-
for (auto& e : r) {
72-
destroy_at(&e);
73-
}
74-
}
75-
7670
struct instantiator {
77-
static constexpr int expected_output[] = {13, 55, 12345};
78-
static constexpr int expected_input[] = {13, 55, 12345};
71+
static constexpr int expected_output[] = {13, 55, 12345};
72+
static constexpr int expected_output_long[] = {13, 55, 12345, -1};
73+
static constexpr int expected_input[] = {13, 55, 12345};
74+
static constexpr int expected_input_long[] = {13, 55, 12345, 42};
7975

8076
template <ranges::input_range Read, ranges::forward_range Write>
8177
static void call() {
82-
using ranges::uninitialized_copy_n, ranges::uninitialized_copy_n_result, ranges::equal, ranges::equal_to,
83-
ranges::iterator_t;
78+
using ranges::destroy, ranges::uninitialized_copy_n, ranges::uninitialized_copy_n_result, ranges::equal,
79+
ranges::equal_to, ranges::iterator_t;
80+
81+
{ // Validate equal ranges
82+
int_wrapper input[3] = {13, 55, 12345};
83+
Read wrapped_input{input};
84+
holder<int_wrapper, 3> mem;
85+
Write wrapped_output{mem.as_span()};
86+
87+
int_wrapper::clear_counts();
88+
const same_as<uninitialized_copy_n_result<iterator_t<Read>, iterator_t<Write>>> auto result =
89+
uninitialized_copy_n(wrapped_input.begin(), 3, wrapped_output.begin(), wrapped_output.end());
90+
assert(int_wrapper::constructions == 3);
91+
assert(int_wrapper::destructions == 0);
92+
assert(result.in == wrapped_input.end());
93+
assert(result.out == wrapped_output.end());
94+
assert(equal(wrapped_output, expected_output, equal_to{}, &int_wrapper::val));
95+
assert(equal(input, expected_input, equal_to{}, &int_wrapper::val));
96+
destroy(wrapped_output);
97+
assert(int_wrapper::constructions == 3);
98+
assert(int_wrapper::destructions == 3);
99+
}
84100

85-
int_wrapper input[3] = {13, 55, 12345};
86-
Read wrapped_input{input};
87-
holder<int_wrapper, 3> mem;
88-
Write wrapped_output{mem.as_span()};
101+
{ // Validate shorter output
102+
int_wrapper input[4] = {13, 55, 12345, 42};
103+
Read wrapped_input{input};
104+
holder<int_wrapper, 3> mem;
105+
Write wrapped_output{mem.as_span()};
106+
107+
int_wrapper::clear_counts();
108+
same_as<uninitialized_copy_n_result<iterator_t<Read>, iterator_t<Write>>> auto result =
109+
uninitialized_copy_n(wrapped_input.begin(), 3, wrapped_output.begin(), wrapped_output.end());
110+
assert(int_wrapper::constructions == 3);
111+
assert(int_wrapper::destructions == 0);
112+
assert(++result.in == wrapped_input.end());
113+
assert(result.out == wrapped_output.end());
114+
assert(equal(wrapped_output, expected_output, equal_to{}, &int_wrapper::val));
115+
assert(equal(input, expected_input_long, equal_to{}, &int_wrapper::val));
116+
destroy(wrapped_output);
117+
assert(int_wrapper::constructions == 3);
118+
assert(int_wrapper::destructions == 3);
119+
}
89120

90-
int_wrapper::clear_counts();
91-
const same_as<uninitialized_copy_n_result<iterator_t<Read>, iterator_t<Write>>> auto result =
92-
uninitialized_copy_n(wrapped_input.begin(), 3, wrapped_output.begin(), wrapped_output.end());
93-
assert(int_wrapper::constructions == 3);
94-
assert(int_wrapper::destructions == 0);
95-
assert(result.in == wrapped_input.end());
96-
assert(result.out == wrapped_output.end());
97-
assert(equal(wrapped_output, expected_output, equal_to{}, &int_wrapper::val));
98-
assert(equal(input, expected_input, equal_to{}, &int_wrapper::val));
99-
not_ranges_destroy(wrapped_output);
100-
assert(int_wrapper::constructions == 3);
101-
assert(int_wrapper::destructions == 3);
121+
{ // Validate shorter input
122+
int_wrapper input[3] = {13, 55, 12345};
123+
Read wrapped_input{input};
124+
holder<int_wrapper, 4> mem;
125+
Write wrapped_output{mem.as_span()};
126+
127+
int_wrapper::clear_counts();
128+
same_as<uninitialized_copy_n_result<iterator_t<Read>, iterator_t<Write>>> auto result =
129+
uninitialized_copy_n(wrapped_input.begin(), 3, wrapped_output.begin(), wrapped_output.end());
130+
assert(int_wrapper::constructions == 3);
131+
assert(int_wrapper::destructions == 0);
132+
assert(result.in == wrapped_input.end());
133+
construct_at(addressof(*result.out), -1); // Need to construct non written element for comparison
134+
assert(++result.out == wrapped_output.end());
135+
assert(equal(wrapped_output, expected_output_long, equal_to{}, &int_wrapper::val));
136+
assert(equal(input, expected_input, equal_to{}, &int_wrapper::val));
137+
destroy(wrapped_output);
138+
assert(int_wrapper::constructions == 4);
139+
assert(int_wrapper::destructions == 4);
140+
}
102141
}
103142
};
104143

@@ -126,6 +165,53 @@ struct throwing_test {
126165
}
127166
};
128167

168+
struct memcpy_test {
169+
static constexpr int expected_output[] = {13, 55, 12345, -1};
170+
static constexpr int expected_output_long[] = {13, 55, -1, -1};
171+
static constexpr int expected_input[] = {13, 55, 12345, 42};
172+
static constexpr int expected_input_short[] = {13, 55};
173+
static constexpr int expected_input_long[] = {13, 55, 12345, 42};
174+
175+
static void call() {
176+
using ranges::uninitialized_copy_n, ranges::uninitialized_copy_n_result, ranges::equal, ranges::iterator_t;
177+
{ // Validate range overload
178+
vector<int> input = {13, 55, 12345, 42};
179+
vector<int> output = {-1, -1, -1, -1};
180+
181+
const same_as<uninitialized_copy_n_result<iterator_t<vector<int>>, iterator_t<vector<int>>>> auto result =
182+
uninitialized_copy_n(input.begin(), 3, output.begin(), output.end());
183+
assert(next(result.in) == input.end());
184+
assert(next(result.out) == output.end());
185+
assert(equal(input, expected_input));
186+
assert(equal(output, expected_output));
187+
}
188+
189+
{ // Validate shorter input
190+
vector<int> input = {13, 55};
191+
vector<int> output = {-1, -1, -1, -1};
192+
193+
const same_as<uninitialized_copy_n_result<iterator_t<vector<int>>, iterator_t<vector<int>>>> auto result =
194+
uninitialized_copy_n(input.begin(), 2, output.begin(), output.end());
195+
assert(result.in == input.end());
196+
assert(next(result.out, 2) == output.end());
197+
assert(equal(input, expected_input_short));
198+
assert(equal(output, expected_output_long));
199+
}
200+
201+
{ // Validate shorter output
202+
vector<int> input = {13, 55, 12345, 42};
203+
vector<int> output = {-1, -1};
204+
205+
const same_as<uninitialized_copy_n_result<iterator_t<vector<int>>, iterator_t<vector<int>>>> auto result =
206+
uninitialized_copy_n(input.begin(), 2, output.begin(), output.end());
207+
assert(next(result.in, 2) == input.end());
208+
assert(result.out == output.end());
209+
assert(equal(input, expected_input));
210+
assert(equal(output, expected_input_short));
211+
}
212+
}
213+
};
214+
129215
template <test::ProxyRef IsProxy>
130216
using test_input = test::range<test::input, int_wrapper, test::Sized::no, test::CanDifference::no, test::Common::no,
131217
test::CanCompare::yes, IsProxy>;
@@ -140,4 +226,5 @@ int main() {
140226
instantiator::call<test_input<test::ProxyRef::yes>, test_output>();
141227
throwing_test::call<test_input<test::ProxyRef::no>, test_output>();
142228
throwing_test::call<test_input<test::ProxyRef::yes>, test_output>();
229+
memcpy_test::call();
143230
}

0 commit comments

Comments
 (0)