| Message ID | 20240424225705.929812-10-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Optimize buffer_is_zero | expand |
Cc'ing Ard :) On 25/4/24 00:57, Richard Henderson wrote: > Because non-embedded aarch64 is expected to have AdvSIMD enabled, merely > double-check with the compiler flags for __ARM_NEON and don't bother with > a runtime check. Otherwise, model the loop after the x86 SSE2 function. > > Use UMAXV for the vector reduction. This is 3 cycles on cortex-a76 and > 2 cycles on neoverse-n1. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > util/bufferiszero.c | 77 +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 77 insertions(+) > > diff --git a/util/bufferiszero.c b/util/bufferiszero.c > index ff003dc40e..38477a3eac 100644 > --- a/util/bufferiszero.c > +++ b/util/bufferiszero.c > @@ -213,7 +213,84 @@ bool test_buffer_is_zero_next_accel(void) > } > return false; > } > + > +#elif defined(__aarch64__) && defined(__ARM_NEON) > +#include <arm_neon.h> > + > +#define REASSOC_BARRIER(vec0, vec1) asm("" : "+w"(vec0), "+w"(vec1)) > + > +static bool buffer_is_zero_simd(const void *buf, size_t len) > +{ > + uint32x4_t t0, t1, t2, t3; > + > + /* Align head/tail to 16-byte boundaries. */ > + const uint32x4_t *p = QEMU_ALIGN_PTR_DOWN(buf + 16, 16); > + const uint32x4_t *e = QEMU_ALIGN_PTR_DOWN(buf + len - 1, 16); > + > + /* Unaligned loads at head/tail. */ > + t0 = vld1q_u32(buf) | vld1q_u32(buf + len - 16); > + > + /* Collect a partial block at tail end. */ > + t1 = e[-7] | e[-6]; > + t2 = e[-5] | e[-4]; > + t3 = e[-3] | e[-2]; > + t0 |= e[-1]; > + REASSOC_BARRIER(t0, t1); > + REASSOC_BARRIER(t2, t3); > + t0 |= t1; > + t2 |= t3; > + REASSOC_BARRIER(t0, t2); > + t0 |= t2; > + > + /* > + * Loop over complete 128-byte blocks. > + * With the head and tail removed, e - p >= 14, so the loop > + * must iterate at least once. > + */ > + do { > + /* > + * Reduce via UMAXV. Whatever the actual result, > + * it will only be zero if all input bytes are zero. > + */ > + if (unlikely(vmaxvq_u32(t0) != 0)) { > + return false; > + } > + > + t0 = p[0] | p[1]; > + t1 = p[2] | p[3]; > + t2 = p[4] | p[5]; > + t3 = p[6] | p[7]; > + REASSOC_BARRIER(t0, t1); > + REASSOC_BARRIER(t2, t3); > + t0 |= t1; > + t2 |= t3; > + REASSOC_BARRIER(t0, t2); > + t0 |= t2; > + p += 8; > + } while (p < e - 7); > + > + return vmaxvq_u32(t0) == 0; > +} > + > +static biz_accel_fn const accel_table[] = { > + buffer_is_zero_int_ge256, > + buffer_is_zero_simd, > +}; > + > +static unsigned accel_index = 1; > +#define INIT_ACCEL buffer_is_zero_simd > + > +bool test_buffer_is_zero_next_accel(void) > +{ > + if (accel_index != 0) { > + buffer_is_zero_accel = accel_table[--accel_index]; > + return true; > + } > + return false; > +} > + > #else > + > bool test_buffer_is_zero_next_accel(void) > { > return false;
On 25/4/24 00:57, Richard Henderson wrote: > Because non-embedded aarch64 is expected to have AdvSIMD enabled, merely > double-check with the compiler flags for __ARM_NEON and don't bother with > a runtime check. Otherwise, model the loop after the x86 SSE2 function. > > Use UMAXV for the vector reduction. This is 3 cycles on cortex-a76 and > 2 cycles on neoverse-n1. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > util/bufferiszero.c | 77 +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 77 insertions(+) > > diff --git a/util/bufferiszero.c b/util/bufferiszero.c > index ff003dc40e..38477a3eac 100644 > --- a/util/bufferiszero.c > +++ b/util/bufferiszero.c > @@ -213,7 +213,84 @@ bool test_buffer_is_zero_next_accel(void) > } > return false; > } > + > +#elif defined(__aarch64__) && defined(__ARM_NEON) > +#include <arm_neon.h> > + Maybe use the same SSE comment: /* * Helper for preventing the compiler from reassociating * chains of binary vector operations. */ > +#define REASSOC_BARRIER(vec0, vec1) asm("" : "+w"(vec0), "+w"(vec1)) > +static unsigned accel_index = 1; > +#define INIT_ACCEL buffer_is_zero_simd > + > +bool test_buffer_is_zero_next_accel(void) > +{ > + if (accel_index != 0) { > + buffer_is_zero_accel = accel_table[--accel_index]; > + return true; > + } > + return false; > +} Alternatively we could initialize accel_index in __attribute__((constructor)) init_accel(void) and keep a single test_buffer_is_zero_next_accel(), squashing: -- >8 -- diff --git a/util/bufferiszero.c b/util/bufferiszero.c index 38477a3eac..afb3822251 100644 --- a/util/bufferiszero.c +++ b/util/bufferiszero.c @@ -82,6 +82,17 @@ static bool buffer_is_zero_int_ge256(const void *buf, size_t len) return t == 0; } +static unsigned accel_index; + +bool test_buffer_is_zero_next_accel(void) +{ + if (accel_index != 0) { + buffer_is_zero_accel = accel_table[--accel_index]; + return true; + } + return false; +} + #if defined(CONFIG_AVX2_OPT) || defined(__SSE2__) #include <immintrin.h> @@ -186,7 +197,6 @@ static biz_accel_fn const accel_table[] = { buffer_zero_avx2, #endif }; -static unsigned accel_index; static void __attribute__((constructor)) init_accel(void) { @@ -205,15 +215,6 @@ static void __attribute__((constructor)) init_accel(void) #define INIT_ACCEL NULL -bool test_buffer_is_zero_next_accel(void) -{ - if (accel_index != 0) { - buffer_is_zero_accel = accel_table[--accel_index]; - return true; - } - return false; -} - #elif defined(__aarch64__) && defined(__ARM_NEON) #include <arm_neon.h> @@ -277,25 +278,15 @@ static biz_accel_fn const accel_table[] = { buffer_is_zero_simd, }; -static unsigned accel_index = 1; #define INIT_ACCEL buffer_is_zero_simd -bool test_buffer_is_zero_next_accel(void) +static void __attribute__((constructor)) init_accel(void) { - if (accel_index != 0) { - buffer_is_zero_accel = accel_table[--accel_index]; - return true; - } - return false; + accel_index = 1; } #else -bool test_buffer_is_zero_next_accel(void) -{ - return false; -} - #define INIT_ACCEL buffer_is_zero_int_ge256 #endif --- Or clearer in 2 patches, unifying test_buffer_is_zero_next_accel() first: -- >8 -- diff --git a/util/bufferiszero.c b/util/bufferiszero.c index ff003dc40e..b4da9d5297 100644 --- a/util/bufferiszero.c +++ b/util/bufferiszero.c @@ -82,6 +82,17 @@ static bool buffer_is_zero_int_ge256(const void *buf, size_t len) return t == 0; } +static unsigned accel_index; + +bool test_buffer_is_zero_next_accel(void) +{ + if (accel_index != 0) { + buffer_is_zero_accel = accel_table[--accel_index]; + return true; + } + return false; +} + #if defined(CONFIG_AVX2_OPT) || defined(__SSE2__) #include <immintrin.h> @@ -186,7 +197,6 @@ static biz_accel_fn const accel_table[] = { buffer_zero_avx2, #endif }; -static unsigned accel_index; static void __attribute__((constructor)) init_accel(void) { @@ -205,19 +215,7 @@ static void __attribute__((constructor)) init_accel(void) #define INIT_ACCEL NULL -bool test_buffer_is_zero_next_accel(void) -{ - if (accel_index != 0) { - buffer_is_zero_accel = accel_table[--accel_index]; - return true; - } - return false; -} #else -bool test_buffer_is_zero_next_accel(void) -{ - return false; -} #define INIT_ACCEL buffer_is_zero_int_ge256 #endif --- Then this patch becomes: -- >8 -- diff --git a/util/bufferiszero.c b/util/bufferiszero.c index b4da9d5297..afb3822251 100644 --- a/util/bufferiszero.c +++ b/util/bufferiszero.c @@ -215,6 +215,76 @@ static void __attribute__((constructor)) init_accel(void) #define INIT_ACCEL NULL +#elif defined(__aarch64__) && defined(__ARM_NEON) +#include <arm_neon.h> + +#define REASSOC_BARRIER(vec0, vec1) asm("" : "+w"(vec0), "+w"(vec1)) + +static bool buffer_is_zero_simd(const void *buf, size_t len) +{ + uint32x4_t t0, t1, t2, t3; + + /* Align head/tail to 16-byte boundaries. */ + const uint32x4_t *p = QEMU_ALIGN_PTR_DOWN(buf + 16, 16); + const uint32x4_t *e = QEMU_ALIGN_PTR_DOWN(buf + len - 1, 16); + + /* Unaligned loads at head/tail. */ + t0 = vld1q_u32(buf) | vld1q_u32(buf + len - 16); + + /* Collect a partial block at tail end. */ + t1 = e[-7] | e[-6]; + t2 = e[-5] | e[-4]; + t3 = e[-3] | e[-2]; + t0 |= e[-1]; + REASSOC_BARRIER(t0, t1); + REASSOC_BARRIER(t2, t3); + t0 |= t1; + t2 |= t3; + REASSOC_BARRIER(t0, t2); + t0 |= t2; + + /* + * Loop over complete 128-byte blocks. + * With the head and tail removed, e - p >= 14, so the loop + * must iterate at least once. + */ + do { + /* + * Reduce via UMAXV. Whatever the actual result, + * it will only be zero if all input bytes are zero. + */ + if (unlikely(vmaxvq_u32(t0) != 0)) { + return false; + } + + t0 = p[0] | p[1]; + t1 = p[2] | p[3]; + t2 = p[4] | p[5]; + t3 = p[6] | p[7]; + REASSOC_BARRIER(t0, t1); + REASSOC_BARRIER(t2, t3); + t0 |= t1; + t2 |= t3; + REASSOC_BARRIER(t0, t2); + t0 |= t2; + p += 8; + } while (p < e - 7); + + return vmaxvq_u32(t0) == 0; +} + +static biz_accel_fn const accel_table[] = { + buffer_is_zero_int_ge256, + buffer_is_zero_simd, +}; + +#define INIT_ACCEL buffer_is_zero_simd + +static void __attribute__((constructor)) init_accel(void) +{ + accel_index = 1; +} + #else #define INIT_ACCEL buffer_is_zero_int_ge256 --- Anyhow, Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
diff --git a/util/bufferiszero.c b/util/bufferiszero.c index ff003dc40e..38477a3eac 100644 --- a/util/bufferiszero.c +++ b/util/bufferiszero.c @@ -213,7 +213,84 @@ bool test_buffer_is_zero_next_accel(void) } return false; } + +#elif defined(__aarch64__) && defined(__ARM_NEON) +#include <arm_neon.h> + +#define REASSOC_BARRIER(vec0, vec1) asm("" : "+w"(vec0), "+w"(vec1)) + +static bool buffer_is_zero_simd(const void *buf, size_t len) +{ + uint32x4_t t0, t1, t2, t3; + + /* Align head/tail to 16-byte boundaries. */ + const uint32x4_t *p = QEMU_ALIGN_PTR_DOWN(buf + 16, 16); + const uint32x4_t *e = QEMU_ALIGN_PTR_DOWN(buf + len - 1, 16); + + /* Unaligned loads at head/tail. */ + t0 = vld1q_u32(buf) | vld1q_u32(buf + len - 16); + + /* Collect a partial block at tail end. */ + t1 = e[-7] | e[-6]; + t2 = e[-5] | e[-4]; + t3 = e[-3] | e[-2]; + t0 |= e[-1]; + REASSOC_BARRIER(t0, t1); + REASSOC_BARRIER(t2, t3); + t0 |= t1; + t2 |= t3; + REASSOC_BARRIER(t0, t2); + t0 |= t2; + + /* + * Loop over complete 128-byte blocks. + * With the head and tail removed, e - p >= 14, so the loop + * must iterate at least once. + */ + do { + /* + * Reduce via UMAXV. Whatever the actual result, + * it will only be zero if all input bytes are zero. + */ + if (unlikely(vmaxvq_u32(t0) != 0)) { + return false; + } + + t0 = p[0] | p[1]; + t1 = p[2] | p[3]; + t2 = p[4] | p[5]; + t3 = p[6] | p[7]; + REASSOC_BARRIER(t0, t1); + REASSOC_BARRIER(t2, t3); + t0 |= t1; + t2 |= t3; + REASSOC_BARRIER(t0, t2); + t0 |= t2; + p += 8; + } while (p < e - 7); + + return vmaxvq_u32(t0) == 0; +} + +static biz_accel_fn const accel_table[] = { + buffer_is_zero_int_ge256, + buffer_is_zero_simd, +}; + +static unsigned accel_index = 1; +#define INIT_ACCEL buffer_is_zero_simd + +bool test_buffer_is_zero_next_accel(void) +{ + if (accel_index != 0) { + buffer_is_zero_accel = accel_table[--accel_index]; + return true; + } + return false; +} + #else + bool test_buffer_is_zero_next_accel(void) { return false;
Because non-embedded aarch64 is expected to have AdvSIMD enabled, merely double-check with the compiler flags for __ARM_NEON and don't bother with a runtime check. Otherwise, model the loop after the x86 SSE2 function. Use UMAXV for the vector reduction. This is 3 cycles on cortex-a76 and 2 cycles on neoverse-n1. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- util/bufferiszero.c | 77 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+)