From b0f832188ddd578ed7f492bd72c1f19903cbfc55 Mon Sep 17 00:00:00 2001 From: ruema Date: Sun, 11 Aug 2024 12:17:08 +0000 Subject: [PATCH 1/9] Improve performance of find_max_char --- ...-08-11-12-06-23.gh-issue-122901.B2Huom.rst | 1 + Objects/stringlib/find_max_char.h | 205 +++++++++--------- 2 files changed, 105 insertions(+), 101 deletions(-) create mode 100644 Misc/NEWS.d/next/C API/2024-08-11-12-06-23.gh-issue-122901.B2Huom.rst diff --git a/Misc/NEWS.d/next/C API/2024-08-11-12-06-23.gh-issue-122901.B2Huom.rst b/Misc/NEWS.d/next/C API/2024-08-11-12-06-23.gh-issue-122901.B2Huom.rst new file mode 100644 index 000000000000000..2b7793d71e1edf2 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2024-08-11-12-06-23.gh-issue-122901.B2Huom.rst @@ -0,0 +1 @@ +Improve performance of :c:func:`find_max_char`. diff --git a/Objects/stringlib/find_max_char.h b/Objects/stringlib/find_max_char.h index 7ab3fc88b331b14..0a7bb3f89076cc6 100644 --- a/Objects/stringlib/find_max_char.h +++ b/Objects/stringlib/find_max_char.h @@ -5,123 +5,129 @@ # error "find_max_char.h is specific to Unicode" #endif +#define MAX_CHAR_ASCII 0x7f +#define MAX_CHAR_UCS1 0xff +#define MAX_CHAR_UCS2 0xffff +#define MAX_CHAR_UCS4 0x10ffff + /* Mask to quickly check whether a C 'size_t' contains a non-ASCII, UTF8-encoded char. */ -#if (SIZEOF_SIZE_T == 8) -# define UCS1_ASCII_CHAR_MASK 0x8080808080808080ULL -#elif (SIZEOF_SIZE_T == 4) -# define UCS1_ASCII_CHAR_MASK 0x80808080U -#else -# error C 'size_t' size should be either 4 or 8! -#endif #if STRINGLIB_SIZEOF_CHAR == 1 +# if SIZEOF_SIZE_T == 8 +# define MASK_ASCII 0x8080808080808080ULL +# elif SIZEOF_SIZE_T == 4 +# define MASK_ASCII 0x80808080U +# else +# error C 'size_t' size should be either 4 or 8! +# endif +# define MASK_MAX_CHAR MASK_ASCII +# define MAX_CHAR MAX_CHAR_UCS1 +#elif STRINGLIB_SIZEOF_CHAR == 2 +# if SIZEOF_SIZE_T == 8 +# define MASK_ASCII 0xFF80FF80FF80FF80ULL +# define MASK_UCS1 0xFF00FF00FF00FF00ULL +# elif SIZEOF_SIZE_T == 4 +# define MASK_ASCII 0xFF80FF80U +# define MASK_UCS1 0xFF00FF00U +# else +# error C 'size_t' size should be either 4 or 8! +# endif +# define MASK_MAX_CHAR MASK_UCS1 +# define MAX_CHAR MAX_CHAR_UCS2 +#elif STRINGLIB_SIZEOF_CHAR == 4 +# if SIZEOF_SIZE_T == 8 +# define MASK_ASCII 0xFFFFFF80FFFFFF80ULL +# define MASK_UCS1 0xFFFFFF00FFFFFF00ULL +# define MASK_UCS2 0xFFFF0000FFFF0000ULL +# elif SIZEOF_SIZE_T == 4 +# define MASK_ASCII 0xFFFFFF80U +# define MASK_UCS1 0xFFFFFF00U +# define MASK_UCS2 0xFFFF0000U +# else +# error C 'size_t' size should be either 4 or 8! +# endif +# define MASK_MAX_CHAR MASK_UCS2 +# define MAX_CHAR MAX_CHAR_UCS4 +#else +#error Invalid STRINGLIB_SIZEOF_CHAR (must be 1, 2 or 4) +#endif Py_LOCAL_INLINE(Py_UCS4) STRINGLIB(find_max_char)(const STRINGLIB_CHAR *begin, const STRINGLIB_CHAR *end) { - const unsigned char *p = (const unsigned char *) begin; + const unsigned char *p = (const unsigned char *)begin; const unsigned char *_end = (const unsigned char *)end; - while (p < _end) { - if (_Py_IS_ALIGNED(p, ALIGNOF_SIZE_T)) { - /* Help register allocation */ - const unsigned char *_p = p; - while (_p + SIZEOF_SIZE_T <= _end) { - size_t value = *(const size_t *) _p; - if (value & UCS1_ASCII_CHAR_MASK) - return 255; - _p += SIZEOF_SIZE_T; - } - p = _p; - if (p == _end) - break; + size_t value = 0; + + if (!_Py_IS_ALIGNED(p, ALIGNOF_SIZE_T) { +#if STRINGLIB_SIZEOF_CHAR <= 1 + if (!_Py_IS_ALIGNED(p, 1)) { + value |= *p++; } - if (*p++ & 0x80) - return 255; +#endif +#if STRINGLIB_SIZEOF_CHAR <= 2 + if (!_Py_IS_ALIGNED(p, sizeof(uint16_t))) { + value |= *(const uint16_t*)p; + p += sizeof(uint16_t); + } +#endif +#if SIZEOF_SIZE_T == 8 + if (!_Py_IS_ALIGNED(p, sizeof(uint32_t))) { + value |= *(const uint32_t*)p; + p += sizeof(uint32_t); + } +#endif } - return 127; -} -#undef ASCII_CHAR_MASK - -#else /* STRINGLIB_SIZEOF_CHAR == 1 */ - -#define MASK_ASCII 0xFFFFFF80 -#define MASK_UCS1 0xFFFFFF00 -#define MASK_UCS2 0xFFFF0000 - -#define MAX_CHAR_ASCII 0x7f -#define MAX_CHAR_UCS1 0xff -#define MAX_CHAR_UCS2 0xffff -#define MAX_CHAR_UCS4 0x10ffff + while (p + SIZEOF_SIZE_T * 32 <= _end) { + const size_t *pp = (const size_t *)p; + for(int i=0; i<32; i++) { + value |= pp[i]; + } + if (value & MASK_MAX_CHAR) { + return MAX_CHAR; + } + p += SIZEOF_SIZE_T * 32; + } -Py_LOCAL_INLINE(Py_UCS4) -STRINGLIB(find_max_char)(const STRINGLIB_CHAR *begin, const STRINGLIB_CHAR *end) -{ -#if STRINGLIB_SIZEOF_CHAR == 2 - const Py_UCS4 mask_limit = MASK_UCS1; - const Py_UCS4 max_char_limit = MAX_CHAR_UCS2; -#elif STRINGLIB_SIZEOF_CHAR == 4 - const Py_UCS4 mask_limit = MASK_UCS2; - const Py_UCS4 max_char_limit = MAX_CHAR_UCS4; -#else -#error Invalid STRINGLIB_SIZEOF_CHAR (must be 1, 2 or 4) + while (p + SIZEOF_SIZE_T <= _end) { + value |= *(const size_t *)p; + p += SIZEOF_SIZE_T; + } +#if SIZEOF_SIZE_T == 8 + if (p + sizeof(uint32_t) <= _end) { + value |= *(const uint32_t*)p; + p += sizeof(uint32_t); + } #endif - Py_UCS4 mask; - Py_ssize_t n = end - begin; - const STRINGLIB_CHAR *p = begin; - const STRINGLIB_CHAR *unrolled_end = begin + _Py_SIZE_ROUND_DOWN(n, 4); - Py_UCS4 max_char; - - max_char = MAX_CHAR_ASCII; - mask = MASK_ASCII; - while (p < unrolled_end) { - STRINGLIB_CHAR bits = p[0] | p[1] | p[2] | p[3]; - if (bits & mask) { - if (mask == mask_limit) { - /* Limit reached */ - return max_char_limit; - } - if (mask == MASK_ASCII) { - max_char = MAX_CHAR_UCS1; - mask = MASK_UCS1; - } - else { - /* mask can't be MASK_UCS2 because of mask_limit above */ - assert(mask == MASK_UCS1); - max_char = MAX_CHAR_UCS2; - mask = MASK_UCS2; - } - /* We check the new mask on the same chars in the next iteration */ - continue; - } - p += 4; +#if STRINGLIB_SIZEOF_CHAR <= 2 + if (p + sizeof(uint16_t) <= _end) { + value |= *(const uint16_t*)p; + p += sizeof(uint16_t); } - while (p < end) { - if (p[0] & mask) { - if (mask == mask_limit) { - /* Limit reached */ - return max_char_limit; - } - if (mask == MASK_ASCII) { - max_char = MAX_CHAR_UCS1; - mask = MASK_UCS1; - } - else { - /* mask can't be MASK_UCS2 because of mask_limit above */ - assert(mask == MASK_UCS1); - max_char = MAX_CHAR_UCS2; - mask = MASK_UCS2; - } - /* We check the new mask on the same chars in the next iteration */ - continue; - } - p++; +#endif +#if STRINGLIB_SIZEOF_CHAR <= 1 + if (p + 1 <= _end) { + value |= *p++; } - return max_char; +#endif +#ifdef MASK_UCS2 + if (value & MASK_UCS2) + return MAX_CHAR_UCS4; +#endif +#ifdef MASK_UCS1 + if (value & MASK_UCS1) + return MAX_CHAR_UCS2; +#endif + if (value & MASK_ASCII) + return MAX_CHAR_UCS1; + return MAX_CHAR_ASCII; } +#undef MASK_MAX_CHAR +#undef MAX_CHAR #undef MASK_ASCII #undef MASK_UCS1 #undef MASK_UCS2 @@ -129,6 +135,3 @@ STRINGLIB(find_max_char)(const STRINGLIB_CHAR *begin, const STRINGLIB_CHAR *end) #undef MAX_CHAR_UCS1 #undef MAX_CHAR_UCS2 #undef MAX_CHAR_UCS4 - -#endif /* STRINGLIB_SIZEOF_CHAR == 1 */ - From a43c1fc69953e2606b9e6aeb2b289a32dd282630 Mon Sep 17 00:00:00 2001 From: ruema Date: Sun, 11 Aug 2024 13:17:25 +0000 Subject: [PATCH 2/9] missing ) added --- .../next/C API/2024-08-11-12-06-23.gh-issue-122901.B2Huom.rst | 2 +- Objects/stringlib/find_max_char.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/C API/2024-08-11-12-06-23.gh-issue-122901.B2Huom.rst b/Misc/NEWS.d/next/C API/2024-08-11-12-06-23.gh-issue-122901.B2Huom.rst index 2b7793d71e1edf2..9c3cb08f755837d 100644 --- a/Misc/NEWS.d/next/C API/2024-08-11-12-06-23.gh-issue-122901.B2Huom.rst +++ b/Misc/NEWS.d/next/C API/2024-08-11-12-06-23.gh-issue-122901.B2Huom.rst @@ -1 +1 @@ -Improve performance of :c:func:`find_max_char`. +Improve performance of ``find_max_char``. diff --git a/Objects/stringlib/find_max_char.h b/Objects/stringlib/find_max_char.h index 0a7bb3f89076cc6..ff5088bf4d82bc9 100644 --- a/Objects/stringlib/find_max_char.h +++ b/Objects/stringlib/find_max_char.h @@ -61,7 +61,7 @@ STRINGLIB(find_max_char)(const STRINGLIB_CHAR *begin, const STRINGLIB_CHAR *end) size_t value = 0; - if (!_Py_IS_ALIGNED(p, ALIGNOF_SIZE_T) { + if (!_Py_IS_ALIGNED(p, ALIGNOF_SIZE_T)) { #if STRINGLIB_SIZEOF_CHAR <= 1 if (!_Py_IS_ALIGNED(p, 1)) { value |= *p++; From d597175261366d38546cc8010fd76f3f2a74d32b Mon Sep 17 00:00:00 2001 From: ruema Date: Sun, 11 Aug 2024 14:58:55 +0000 Subject: [PATCH 3/9] bugfix for unaligned short strings. --- Objects/stringlib/find_max_char.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Objects/stringlib/find_max_char.h b/Objects/stringlib/find_max_char.h index ff5088bf4d82bc9..10ba11769a3a7db 100644 --- a/Objects/stringlib/find_max_char.h +++ b/Objects/stringlib/find_max_char.h @@ -63,18 +63,18 @@ STRINGLIB(find_max_char)(const STRINGLIB_CHAR *begin, const STRINGLIB_CHAR *end) if (!_Py_IS_ALIGNED(p, ALIGNOF_SIZE_T)) { #if STRINGLIB_SIZEOF_CHAR <= 1 - if (!_Py_IS_ALIGNED(p, 1)) { + if (!_Py_IS_ALIGNED(p, 1) && p + 1 <= _end) { value |= *p++; } #endif #if STRINGLIB_SIZEOF_CHAR <= 2 - if (!_Py_IS_ALIGNED(p, sizeof(uint16_t))) { + if (!_Py_IS_ALIGNED(p, sizeof(uint16_t)) && p + sizeof(uint16_t) <= _end) { value |= *(const uint16_t*)p; p += sizeof(uint16_t); } #endif #if SIZEOF_SIZE_T == 8 - if (!_Py_IS_ALIGNED(p, sizeof(uint32_t))) { + if (!_Py_IS_ALIGNED(p, sizeof(uint32_t)) && p + sizeof(uint32_t) <= _end) { value |= *(const uint32_t*)p; p += sizeof(uint32_t); } @@ -113,6 +113,7 @@ STRINGLIB(find_max_char)(const STRINGLIB_CHAR *begin, const STRINGLIB_CHAR *end) value |= *p++; } #endif + #ifdef MASK_UCS2 if (value & MASK_UCS2) return MAX_CHAR_UCS4; From fe362fe54236e3e0696ca4ef327be787d0424bbb Mon Sep 17 00:00:00 2001 From: Stan Ulbrych Date: Fri, 30 Oct 2026 14:20:42 +0000 Subject: [PATCH 4/9] Re-write and move the news entry --- .../next/C API/2024-08-11-12-06-23.gh-issue-122901.B2Huom.rst | 1 - .../2024-08-11-12-06-23.gh-issue-122901.B2Huom.rst | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) delete mode 100644 Misc/NEWS.d/next/C API/2024-08-11-12-06-23.gh-issue-122901.B2Huom.rst create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2024-08-11-12-06-23.gh-issue-122901.B2Huom.rst diff --git a/Misc/NEWS.d/next/C API/2024-08-11-12-06-23.gh-issue-122901.B2Huom.rst b/Misc/NEWS.d/next/C API/2024-08-11-12-06-23.gh-issue-122901.B2Huom.rst deleted file mode 100644 index 9c3cb08f755837d..000000000000000 --- a/Misc/NEWS.d/next/C API/2024-08-11-12-06-23.gh-issue-122901.B2Huom.rst +++ /dev/null @@ -1 +0,0 @@ -Improve performance of ``find_max_char``. diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-08-11-12-06-23.gh-issue-122901.B2Huom.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-08-11-12-06-23.gh-issue-122901.B2Huom.rst new file mode 100644 index 000000000000000..734bbc63f381ab4 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2024-08-11-12-06-23.gh-issue-122901.B2Huom.rst @@ -0,0 +1 @@ +Speed up creating :class:`str` objects from non-ASCII character data. From 1650b4b2914b2b805e07d55576f830592f8f3f6a Mon Sep 17 00:00:00 2001 From: Stan Ulbrych Date: Fri, 30 Oct 2026 14:33:09 +0000 Subject: [PATCH 5/9] Fix alignment prologue to actually align the pointer (+ some PEP 7 nits) --- Objects/stringlib/find_max_char.h | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/Objects/stringlib/find_max_char.h b/Objects/stringlib/find_max_char.h index 10ba11769a3a7db..54b9c0d28ca08c7 100644 --- a/Objects/stringlib/find_max_char.h +++ b/Objects/stringlib/find_max_char.h @@ -12,7 +12,6 @@ /* Mask to quickly check whether a C 'size_t' contains a non-ASCII, UTF8-encoded char. */ - #if STRINGLIB_SIZEOF_CHAR == 1 # if SIZEOF_SIZE_T == 8 # define MASK_ASCII 0x8080808080808080ULL @@ -53,6 +52,12 @@ #error Invalid STRINGLIB_SIZEOF_CHAR (must be 1, 2 or 4) #endif +/* OR all characters into a single size_t accumulator and test the high bits + at the end, instead of comparing each character. The accumulation loop has + no branches, so compilers vectorize it, and the result does not depend on + where each byte sits in the word, so unaligned ends can be read in smaller + pieces. Every 32 words, check whether the widest kind is already known and + exit early. */ Py_LOCAL_INLINE(Py_UCS4) STRINGLIB(find_max_char)(const STRINGLIB_CHAR *begin, const STRINGLIB_CHAR *end) { @@ -63,18 +68,18 @@ STRINGLIB(find_max_char)(const STRINGLIB_CHAR *begin, const STRINGLIB_CHAR *end) if (!_Py_IS_ALIGNED(p, ALIGNOF_SIZE_T)) { #if STRINGLIB_SIZEOF_CHAR <= 1 - if (!_Py_IS_ALIGNED(p, 1) && p + 1 <= _end) { + if (!_Py_IS_ALIGNED(p, sizeof(uint16_t)) && p + 1 <= _end) { value |= *p++; } #endif #if STRINGLIB_SIZEOF_CHAR <= 2 - if (!_Py_IS_ALIGNED(p, sizeof(uint16_t)) && p + sizeof(uint16_t) <= _end) { + if (!_Py_IS_ALIGNED(p, sizeof(uint32_t)) && p + sizeof(uint16_t) <= _end) { value |= *(const uint16_t*)p; p += sizeof(uint16_t); } #endif #if SIZEOF_SIZE_T == 8 - if (!_Py_IS_ALIGNED(p, sizeof(uint32_t)) && p + sizeof(uint32_t) <= _end) { + if (!_Py_IS_ALIGNED(p, SIZEOF_SIZE_T) && p + sizeof(uint32_t) <= _end) { value |= *(const uint32_t*)p; p += sizeof(uint32_t); } @@ -83,7 +88,7 @@ STRINGLIB(find_max_char)(const STRINGLIB_CHAR *begin, const STRINGLIB_CHAR *end) while (p + SIZEOF_SIZE_T * 32 <= _end) { const size_t *pp = (const size_t *)p; - for(int i=0; i<32; i++) { + for(int i = 0; i < 32; i++) { value |= pp[i]; } if (value & MASK_MAX_CHAR) { @@ -115,15 +120,18 @@ STRINGLIB(find_max_char)(const STRINGLIB_CHAR *begin, const STRINGLIB_CHAR *end) #endif #ifdef MASK_UCS2 - if (value & MASK_UCS2) + if (value & MASK_UCS2) { return MAX_CHAR_UCS4; + } #endif #ifdef MASK_UCS1 - if (value & MASK_UCS1) + if (value & MASK_UCS1) { return MAX_CHAR_UCS2; + } #endif - if (value & MASK_ASCII) + if (value & MASK_ASCII) { return MAX_CHAR_UCS1; + } return MAX_CHAR_ASCII; } From f524f84c4650094dc34b217bbaaabf00bb0171ec Mon Sep 17 00:00:00 2001 From: Stan Ulbrych Date: Fri, 30 Oct 2026 14:39:28 +0000 Subject: [PATCH 6/9] Little nits --- Objects/stringlib/find_max_char.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Objects/stringlib/find_max_char.h b/Objects/stringlib/find_max_char.h index 54b9c0d28ca08c7..7b0bfd57a2069bb 100644 --- a/Objects/stringlib/find_max_char.h +++ b/Objects/stringlib/find_max_char.h @@ -10,8 +10,8 @@ #define MAX_CHAR_UCS2 0xffff #define MAX_CHAR_UCS4 0x10ffff -/* Mask to quickly check whether a C 'size_t' contains a - non-ASCII, UTF8-encoded char. */ +/* Masks selecting the bits that are set in a C 'size_t' holding several + characters when at least one of them is wider than ASCII, UCS1 or UCS2. */ #if STRINGLIB_SIZEOF_CHAR == 1 # if SIZEOF_SIZE_T == 8 # define MASK_ASCII 0x8080808080808080ULL @@ -88,7 +88,7 @@ STRINGLIB(find_max_char)(const STRINGLIB_CHAR *begin, const STRINGLIB_CHAR *end) while (p + SIZEOF_SIZE_T * 32 <= _end) { const size_t *pp = (const size_t *)p; - for(int i = 0; i < 32; i++) { + for (int i = 0; i < 32; i++) { value |= pp[i]; } if (value & MASK_MAX_CHAR) { From 1daa7ee9cd0c46f37a82997ff367c1f6389badf6 Mon Sep 17 00:00:00 2001 From: Stan Ulbrych Date: Fri, 30 Oct 2026 19:13:38 +0000 Subject: [PATCH 7/9] Use four accumulators and a 16-word block so the loop is fast when not vectorized --- Objects/stringlib/find_max_char.h | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/Objects/stringlib/find_max_char.h b/Objects/stringlib/find_max_char.h index 7b0bfd57a2069bb..94fe2d45c5dba27 100644 --- a/Objects/stringlib/find_max_char.h +++ b/Objects/stringlib/find_max_char.h @@ -56,7 +56,7 @@ at the end, instead of comparing each character. The accumulation loop has no branches, so compilers vectorize it, and the result does not depend on where each byte sits in the word, so unaligned ends can be read in smaller - pieces. Every 32 words, check whether the widest kind is already known and + pieces. Every 16 words, check whether the widest kind is already known and exit early. */ Py_LOCAL_INLINE(Py_UCS4) STRINGLIB(find_max_char)(const STRINGLIB_CHAR *begin, const STRINGLIB_CHAR *end) @@ -86,15 +86,22 @@ STRINGLIB(find_max_char)(const STRINGLIB_CHAR *begin, const STRINGLIB_CHAR *end) #endif } - while (p + SIZEOF_SIZE_T * 32 <= _end) { + while (p + SIZEOF_SIZE_T * 16 <= _end) { const size_t *pp = (const size_t *)p; - for (int i = 0; i < 32; i++) { - value |= pp[i]; + /* Four independent accumulators so the loop does not serialize on + one register when the compiler does not vectorize it. */ + size_t v0 = 0, v1 = 0, v2 = 0, v3 = 0; + for (int i = 0; i < 16; i += 4) { + v0 |= pp[i]; + v1 |= pp[i + 1]; + v2 |= pp[i + 2]; + v3 |= pp[i + 3]; } + value |= v0 | v1 | v2 | v3; if (value & MASK_MAX_CHAR) { return MAX_CHAR; } - p += SIZEOF_SIZE_T * 32; + p += SIZEOF_SIZE_T * 16; } while (p + SIZEOF_SIZE_T <= _end) { From c0f99d1efd23470fa685e7d8007b84ab5e2b2ec4 Mon Sep 17 00:00:00 2001 From: Stan Ulbrych Date: Sat, 31 Oct 2026 18:18:19 +0000 Subject: [PATCH 8/9] Load words with `memcpy` and finish with one overlapping read --- Objects/stringlib/find_max_char.h | 86 +++++++++++++++---------------- 1 file changed, 42 insertions(+), 44 deletions(-) diff --git a/Objects/stringlib/find_max_char.h b/Objects/stringlib/find_max_char.h index 94fe2d45c5dba27..3b7bc069bdf6e97 100644 --- a/Objects/stringlib/find_max_char.h +++ b/Objects/stringlib/find_max_char.h @@ -54,48 +54,31 @@ /* OR all characters into a single size_t accumulator and test the high bits at the end, instead of comparing each character. The accumulation loop has - no branches, so compilers vectorize it, and the result does not depend on - where each byte sits in the word, so unaligned ends can be read in smaller - pieces. Every 16 words, check whether the widest kind is already known and - exit early. */ + no branches, so compilers vectorize it. The words are loaded with memcpy, + so the data does not need to be aligned, and the tail re-reads the last + word of the buffer, which may overlap data already seen. Every 16 words, + check whether the widest kind is already known and exit early. */ Py_LOCAL_INLINE(Py_UCS4) STRINGLIB(find_max_char)(const STRINGLIB_CHAR *begin, const STRINGLIB_CHAR *end) { const unsigned char *p = (const unsigned char *)begin; const unsigned char *_end = (const unsigned char *)end; - size_t value = 0; - if (!_Py_IS_ALIGNED(p, ALIGNOF_SIZE_T)) { -#if STRINGLIB_SIZEOF_CHAR <= 1 - if (!_Py_IS_ALIGNED(p, sizeof(uint16_t)) && p + 1 <= _end) { - value |= *p++; - } -#endif -#if STRINGLIB_SIZEOF_CHAR <= 2 - if (!_Py_IS_ALIGNED(p, sizeof(uint32_t)) && p + sizeof(uint16_t) <= _end) { - value |= *(const uint16_t*)p; - p += sizeof(uint16_t); - } -#endif -#if SIZEOF_SIZE_T == 8 - if (!_Py_IS_ALIGNED(p, SIZEOF_SIZE_T) && p + sizeof(uint32_t) <= _end) { - value |= *(const uint32_t*)p; - p += sizeof(uint32_t); - } -#endif - } - while (p + SIZEOF_SIZE_T * 16 <= _end) { - const size_t *pp = (const size_t *)p; /* Four independent accumulators so the loop does not serialize on one register when the compiler does not vectorize it. */ size_t v0 = 0, v1 = 0, v2 = 0, v3 = 0; - for (int i = 0; i < 16; i += 4) { - v0 |= pp[i]; - v1 |= pp[i + 1]; - v2 |= pp[i + 2]; - v3 |= pp[i + 3]; + for (int i = 0; i < 16 * SIZEOF_SIZE_T; i += 4 * SIZEOF_SIZE_T) { + size_t a, b, c, d; + memcpy(&a, p + i, SIZEOF_SIZE_T); + memcpy(&b, p + i + SIZEOF_SIZE_T, SIZEOF_SIZE_T); + memcpy(&c, p + i + 2 * SIZEOF_SIZE_T, SIZEOF_SIZE_T); + memcpy(&d, p + i + 3 * SIZEOF_SIZE_T, SIZEOF_SIZE_T); + v0 |= a; + v1 |= b; + v2 |= c; + v3 |= d; } value |= v0 | v1 | v2 | v3; if (value & MASK_MAX_CHAR) { @@ -105,26 +88,41 @@ STRINGLIB(find_max_char)(const STRINGLIB_CHAR *begin, const STRINGLIB_CHAR *end) } while (p + SIZEOF_SIZE_T <= _end) { - value |= *(const size_t *)p; + size_t a; + memcpy(&a, p, SIZEOF_SIZE_T); + value |= a; p += SIZEOF_SIZE_T; } + + if (p < _end) { + size_t n = (size_t)(_end - (const unsigned char *)begin); + if (n >= SIZEOF_SIZE_T) { + size_t a; + memcpy(&a, _end - SIZEOF_SIZE_T, SIZEOF_SIZE_T); + value |= a; + } #if SIZEOF_SIZE_T == 8 - if (p + sizeof(uint32_t) <= _end) { - value |= *(const uint32_t*)p; - p += sizeof(uint32_t); - } + else if (n >= 4) { + uint32_t a, b; + memcpy(&a, p, 4); + memcpy(&b, _end - 4, 4); + value |= a | b; + } #endif #if STRINGLIB_SIZEOF_CHAR <= 2 - if (p + sizeof(uint16_t) <= _end) { - value |= *(const uint16_t*)p; - p += sizeof(uint16_t); - } + else if (n >= 2) { + uint16_t a, b; + memcpy(&a, p, 2); + memcpy(&b, _end - 2, 2); + value |= a | b; + } #endif -#if STRINGLIB_SIZEOF_CHAR <= 1 - if (p + 1 <= _end) { - value |= *p++; - } +#if STRINGLIB_SIZEOF_CHAR == 1 + else { + value |= *p; + } #endif + } #ifdef MASK_UCS2 if (value & MASK_UCS2) { From c14a157ded1c70a5fb327932d2399a16a1e112c7 Mon Sep 17 00:00:00 2001 From: Stan Ulbrych Date: Sat, 31 Oct 2026 18:49:57 +0000 Subject: [PATCH 9/9] =?UTF-8?q?B=C3=A9n=C3=A9dikt's=20review=20suggestions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Objects/stringlib/find_max_char.h | 86 +++++++++++++++---------------- 1 file changed, 43 insertions(+), 43 deletions(-) diff --git a/Objects/stringlib/find_max_char.h b/Objects/stringlib/find_max_char.h index 3b7bc069bdf6e97..ca9791ef1d5693a 100644 --- a/Objects/stringlib/find_max_char.h +++ b/Objects/stringlib/find_max_char.h @@ -13,43 +13,43 @@ /* Masks selecting the bits that are set in a C 'size_t' holding several characters when at least one of them is wider than ASCII, UCS1 or UCS2. */ #if STRINGLIB_SIZEOF_CHAR == 1 -# if SIZEOF_SIZE_T == 8 -# define MASK_ASCII 0x8080808080808080ULL -# elif SIZEOF_SIZE_T == 4 -# define MASK_ASCII 0x80808080U -# else -# error C 'size_t' size should be either 4 or 8! -# endif -# define MASK_MAX_CHAR MASK_ASCII -# define MAX_CHAR MAX_CHAR_UCS1 +# if SIZEOF_SIZE_T == 8 +# define MASK_ASCII 0x8080808080808080ULL +# elif SIZEOF_SIZE_T == 4 +# define MASK_ASCII 0x80808080U +# else +# error C 'size_t' size should be either 4 or 8! +# endif +# define MASK_MAX_CHAR MASK_ASCII +# define MAX_CHAR MAX_CHAR_UCS1 #elif STRINGLIB_SIZEOF_CHAR == 2 -# if SIZEOF_SIZE_T == 8 -# define MASK_ASCII 0xFF80FF80FF80FF80ULL -# define MASK_UCS1 0xFF00FF00FF00FF00ULL -# elif SIZEOF_SIZE_T == 4 -# define MASK_ASCII 0xFF80FF80U -# define MASK_UCS1 0xFF00FF00U -# else -# error C 'size_t' size should be either 4 or 8! -# endif -# define MASK_MAX_CHAR MASK_UCS1 -# define MAX_CHAR MAX_CHAR_UCS2 +# if SIZEOF_SIZE_T == 8 +# define MASK_ASCII 0xFF80FF80FF80FF80ULL +# define MASK_UCS1 0xFF00FF00FF00FF00ULL +# elif SIZEOF_SIZE_T == 4 +# define MASK_ASCII 0xFF80FF80U +# define MASK_UCS1 0xFF00FF00U +# else +# error C 'size_t' size should be either 4 or 8! +# endif +# define MASK_MAX_CHAR MASK_UCS1 +# define MAX_CHAR MAX_CHAR_UCS2 #elif STRINGLIB_SIZEOF_CHAR == 4 -# if SIZEOF_SIZE_T == 8 -# define MASK_ASCII 0xFFFFFF80FFFFFF80ULL -# define MASK_UCS1 0xFFFFFF00FFFFFF00ULL -# define MASK_UCS2 0xFFFF0000FFFF0000ULL -# elif SIZEOF_SIZE_T == 4 -# define MASK_ASCII 0xFFFFFF80U -# define MASK_UCS1 0xFFFFFF00U -# define MASK_UCS2 0xFFFF0000U -# else -# error C 'size_t' size should be either 4 or 8! -# endif -# define MASK_MAX_CHAR MASK_UCS2 -# define MAX_CHAR MAX_CHAR_UCS4 +# if SIZEOF_SIZE_T == 8 +# define MASK_ASCII 0xFFFFFF80FFFFFF80ULL +# define MASK_UCS1 0xFFFFFF00FFFFFF00ULL +# define MASK_UCS2 0xFFFF0000FFFF0000ULL +# elif SIZEOF_SIZE_T == 4 +# define MASK_ASCII 0xFFFFFF80U +# define MASK_UCS1 0xFFFFFF00U +# define MASK_UCS2 0xFFFF0000U +# else +# error C 'size_t' size should be either 4 or 8! +# endif +# define MASK_MAX_CHAR MASK_UCS2 +# define MAX_CHAR MAX_CHAR_UCS4 #else -#error Invalid STRINGLIB_SIZEOF_CHAR (must be 1, 2 or 4) +# error Invalid STRINGLIB_SIZEOF_CHAR (must be 1, 2 or 4) #endif /* OR all characters into a single size_t accumulator and test the high bits @@ -70,15 +70,15 @@ STRINGLIB(find_max_char)(const STRINGLIB_CHAR *begin, const STRINGLIB_CHAR *end) one register when the compiler does not vectorize it. */ size_t v0 = 0, v1 = 0, v2 = 0, v3 = 0; for (int i = 0; i < 16 * SIZEOF_SIZE_T; i += 4 * SIZEOF_SIZE_T) { - size_t a, b, c, d; - memcpy(&a, p + i, SIZEOF_SIZE_T); - memcpy(&b, p + i + SIZEOF_SIZE_T, SIZEOF_SIZE_T); - memcpy(&c, p + i + 2 * SIZEOF_SIZE_T, SIZEOF_SIZE_T); - memcpy(&d, p + i + 3 * SIZEOF_SIZE_T, SIZEOF_SIZE_T); - v0 |= a; - v1 |= b; - v2 |= c; - v3 |= d; + size_t w0, w1, w2, w3; + memcpy(&w0, p + i, SIZEOF_SIZE_T); + memcpy(&w1, p + i + SIZEOF_SIZE_T, SIZEOF_SIZE_T); + memcpy(&w2, p + i + 2*SIZEOF_SIZE_T, SIZEOF_SIZE_T); + memcpy(&w3, p + i + 3*SIZEOF_SIZE_T, SIZEOF_SIZE_T); + v0 |= w0; + v1 |= w1; + v2 |= w2; + v3 |= w3; } value |= v0 | v1 | v2 | v3; if (value & MASK_MAX_CHAR) {