Add AVX512BW implementation::convert_utf8_to_utf16 - #2
Conversation
|
I will be reviewing this PR from now on. The other PR has been reviewed. Thanks for your patience. |
| const __m512i v_fc00_fc00 = _mm512_set1_epi32(0xfc00fc00); | ||
| const __m512i v_d800_dc00 = _mm512_set1_epi32(0xd800dc00); | ||
|
|
||
| const __m512i v_0f = _mm512_set1_epi8(0x0f); |
There was a problem hiding this comment.
I would have the same comment as with the other PR. It is debatable whether having these constants (often used in a single function) off to the side like this improves code readability and maintenance. (This is not me asking you to change the code. I am only pointing it out.)
There was a problem hiding this comment.
It bothers me too. But as I wrote previously, it's due to unpredictable compiler output.
|
I'm going to resolve conflicts and push-force this branch. |
- replace `and` with `&&`
2776fa1 to
61c9f29
Compare
|
Please be aware that my next goal is to port your UTF-8 validation that relies on |
| __mmask64 _3bytes; | ||
| { | ||
| const __m512i t0 = _mm512_and_si512(leading_bytes, v_f0); | ||
| _3bytes = _mm512_cmpeq_epi8_mask(t0, v_e0); |
There was a problem hiding this comment.
Same kind of remark, this can be done with a single instruction... maybe _mm512_cmpgt_epu8_mask
| */ | ||
|
|
||
| // Note: equality can be replace by series of xor. | ||
| const uint64_t valid_4bytes = (_4bytes & next4) & ~(next1 | next2 | next3); |
There was a problem hiding this comment.
Something like _ktestz_mask8_u8 can be used around here.
| while (ptr + 64 < end) { | ||
| const __m512i utf8 = _mm512_loadu_si512((const __m512i*)ptr); | ||
| const __mmask64 ascii = _mm512_test_epi8_mask(utf8, v_80); | ||
| if (ascii == 0) { |
There was a problem hiding this comment.
Around here, you will have 6 cycles on latency. 3 cycles for _mm512_test_epi8_mask and then 3 cycles for the ascii==0. So it is not as cheap as it looks.
There was a problem hiding this comment.
Under AVX, you'd have only 3 cycles of latency, thanks to _mm256_testz_si256
My suspicion is that it would be much faster than what you have here if implemented properly. :-) And interesting in its own right!!! |
|
Note that I have not actually proven that your approach is correct. It is possible that there might be a conceptual bug that I have not caught, but this can be handled with unit tests. (Easy enough to do.) |
|
This is important work. |
Thank you Daniel for so detailed review. I appreciate your hard work. I will push some fixes soon and then move to the next pieces that are waiting for integration. |
- rewrite series of bit ops using AVX512 mask ops The resulting assembly output is significantly shorter: 47 vs 36 instructions.
- use masked AVX512 operations
- simplify detection of continuation bytes - detect when the input starts with continuation byte, which is an error that cannot be detected by the current AVX512 code
|
I am merging it into a repository within simdutf, to be worked further. |
* Validate latin * Added Latin1 <=> UTF-X, tests TBD * UTF -X <=> Latin1 conversions, test TBD * Most Latin1 <=> UTF-X now passes compilation * Implemented UTF32 => latin1 for archs on my CPU * "Hello world" test UTF32 => Latin1 * Mostly works * UTF32 => Latin1 test compiling but test failing * Got a segfault here. * Fixed segfault * Less verbosity * Same * utf32 => Latin1 large input test * test for Latin1 => UTF32 test. WIP * fixed tests * UTF32 => latin1 valid tests WIP * Done with UTF 32 <=> Latin1 ? * valid utf32 => latin1 tests * UTF8 => Latin1 mostly done. Need tests. * more tests for UTF8 => Latin1 * Some more tests * UTF8 => Latin1 with errors tests * 4_byte too large test UTF8=>latin1 working * utf8 => latin1 too_large_input test works * Header bits test work + small fixes * UTF8 =>Latin1 /w errors test done * utf8 => latin1 convert_valid tests done * latin1 => UTF8 tests * utf16le => latin1 tests * utf16le => Latin1 with errors tests * utf16le => latin1 tests * utf16be => Latin1 tests * latin1 => utf16be tests * utf8 => Latin1 benchmarks * fixing icu test attempt * icu benchmark test * Various minor fixes * partial cleanup * More cleaning up * buggy cleanup * better safe than sorry increnemental cleanup * Very incremental cleanup that copiles #2 * Slowly but surely! * Small, sure,safe & incremental * More incremental cleanup * More increments * More increments * More increments * More increments * Preliminary Cleanup done * Minor adjustment * Latin1 => utf8 benchmarks (untested) * latin1 => utf16le bench untested * latin1 => utf32 benchmark + basic tests * utf16 => latin1 benchmark * utf32 => Latin1 benchmark * fix wrong size causing segfaults... * Latin1 => utf8 icu * latin1 => utf16 iconv (prev was also iconv) * Latin1 =>Utf32 iconv * Latin1 => UTF16 Iconv bench * UTF32 => Latin1 Iconv * latin1 => utf8 icu bench * Fix some buffer overflow errors * Fixes to latin1 => UTF-16 bench * Latin1 => UTF32 icu bench * UTF16 => Latin1 ICU * UTF32 => Latin1 ICU benches * Working UTF16 =>Latin1 icu bench * utf32 => Latin1 Icu working * Some fixes * Some fixes to benches + modest README changes * README done + minor changes * More ICU checks for safety * Remove comments * fixes * Various fixes on the latin 1 branch * Fixes. * latin1_from_utf16 unused parameter deletion * latin1_from_utf32 unused parameter deletion * utf32 length from latin1 unused parameter deletion * fixes * utf16 length from latin1 unusued parameter deletion * more parameter deletion * more parameter deletions * Windows fixes * arm64 dummy/scalar implementation * More windows fixes * Update tests/convert_utf32_to_latin1_with_errors_tests.cpp Co-authored-by: Daniel Lemire <daniel@lemire.me> * Update tests/convert_utf32_to_latin1_tests.cpp Co-authored-by: Daniel Lemire <daniel@lemire.me> * partial fixes * Remove extraneous comments added by mistake * remove extraneous comments * Windows Fixes/fix utf32 => latin1 tests * CMakeLists fixes * More fixes * Yet more fixes * Made tests deterministic again + removed comments * More Windows fixes * s390/big endian fixes * Windows fixes * Cleaned up comments --------- Co-authored-by: Daniel Lemire <dlemire@lemire.me> Co-authored-by: Daniel Lemire <daniel@lemire.me>
* Validate latin * Added Latin1 <=> UTF-X, tests TBD * UTF -X <=> Latin1 conversions, test TBD * Most Latin1 <=> UTF-X now passes compilation * Implemented UTF32 => latin1 for archs on my CPU * "Hello world" test UTF32 => Latin1 * Mostly works * UTF32 => Latin1 test compiling but test failing * Got a segfault here. * Fixed segfault * Less verbosity * Same * utf32 => Latin1 large input test * test for Latin1 => UTF32 test. WIP * fixed tests * UTF32 => latin1 valid tests WIP * Done with UTF 32 <=> Latin1 ? * valid utf32 => latin1 tests * UTF8 => Latin1 mostly done. Need tests. * more tests for UTF8 => Latin1 * Some more tests * UTF8 => Latin1 with errors tests * 4_byte too large test UTF8=>latin1 working * utf8 => latin1 too_large_input test works * Header bits test work + small fixes * UTF8 =>Latin1 /w errors test done * utf8 => latin1 convert_valid tests done * latin1 => UTF8 tests * utf16le => latin1 tests * utf16le => Latin1 with errors tests * utf16le => latin1 tests * utf16be => Latin1 tests * latin1 => utf16be tests * utf8 => Latin1 benchmarks * fixing icu test attempt * icu benchmark test * Various minor fixes * partial cleanup * More cleaning up * buggy cleanup * better safe than sorry increnemental cleanup * Very incremental cleanup that copiles #2 * Slowly but surely! * Small, sure,safe & incremental * More incremental cleanup * More increments * More increments * More increments * More increments * Preliminary Cleanup done * Minor adjustment * Latin1 => utf8 benchmarks (untested) * latin1 => utf16le bench untested * latin1 => utf32 benchmark + basic tests * utf16 => latin1 benchmark * utf32 => Latin1 benchmark * fix wrong size causing segfaults... * Latin1 => utf8 icu * latin1 => utf16 iconv (prev was also iconv) * Latin1 =>Utf32 iconv * Latin1 => UTF16 Iconv bench * UTF32 => Latin1 Iconv * latin1 => utf8 icu bench * Fix some buffer overflow errors * Fixes to latin1 => UTF-16 bench * Latin1 => UTF32 icu bench * UTF16 => Latin1 ICU * UTF32 => Latin1 ICU benches * Working UTF16 =>Latin1 icu bench * utf32 => Latin1 Icu working * Some fixes * Some fixes to benches + modest README changes * README done + minor changes * More ICU checks for safety * Remove comments * fixes * Various fixes on the latin 1 branch * Fixes. * latin1_from_utf16 unused parameter deletion * latin1_from_utf32 unused parameter deletion * utf32 length from latin1 unused parameter deletion * fixes * utf16 length from latin1 unusued parameter deletion * more parameter deletion * more parameter deletions * Windows fixes * arm64 dummy/scalar implementation * More windows fixes * Update tests/convert_utf32_to_latin1_with_errors_tests.cpp Co-authored-by: Daniel Lemire <daniel@lemire.me> * Update tests/convert_utf32_to_latin1_tests.cpp Co-authored-by: Daniel Lemire <daniel@lemire.me> * partial fixes * Remove extraneous comments added by mistake * remove extraneous comments * Windows Fixes/fix utf32 => latin1 tests * CMakeLists fixes * More fixes * Yet more fixes * Made tests deterministic again + removed comments * More Windows fixes * s390/big endian fixes * Windows fixes * added UTF8 => Latin1 Westmere length * fixed shadowing comments * latin1 => utf8 length + minor cleanup * fixes * Made code clearer --------- Co-authored-by: Daniel Lemire <dlemire@lemire.me> Co-authored-by: Daniel Lemire <daniel@lemire.me>
* Validate latin * Added Latin1 <=> UTF-X, tests TBD * UTF -X <=> Latin1 conversions, test TBD * Most Latin1 <=> UTF-X now passes compilation * Implemented UTF32 => latin1 for archs on my CPU * "Hello world" test UTF32 => Latin1 * Mostly works * UTF32 => Latin1 test compiling but test failing * Got a segfault here. * Fixed segfault * Less verbosity * Same * utf32 => Latin1 large input test * test for Latin1 => UTF32 test. WIP * fixed tests * UTF32 => latin1 valid tests WIP * Done with UTF 32 <=> Latin1 ? * valid utf32 => latin1 tests * UTF8 => Latin1 mostly done. Need tests. * more tests for UTF8 => Latin1 * Some more tests * UTF8 => Latin1 with errors tests * 4_byte too large test UTF8=>latin1 working * utf8 => latin1 too_large_input test works * Header bits test work + small fixes * UTF8 =>Latin1 /w errors test done * utf8 => latin1 convert_valid tests done * latin1 => UTF8 tests * utf16le => latin1 tests * utf16le => Latin1 with errors tests * utf16le => latin1 tests * utf16be => Latin1 tests * latin1 => utf16be tests * utf8 => Latin1 benchmarks * fixing icu test attempt * icu benchmark test * Various minor fixes * partial cleanup * More cleaning up * buggy cleanup * better safe than sorry increnemental cleanup * Very incremental cleanup that copiles #2 * Slowly but surely! * Small, sure,safe & incremental * More incremental cleanup * More increments * More increments * More increments * More increments * Preliminary Cleanup done * Minor adjustment * Latin1 => utf8 benchmarks (untested) * latin1 => utf16le bench untested * latin1 => utf32 benchmark + basic tests * utf16 => latin1 benchmark * utf32 => Latin1 benchmark * fix wrong size causing segfaults... * Latin1 => utf8 icu * latin1 => utf16 iconv (prev was also iconv) * Latin1 =>Utf32 iconv * Latin1 => UTF16 Iconv bench * UTF32 => Latin1 Iconv * latin1 => utf8 icu bench * Fix some buffer overflow errors * Fixes to latin1 => UTF-16 bench * Latin1 => UTF32 icu bench * UTF16 => Latin1 ICU * UTF32 => Latin1 ICU benches * Working UTF16 =>Latin1 icu bench * utf32 => Latin1 Icu working * Some fixes * Some fixes to benches + modest README changes * README done + minor changes * More ICU checks for safety * Remove comments * fixes * Various fixes on the latin 1 branch * Fixes. * latin1_from_utf16 unused parameter deletion * latin1_from_utf32 unused parameter deletion * utf32 length from latin1 unused parameter deletion * fixes * utf16 length from latin1 unusued parameter deletion * more parameter deletion * more parameter deletions * Windows fixes * arm64 dummy/scalar implementation * More windows fixes * Update tests/convert_utf32_to_latin1_with_errors_tests.cpp Co-authored-by: Daniel Lemire <daniel@lemire.me> * Update tests/convert_utf32_to_latin1_tests.cpp Co-authored-by: Daniel Lemire <daniel@lemire.me> * partial fixes * Remove extraneous comments added by mistake * remove extraneous comments * Windows Fixes/fix utf32 => latin1 tests * CMakeLists fixes * More fixes * Yet more fixes * Made tests deterministic again + removed comments * More Windows fixes * s390/big endian fixes * Windows fixes * added UTF8 => Latin1 Westmere length * fixed shadowing comments * latin1 => utf8 length + minor cleanup * fixes * buggy commit * buggy commit * working avx512 needs cleanup and unrolling * Speed improvements * unrolled avx512 * closer to the original bring more speed? * buggy... but on the right path performance-wise * Working but need cleanup * small cleanup * Cmake flag changes * Small cleanup * Revert "buggy commit" This reverts commit 589e4db. Deletes builds360 file accidently uploaded. * fixes * Cmake fixes? * CMake fix attempt * removed extraneous comments * several PR fixes * *forgot one file * Fix attempt * fix typo : missing '>' * cleanup * removed submodule added by mistake * Moving to 4-wise blocks * Revert "Moving to 4-wise blocks" This reverts commit 049d4a1. * Removed UINT64_t check * Fixed gitignore --------- Co-authored-by: Daniel Lemire <dlemire@lemire.me> Co-authored-by: Daniel Lemire <daniel@lemire.me>
…imdutf#576) ==17876==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x517000000307 at pc 0x561dd0419320 bp 0x7ffd11808360 sp 0x7ffd11808358 WRITE of size 16 at 0x517000000307 thread T0 #0 0x561dd041931f in simdutf::haswell::(anonymous namespace)::convert_masked_utf8_to_latin1(char const*, unsigned long, char*&) /home/pauldreik/code/delaktig/simdutf/src/haswell/avx2_convert_utf8_to_latin1.cpp:28:5 #1 0x561dd037a5d7 in simdutf::haswell::(anonymous namespace)::utf8_to_latin1::validating_transcoder::convert(char const*, unsigned long, char*) /home/pauldreik/code/delaktig/simdutf/src/generic/utf8_to_latin1/utf8_to_latin1.h:178:29 #2 0x561dd037a06b in simdutf::haswell::implementation::convert_utf8_to_latin1(char const*, unsigned long, char*) const /home/pauldreik/code/delaktig/simdutf/src/haswell/implementation.cpp:309:20 simdutf#3 0x561dd0286e12 in test_impl_ossfuzz_372067232(simdutf::implementation const&) /home/pauldreik/code/delaktig/simdutf/tests/convert_utf8_to_latin1_tests.cpp:93:33 simdutf#4 0x561dd028516f in ossfuzz_372067232(simdutf::implementation const&) /home/pauldreik/code/delaktig/simdutf/tests/convert_utf8_to_latin1_tests.cpp:20:1 simdutf#5 0x561dd0299782 in simdutf::test::test_entry::operator()(simdutf::implementation const&) /home/pauldreik/code/delaktig/simdutf/tests/helpers/test.h:18:58 simdutf#6 0x561dd02926ca in (anonymous namespace)::run((anonymous namespace)::CommandLine const&) /home/pauldreik/code/delaktig/simdutf/tests/helpers/test.cpp:179:9 simdutf#7 0x561dd0290fc5 in simdutf::test::main(int, char**) /home/pauldreik/code/delaktig/simdutf/tests/helpers/test.cpp:207:3 simdutf#8 0x561dd028d611 in main /home/pauldreik/code/delaktig/simdutf/tests/convert_utf8_to_latin1_tests.cpp:293:1 simdutf#9 0x7f31b415adb9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 simdutf#10 0x7f31b415ae74 in __libc_start_main csu/../csu/libc-start.c:360:3 simdutf#11 0x561dd01a9650 in _start (/home/pauldreik/code/delaktig/simdutf/build/paul_clang_18-Debug/tests/convert_utf8_to_latin1_tests+0x35650) (BuildId: ab54037bea01a246225d9f551b72ba81eb4f6416) 0x517000000307 is located 0 bytes after 647-byte region [0x517000000080,0x517000000307) allocated by thread T0 here: #0 0x561dd0282be1 in operator new(unsigned long) (/home/pauldreik/code/delaktig/simdutf/build/paul_clang_18-Debug/tests/convert_utf8_to_latin1_tests+0x10ebe1) (BuildId: ab54037bea01a246225d9f551b72ba81eb4f6416) #1 0x561dd028f28e in std::__new_allocator<char>::allocate(unsigned long, void const*) /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/new_allocator.h:151:27 #2 0x561dd0290023 in std::allocator<char>::allocate(unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/allocator.h:196:32 simdutf#3 0x561dd0290023 in std::allocator_traits<std::allocator<char>>::allocate(std::allocator<char>&, unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/alloc_traits.h:478:20 simdutf#4 0x561dd0290023 in std::_Vector_base<char, std::allocator<char>>::_M_allocate(unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/stl_vector.h:380:20 simdutf#5 0x561dd028fe20 in std::_Vector_base<char, std::allocator<char>>::_M_create_storage(unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/stl_vector.h:398:33 simdutf#6 0x561dd028f9d1 in std::_Vector_base<char, std::allocator<char>>::_Vector_base(unsigned long, std::allocator<char> const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/stl_vector.h:334:9 simdutf#7 0x561dd028e4d8 in std::vector<char, std::allocator<char>>::vector(unsigned long, std::allocator<char> const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/stl_vector.h:557:9 simdutf#8 0x561dd0286d70 in test_impl_ossfuzz_372067232(simdutf::implementation const&) /home/pauldreik/code/delaktig/simdutf/tests/convert_utf8_to_latin1_tests.cpp:92:21 simdutf#9 0x561dd028516f in ossfuzz_372067232(simdutf::implementation const&) /home/pauldreik/code/delaktig/simdutf/tests/convert_utf8_to_latin1_tests.cpp:20:1 simdutf#10 0x561dd0299782 in simdutf::test::test_entry::operator()(simdutf::implementation const&) /home/pauldreik/code/delaktig/simdutf/tests/helpers/test.h:18:58 simdutf#11 0x561dd02926ca in (anonymous namespace)::run((anonymous namespace)::CommandLine const&) /home/pauldreik/code/delaktig/simdutf/tests/helpers/test.cpp:179:9 simdutf#12 0x561dd0290fc5 in simdutf::test::main(int, char**) /home/pauldreik/code/delaktig/simdutf/tests/helpers/test.cpp:207:3 simdutf#13 0x561dd028d611 in main /home/pauldreik/code/delaktig/simdutf/tests/convert_utf8_to_latin1_tests.cpp:293:1 simdutf#14 0x7f31b415adb9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 SUMMARY: AddressSanitizer: heap-buffer-overflow /home/pauldreik/code/delaktig/simdutf/src/haswell/avx2_convert_utf8_to_latin1.cpp:28:5 in simdutf::haswell::(anonymous namespace)::convert_masked_utf8_to_latin1(char const*, unsigned long, char*&) Shadow bytes around the buggy address: 0x517000000080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x517000000100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x517000000180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x517000000200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x517000000280: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x517000000300:[07]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x517000000380: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x517000000400: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x517000000480: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x517000000500: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x517000000580: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==17876==ABORTING
No description provided.