Skip to content

Commit 41ec195

Browse files
<algorithm>: Fix conditions for vectorization on trivial assignability (#5528)
Co-authored-by: Stephan T. Lavavej <[email protected]>
1 parent 3da7d27 commit 41ec195

File tree

5 files changed

+290
-5
lines changed

5 files changed

+290
-5
lines changed

stl/inc/algorithm

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5611,6 +5611,10 @@ namespace ranges {
56115611
} // namespace ranges
56125612
#endif // _HAS_CXX20
56135613

5614+
template <class _Ty>
5615+
struct _Is_trivially_copy_assignable_returning_same_reference
5616+
: bool_constant<_Is_trivially_assignable_returning_same_reference_v<_Ty&, const _Ty&>> {};
5617+
56145618
_EXPORT_STD template <class _BidIt, class _OutIt>
56155619
_CONSTEXPR20 _OutIt reverse_copy(_BidIt _First, _BidIt _Last, _OutIt _Dest) {
56165620
// copy reversing elements in [_First, _Last)
@@ -5624,8 +5628,8 @@ _CONSTEXPR20 _OutIt reverse_copy(_BidIt _First, _BidIt _Last, _OutIt _Dest) {
56245628
using _Elem = remove_reference_t<_Iter_ref_t<remove_const_t<decltype(_UFirst)>>>;
56255629
using _DestElem = remove_reference_t<_Iter_ref_t<decltype(_UDest)>>;
56265630
constexpr bool _Allow_vectorization = conjunction_v<is_same<remove_const_t<_Elem>, _DestElem>,
5627-
bool_constant<_Iterators_are_contiguous<decltype(_ULast), decltype(_UDest)>>, is_trivially_copyable<_Elem>,
5628-
negation<is_volatile<_Elem>>>;
5631+
bool_constant<_Iterators_are_contiguous<decltype(_ULast), decltype(_UDest)>>,
5632+
_Is_trivially_copy_assignable_returning_same_reference<_Elem>, negation<is_volatile<_Elem>>>;
56295633
constexpr size_t _Nx = sizeof(_Elem);
56305634

56315635
if constexpr (_Allow_vectorization && _Nx <= 8 && (_Nx & (_Nx - 1)) == 0) {
@@ -5714,7 +5718,7 @@ namespace ranges {
57145718
using _Elem = remove_reference_t<iter_reference_t<_It>>;
57155719
using _DestElem = remove_reference_t<iter_reference_t<_Out>>;
57165720
constexpr bool _Allow_vectorization = conjunction_v<is_same<remove_const_t<_Elem>, _DestElem>,
5717-
is_trivially_copyable<_Elem>, negation<is_volatile<_Elem>>>;
5721+
_Is_trivially_copy_assignable_returning_same_reference<_Elem>, negation<is_volatile<_Elem>>>;
57185722
constexpr size_t _Nx = sizeof(_Elem);
57195723

57205724
if constexpr (_Allow_vectorization && _Nx <= 8 && (_Nx & (_Nx - 1)) == 0) {

stl/inc/xutility

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4691,6 +4691,25 @@ constexpr bool _Is_pointer_address_convertible = is_void_v<_Source>
46914691
#endif // defined(__cpp_lib_is_pointer_interconvertible)
46924692
;
46934693

4694+
#pragma warning(push)
4695+
#pragma warning(disable : 4242) // '%s': conversion from '%s' to '%s', possible loss of data
4696+
#pragma warning(disable : 4244) // '%s': conversion from '%s' to '%s', possible loss of data (Yes, duplicated message.)
4697+
#pragma warning(disable : 4267) // '%s': conversion from '%s' to '%s', possible loss of data (Yes, duplicated message!)
4698+
#pragma warning(disable : 4365) // '%s': conversion from '%s' to '%s', signed/unsigned mismatch
4699+
#pragma warning(disable : 5267) // definition of implicit assignment operator for '%s' is deprecated because it has a
4700+
// user-provided copy constructor
4701+
4702+
// Determines whether _DestRef is trivially assignable from _SourceRef, and if _Remove_cvref_t<_DestRef> is a class, the
4703+
// assignment uses _Remove_cvref_t<_DestRef>::operator=.
4704+
// Not pedantically reliable for vectorization, but working due to bugs of MSVC, Clang, and EDG. See LLVM-37038.
4705+
template <class _DestRef, class _SourceRef, bool = is_trivially_assignable_v<_DestRef, _SourceRef>>
4706+
constexpr bool _Is_trivially_assignable_returning_same_reference_v =
4707+
is_same_v<_DestRef, decltype(_STD declval<_DestRef>() = _STD declval<_SourceRef>())>;
4708+
template <class _DestRef, class _SourceRef>
4709+
constexpr bool _Is_trivially_assignable_returning_same_reference_v<_DestRef, _SourceRef, false> = false;
4710+
4711+
#pragma warning(pop)
4712+
46944713
template <class _Source, class _Dest, class _SourceRef, class _DestRef>
46954714
struct _Trivial_cat {
46964715
using _USource = _Unwrap_enum_t<_Source>;
@@ -4709,7 +4728,7 @@ struct _Trivial_cat {
47094728
_Same_size_and_compatible && is_trivially_constructible_v<_Dest, _SourceRef>;
47104729

47114730
static constexpr bool _Bitcopy_assignable =
4712-
_Same_size_and_compatible && is_trivially_assignable_v<_DestRef, _SourceRef>;
4731+
_Same_size_and_compatible && _Is_trivially_assignable_returning_same_reference_v<_DestRef, _SourceRef>;
47134732
};
47144733

47154734
template <class _Source, class _Dest, class _SourceRef, class _DestRef>
@@ -4718,7 +4737,8 @@ struct _Trivial_cat<_Source*, _Dest*, _SourceRef, _DestRef> {
47184737
_Is_pointer_address_convertible<_Source, _Dest> && is_trivially_constructible_v<_Dest*, _SourceRef>;
47194738

47204739
static constexpr bool _Bitcopy_assignable =
4721-
_Is_pointer_address_convertible<_Source, _Dest> && is_trivially_assignable_v<_DestRef, _SourceRef>;
4740+
_Is_pointer_address_convertible<_Source, _Dest>
4741+
&& _Is_trivially_assignable_returning_same_reference_v<_DestRef, _SourceRef>;
47224742
};
47234743

47244744
struct _False_trivial_cat {

tests/std/test.lst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@ tests\GH_004609_heterogeneous_cmp_overloads
252252
tests\GH_004618_mixed_operator_usage_keeps_statistical_properties
253253
tests\GH_004618_normal_distribution_avoids_resets
254254
tests\GH_004657_expected_constraints_permissive
255+
tests\GH_004686_vectorization_on_trivial_assignability
255256
tests\GH_004845_logical_operator_traits_with_non_bool_constant
256257
tests\GH_004929_internal_tag_constructors
257258
tests\GH_004930_char_traits_user_specialization
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
# Copyright (c) Microsoft Corporation.
2+
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
3+
4+
RUNALL_INCLUDE ..\usual_matrix.lst
Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
3+
4+
#include <algorithm>
5+
#include <cassert>
6+
#include <iterator>
7+
#include <utility>
8+
9+
#if _HAS_CXX20
10+
#define CONSTEXPR20 constexpr
11+
#else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv
12+
#define CONSTEXPR20 inline
13+
#endif // ^^^ !_HAS_CXX20 ^^^
14+
15+
using namespace std;
16+
17+
struct Cat {};
18+
struct Leopard : Cat {
19+
int spots_;
20+
21+
Leopard() = default;
22+
Leopard(const Leopard&) = default;
23+
Leopard(Leopard&&) = default;
24+
Leopard& operator=(Leopard&) && = delete;
25+
using Cat::operator=;
26+
};
27+
28+
constexpr pair<int, int> expected_results[]{{5, 6}, {3, 4}, {1, 2}};
29+
30+
CONSTEXPR20 void test_reverse_copy() {
31+
{
32+
pair<int, int> src[] = {{1, 2}, {3, 4}, {5, 6}};
33+
pair<int, int> dst[] = {{3, 1}, {4, 1}, {5, 9}};
34+
pair<int&, int&> srcref[] = {
35+
{src[0].first, src[0].second}, {src[1].first, src[1].second}, {src[2].first, src[2].second}};
36+
pair<int&, int&> dstref[] = {
37+
{dst[0].first, dst[0].second}, {dst[1].first, dst[1].second}, {dst[2].first, dst[2].second}};
38+
39+
reverse_copy(begin(srcref), end(srcref), dstref);
40+
assert(equal(begin(dst), end(dst), begin(expected_results), end(expected_results)));
41+
}
42+
#if _HAS_CXX20
43+
{
44+
pair<int, int> src[] = {{1, 2}, {3, 4}, {5, 6}};
45+
pair<int, int> dst[] = {{3, 1}, {4, 1}, {5, 9}};
46+
pair<int&, int&> srcref[] = {
47+
{src[0].first, src[0].second}, {src[1].first, src[1].second}, {src[2].first, src[2].second}};
48+
pair<int&, int&> dstref[] = {
49+
{dst[0].first, dst[0].second}, {dst[1].first, dst[1].second}, {dst[2].first, dst[2].second}};
50+
51+
ranges::reverse_copy(srcref, dstref);
52+
assert(ranges::equal(dst, expected_results));
53+
}
54+
#endif // _HAS_CXX20
55+
#if _HAS_CXX23
56+
{
57+
pair<int, int> src[] = {{1, 2}, {3, 4}, {5, 6}};
58+
pair<int, int> dst[] = {{3, 1}, {4, 1}, {5, 9}};
59+
pair<int&, int&> srcref[] = {src[0], src[1], src[2]};
60+
pair<int&, int&> dstref[] = {dst[0], dst[1], dst[2]};
61+
62+
reverse_copy(begin(srcref), end(srcref), dstref);
63+
assert(equal(begin(dst), end(dst), begin(expected_results), end(expected_results)));
64+
}
65+
{
66+
pair<int, int> src[] = {{1, 2}, {3, 4}, {5, 6}};
67+
pair<int, int> dst[] = {{3, 1}, {4, 1}, {5, 9}};
68+
pair<int&, int&> srcref[] = {src[0], src[1], src[2]};
69+
pair<int&, int&> dstref[] = {dst[0], dst[1], dst[2]};
70+
71+
ranges::reverse_copy(srcref, dstref);
72+
assert(ranges::equal(dst, expected_results));
73+
}
74+
#endif // _HAS_CXX23
75+
}
76+
77+
constexpr Leopard make_leopard(const int n) noexcept {
78+
Leopard result{};
79+
result.spots_ = n;
80+
return result;
81+
}
82+
83+
CONSTEXPR20 void test_copy_move_leopards() {
84+
constexpr Leopard expected_leopards[]{
85+
make_leopard(3), make_leopard(1), make_leopard(4), make_leopard(1), make_leopard(5), make_leopard(9)};
86+
constexpr Leopard zero_leopards[6]{};
87+
auto equal_leopard = [](const Leopard& lhs, const Leopard& rhs) { return lhs.spots_ == rhs.spots_; };
88+
{
89+
Leopard dst[]{
90+
make_leopard(3), make_leopard(1), make_leopard(4), make_leopard(1), make_leopard(5), make_leopard(9)};
91+
copy(begin(zero_leopards), end(zero_leopards), begin(dst));
92+
assert(equal(begin(dst), end(dst), begin(expected_leopards), end(expected_leopards), equal_leopard));
93+
}
94+
{
95+
Leopard dst[]{
96+
make_leopard(3), make_leopard(1), make_leopard(4), make_leopard(1), make_leopard(5), make_leopard(9)};
97+
copy_n(begin(zero_leopards), size(zero_leopards), begin(dst));
98+
assert(equal(begin(dst), end(dst), begin(expected_leopards), end(expected_leopards), equal_leopard));
99+
}
100+
{
101+
Leopard dst[]{
102+
make_leopard(3), make_leopard(1), make_leopard(4), make_leopard(1), make_leopard(5), make_leopard(9)};
103+
copy_backward(begin(zero_leopards), end(zero_leopards), end(dst));
104+
assert(equal(begin(dst), end(dst), begin(expected_leopards), end(expected_leopards), equal_leopard));
105+
}
106+
{
107+
Leopard dst[]{
108+
make_leopard(3), make_leopard(1), make_leopard(4), make_leopard(1), make_leopard(5), make_leopard(9)};
109+
move(begin(zero_leopards), end(zero_leopards), begin(dst));
110+
assert(equal(begin(dst), end(dst), begin(expected_leopards), end(expected_leopards), equal_leopard));
111+
}
112+
{
113+
Leopard dst[]{
114+
make_leopard(3), make_leopard(1), make_leopard(4), make_leopard(1), make_leopard(5), make_leopard(9)};
115+
move_backward(begin(zero_leopards), end(zero_leopards), end(dst));
116+
assert(equal(begin(dst), end(dst), begin(expected_leopards), end(expected_leopards), equal_leopard));
117+
}
118+
#if _HAS_CXX20
119+
{
120+
Leopard dst[]{
121+
make_leopard(3), make_leopard(1), make_leopard(4), make_leopard(1), make_leopard(5), make_leopard(9)};
122+
ranges::copy(zero_leopards, dst);
123+
assert(ranges::equal(dst, expected_leopards, equal_leopard));
124+
}
125+
{
126+
Leopard dst[]{
127+
make_leopard(3), make_leopard(1), make_leopard(4), make_leopard(1), make_leopard(5), make_leopard(9)};
128+
ranges::copy_n(ranges::begin(zero_leopards), ranges::distance(zero_leopards), dst);
129+
assert(ranges::equal(dst, expected_leopards, equal_leopard));
130+
}
131+
{
132+
Leopard dst[]{
133+
make_leopard(3), make_leopard(1), make_leopard(4), make_leopard(1), make_leopard(5), make_leopard(9)};
134+
ranges::copy_backward(zero_leopards, ranges::end(dst));
135+
assert(ranges::equal(dst, expected_leopards, equal_leopard));
136+
}
137+
{
138+
Leopard dst[]{
139+
make_leopard(3), make_leopard(1), make_leopard(4), make_leopard(1), make_leopard(5), make_leopard(9)};
140+
ranges::move(zero_leopards, dst);
141+
assert(ranges::equal(dst, expected_leopards, equal_leopard));
142+
}
143+
{
144+
Leopard dst[]{
145+
make_leopard(3), make_leopard(1), make_leopard(4), make_leopard(1), make_leopard(5), make_leopard(9)};
146+
ranges::move_backward(zero_leopards, ranges::end(dst));
147+
assert(ranges::equal(dst, expected_leopards, equal_leopard));
148+
}
149+
#endif // _HAS_CXX20
150+
}
151+
152+
// Pedantically, all of MSVC, Clang, and EDG are currently wrong on this, see LLVM-37038.
153+
// However, if compilers get corrected, the assignment operators of `DerivedLeopard` and `LeopardHouse` will be trivial
154+
// but no-op, and the library side can't correctly conclude that assignments for them shouldn't be vectorized.
155+
// As a result, we keep this as a regression test.
156+
CONSTEXPR20 void test_llvm_37038() {
157+
struct DerivedLeopard : Leopard {};
158+
static_assert(is_trivially_move_assignable_v<DerivedLeopard>, "");
159+
160+
auto make_derived_leopard = [](int n) {
161+
DerivedLeopard ret{};
162+
ret.spots_ = n;
163+
return ret;
164+
};
165+
166+
auto equal_derived = [](const DerivedLeopard& lhs, const DerivedLeopard& rhs) { return lhs.spots_ == rhs.spots_; };
167+
168+
{
169+
DerivedLeopard src[]{
170+
make_derived_leopard(1), make_derived_leopard(7), make_derived_leopard(2), make_derived_leopard(9)};
171+
DerivedLeopard dst[4]{};
172+
move(begin(src), end(src), dst);
173+
assert(equal(begin(dst), end(dst), begin(src), end(src), equal_derived));
174+
}
175+
{
176+
DerivedLeopard src[]{
177+
make_derived_leopard(1), make_derived_leopard(7), make_derived_leopard(2), make_derived_leopard(9)};
178+
DerivedLeopard dst[4]{};
179+
move_backward(begin(src), end(src), end(dst));
180+
assert(equal(begin(dst), end(dst), begin(src), end(src), equal_derived));
181+
}
182+
#if _HAS_CXX20
183+
{
184+
DerivedLeopard src[]{
185+
make_derived_leopard(1), make_derived_leopard(7), make_derived_leopard(2), make_derived_leopard(9)};
186+
DerivedLeopard dst[4]{};
187+
ranges::move(src, dst);
188+
assert(ranges::equal(src, dst, equal_derived));
189+
}
190+
{
191+
DerivedLeopard src[]{
192+
make_derived_leopard(1), make_derived_leopard(7), make_derived_leopard(2), make_derived_leopard(9)};
193+
DerivedLeopard dst[4]{};
194+
ranges::move_backward(src, ranges::end(dst));
195+
assert(ranges::equal(src, dst, equal_derived));
196+
}
197+
#endif // _HAS_CXX20
198+
199+
struct LeopardHouse {
200+
Leopard bigcat_;
201+
};
202+
static_assert(is_trivially_move_assignable_v<LeopardHouse>, "");
203+
204+
auto make_leopard_house = [](int n) {
205+
LeopardHouse ret{};
206+
ret.bigcat_.spots_ = n;
207+
return ret;
208+
};
209+
210+
auto equal_house = [](const LeopardHouse& lhs, const LeopardHouse& rhs) {
211+
return lhs.bigcat_.spots_ == rhs.bigcat_.spots_;
212+
};
213+
214+
{
215+
LeopardHouse src[]{make_leopard_house(1), make_leopard_house(7), make_leopard_house(2), make_leopard_house(9)};
216+
LeopardHouse dst[4]{};
217+
move(begin(src), end(src), dst);
218+
assert(equal(begin(dst), end(dst), begin(src), end(src), equal_house));
219+
}
220+
{
221+
LeopardHouse src[]{make_leopard_house(1), make_leopard_house(7), make_leopard_house(2), make_leopard_house(9)};
222+
LeopardHouse dst[4]{};
223+
move_backward(begin(src), end(src), end(dst));
224+
assert(equal(begin(dst), end(dst), begin(src), end(src), equal_house));
225+
}
226+
#if _HAS_CXX20
227+
{
228+
LeopardHouse src[]{make_leopard_house(1), make_leopard_house(7), make_leopard_house(2), make_leopard_house(9)};
229+
LeopardHouse dst[4]{};
230+
ranges::move(src, dst);
231+
assert(ranges::equal(src, dst, equal_house));
232+
}
233+
{
234+
LeopardHouse src[]{make_leopard_house(1), make_leopard_house(7), make_leopard_house(2), make_leopard_house(9)};
235+
LeopardHouse dst[4]{};
236+
ranges::move_backward(src, ranges::end(dst));
237+
assert(ranges::equal(src, dst, equal_house));
238+
}
239+
#endif // _HAS_CXX20
240+
}
241+
242+
CONSTEXPR20 bool test() {
243+
test_reverse_copy();
244+
test_copy_move_leopards();
245+
test_llvm_37038();
246+
247+
return true;
248+
}
249+
250+
#if _HAS_CXX20
251+
static_assert(test());
252+
#endif // _HAS_CXX20
253+
254+
int main() {
255+
test();
256+
}

0 commit comments

Comments
 (0)