-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Extend memcpy, memmove and memcmp optimizations
#2158
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
Extend memcpy, memmove and memcmp optimizations
#2158
Conversation
memcpy, memmove and memcmp optimizations.memcpy, memmove and memcmp optimizations
|
WOW, how is it possible that |
…r doesn't run out of memory
| tests\GH_000431_copy_move_family | ||
| tests\GH_000431_equal_family | ||
| tests\GH_000431_equal_memcmp_is_safe | ||
| tests\GH_000431_iter_copy_move_cat | ||
| tests\GH_000431_lex_compare_family | ||
| tests\GH_000431_lex_compare_memcmp_classify |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While having separate tests is more modular, and gives a better idea what failed when a test fails, they contribute to test execution time. Not sure what is more important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having them separate is better for development because you only run one of them and not all of them which would take much more time. Also I already had problems with GH_000431_lex_compare_memcmp_classify test failing on x86 due to compiler running out of memory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with both of you! 😸 There is definitely a test throughput concern with having lots of small test directories, but in this case I believe that it's reasonable to have separate directories for the reasons that @AdamBucior stated.
StephanTLavavej
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks - looking good so far, I've partially reviewed the product code (leaving a note to myself where I paused) in a video review. Will continue reviewing this later. 😸
Co-authored-by: Stephan T. Lavavej <stl@nuwen.net>
StephanTLavavej
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks - reviewed this in part 2 of the video review. Looked at the rest of <xutility>, still need to review the test code.
StephanTLavavej
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finished reviewing the tests! Looks good, should need just one more round of minor changes. Thanks again for this major PR.
tests/std/tests/GH_000431_iter_copy_move_cat/test.compile.pass.cpp
Outdated
Show resolved
Hide resolved
tests/std/tests/GH_000431_iter_copy_move_cat/test.compile.pass.cpp
Outdated
Show resolved
Hide resolved
|
Thanks! I pushed two small commits, one to use |
|
I'm mirroring this to an MSVC-internal PR. Please notify me if any further changes are pushed. |
|
I pushed workarounds for VSO-1409786 " |
|
Thanks to @joemmett, I've pushed a mechanical refactoring of the Jonathan also confirmed that |
|
Thanks for this major optimization overhaul! 🚀 😻 ⚡ |
Fixes #431.
Product code changes
memcpyandmemmoveoptimizations to not requireis_trivially_copyableandis_trivialbut justis_trivially_copy_constructible,is_trivially_move_constructible,is_trivially_copy_assignableoris_trivially_move_assignabledepending on operation._Ptr_copy_catand_Ptr_move_catto_Iter_copy_catand_Iter_move_catbecause they no longer work only on pointers.move_iteratorhandling inmemcpy/memmoveoptimizations by specializing_To_addressfor it so that it doesn't warn about deprecatedmove_iterator::operator->.memcpy,memmoveandmemcmpoptimization to handle pointers to pointer-interconvertible types.nullptr_tin_Is_all_bits_zero._Lex_compare_memcmp_classifyand extended it tobools.memcmpinlexicographical_compare_three_wayto handle comparators likestrong_order,weak_orderandpartial_order.memcmpoptimization to handle non-transparent functors (ex. to enable optimization inequalwhen usingint,unsigned intandequal_to<int>. This also enablesmemcmpoptimization with enums when using functor that converts them to their underlying type).memcpyandmemmoveoptimization forvolatile.memcpyoptimization for const destination in(ranges::)unitialized_(copy|move)(_n)algorithms (see<memory>:uninitialized_meowshould handlevoidify's wording #1780).Test changes
VSO_0180469_ptr_catin favor of more detailedGH_000431_iter_copy_move_cat,GH_000431_equal_memcmp_is_safeandGH_000431_lex_compare_memcmp_classify.VSO_0180469_fill_familyaddedGH_000431_copy_move_family,GH_000431_equal_familyandGH_000431_lex_compare_familyto test that all of the optimizations work well (also the Some StdLib algorithms fail /std:c++latest compilation with custom contiguous iterators (Visual Studio 2019 16.8) #1523 test fromVSO_0180469_ptr_catwas moved toGH_000431_equal_family).P0202R3_constexpr_algorithm_and_exchangeto define all necessary typedefs inoutput_pointer.