-
Notifications
You must be signed in to change notification settings - Fork 1.6k
<algorithm>
: Fix conditions for vectorization on trivial assignability
#5528
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
<algorithm>
: Fix conditions for vectorization on trivial assignability
#5528
Conversation
tests/std/tests/GH_004686_vectorization_on_trivial_assignability/test.cpp
Outdated
Show resolved
Hide resolved
tests/std/tests/GH_004686_vectorization_on_trivial_assignability/test.cpp
Outdated
Show resolved
Hide resolved
tests/std/tests/GH_004686_vectorization_on_trivial_assignability/test.cpp
Outdated
Show resolved
Hide resolved
tests/std/tests/GH_004686_vectorization_on_trivial_assignability/test.cpp
Outdated
Show resolved
Hide resolved
tests/std/tests/GH_004686_vectorization_on_trivial_assignability/test.cpp
Outdated
Show resolved
Hide resolved
tests/std/tests/GH_004686_vectorization_on_trivial_assignability/test.cpp
Outdated
Show resolved
Hide resolved
Thanks! 😻 I pushed test cleanups/simplifications. |
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
…n when calling ranges algorithms with PMD projections".
I've pushed a commit to perma-workaround VSO-1664341 " |
Thanks for figuring out how to fix this "trivial" bug! 🐞 🧠 🛠️ |
It's shown in WG21-P3279R0 that
is_trivially_(copy_,move_)assignable
is sometimes not it self a suitable condition for vectorization, because a derived class can be "trivially-assigned" via a trivial assignment operator of its base class.This PR strengthens the conditions by checking the return type of the assignment. I.e. the corresponding conditions are now that
T
is trivially assignable from the source, andT&
, which means that whenT
is a class, a defaulted and trivialT::operator=
is selected, andPedantically, a defaulted and trivial
T::operator=
possibly skips a subobject due to aforementioned weird derived classes. But currently, all mainstream compilers don't think so (see LLVM-37038, "reject-valid" demo, and "accept-invalid" demo). So I think the condition is safe despite not being theoretically sufficient.Also,
(ranges::)reverse_copy
should rely on trivial assignability, and we shouldn't vectorize it for element typepair<T&, U&>
just because trivially copyability. Actually, being trivially copyable doesn't mean any expected operation is publicly available and trivial.Affected algorithms:
(ranges::)copy
,(ranges::)copy_n
,(ranges::)copy_backward
,(ranges::)move
,(ranges::)move_backward
,(ranges::)reverse_copy
.Fixes #4686.