]> git.ipfire.org Git - thirdparty/glibc.git/commitdiff
x86: Code cleanup in strchr-avx2 and comment justifying branch
authorNoah Goldstein <goldstein.w.n@gmail.com>
Wed, 23 Mar 2022 21:57:16 +0000 (16:57 -0500)
committerNoah Goldstein <goldstein.w.n@gmail.com>
Fri, 25 Mar 2022 16:46:13 +0000 (11:46 -0500)
Small code cleanup for size: -53 bytes.

Add comment justifying using a branch to do NULL/non-null return.

All string/memory tests pass and no regressions in benchtests.

geometric_mean(N=20) of all benchmarks Original / New: 1.00
Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
sysdeps/x86_64/multiarch/strchr-avx2.S

index 086cabf76a1d56509a12abd5b43e4c648d5b4f6c..1a916cc95185daac2c6a657cf2d9817809ae34ca 100644 (file)
 # define PAGE_SIZE 4096
 
        .section SECTION(.text),"ax",@progbits
-ENTRY (STRCHR)
+ENTRY_P2ALIGN (STRCHR, 5)
        /* Broadcast CHAR to YMM0.      */
        vmovd   %esi, %xmm0
        movl    %edi, %eax
        andl    $(PAGE_SIZE - 1), %eax
        VPBROADCAST     %xmm0, %ymm0
-       vpxor   %xmm9, %xmm9, %xmm9
+       vpxor   %xmm1, %xmm1, %xmm1
 
        /* Check if we cross page boundary with one vector load.  */
        cmpl    $(PAGE_SIZE - VEC_SIZE), %eax
@@ -62,37 +62,29 @@ ENTRY (STRCHR)
 
        /* Check the first VEC_SIZE bytes.      Search for both CHAR and the
           null byte.  */
-       vmovdqu (%rdi), %ymm8
-       VPCMPEQ %ymm8, %ymm0, %ymm1
-       VPCMPEQ %ymm8, %ymm9, %ymm2
-       vpor    %ymm1, %ymm2, %ymm1
-       vpmovmskb %ymm1, %eax
+       vmovdqu (%rdi), %ymm2
+       VPCMPEQ %ymm2, %ymm0, %ymm3
+       VPCMPEQ %ymm2, %ymm1, %ymm2
+       vpor    %ymm3, %ymm2, %ymm3
+       vpmovmskb %ymm3, %eax
        testl   %eax, %eax
        jz      L(aligned_more)
        tzcntl  %eax, %eax
 # ifndef USE_AS_STRCHRNUL
-       /* Found CHAR or the null byte.  */
-       cmp     (%rdi, %rax), %CHAR_REG
-       jne     L(zero)
-# endif
-       addq    %rdi, %rax
-       VZEROUPPER_RETURN
-
-       /* .p2align 5 helps keep performance more consistent if ENTRY()
-          alignment % 32 was either 16 or 0. As well this makes the
-          alignment % 32 of the loop_4x_vec fixed which makes tuning it
-          easier.  */
-       .p2align 5
-L(first_vec_x4):
-       tzcntl  %eax, %eax
-       addq    $(VEC_SIZE * 3 + 1), %rdi
-# ifndef USE_AS_STRCHRNUL
-       /* Found CHAR or the null byte.  */
+       /* Found CHAR or the null byte.  */
        cmp     (%rdi, %rax), %CHAR_REG
+       /* NB: Use a branch instead of cmovcc here. The expectation is
+          that with strchr the user will branch based on input being
+          null. Since this branch will be 100% predictive of the user
+          branch a branch miss here should save what otherwise would
+          be branch miss in the user code. Otherwise using a branch 1)
+          saves code size and 2) is faster in highly predictable
+          environments.  */
        jne     L(zero)
 # endif
        addq    %rdi, %rax
-       VZEROUPPER_RETURN
+L(return_vzeroupper):
+       ZERO_UPPER_VEC_REGISTERS_RETURN
 
 # ifndef USE_AS_STRCHRNUL
 L(zero):
@@ -103,7 +95,8 @@ L(zero):
 
        .p2align 4
 L(first_vec_x1):
-       tzcntl  %eax, %eax
+       /* Use bsf to save code size.  */
+       bsfl    %eax, %eax
        incq    %rdi
 # ifndef USE_AS_STRCHRNUL
        /* Found CHAR or the null byte.  */
@@ -113,9 +106,10 @@ L(first_vec_x1):
        addq    %rdi, %rax
        VZEROUPPER_RETURN
 
-       .p2align 4
+       .p2align 4,, 10
 L(first_vec_x2):
-       tzcntl  %eax, %eax
+       /* Use bsf to save code size.  */
+       bsfl    %eax, %eax
        addq    $(VEC_SIZE + 1), %rdi
 # ifndef USE_AS_STRCHRNUL
        /* Found CHAR or the null byte.  */
@@ -125,9 +119,10 @@ L(first_vec_x2):
        addq    %rdi, %rax
        VZEROUPPER_RETURN
 
-       .p2align 4
+       .p2align 4,, 8
 L(first_vec_x3):
-       tzcntl  %eax, %eax
+       /* Use bsf to save code size.  */
+       bsfl    %eax, %eax
        addq    $(VEC_SIZE * 2 + 1), %rdi
 # ifndef USE_AS_STRCHRNUL
        /* Found CHAR or the null byte.  */
@@ -137,6 +132,21 @@ L(first_vec_x3):
        addq    %rdi, %rax
        VZEROUPPER_RETURN
 
+       .p2align 4,, 10
+L(first_vec_x4):
+       /* Use bsf to save code size.  */
+       bsfl    %eax, %eax
+       addq    $(VEC_SIZE * 3 + 1), %rdi
+# ifndef USE_AS_STRCHRNUL
+       /* Found CHAR or the null byte.  */
+       cmp     (%rdi, %rax), %CHAR_REG
+       jne     L(zero)
+# endif
+       addq    %rdi, %rax
+       VZEROUPPER_RETURN
+
+
+
        .p2align 4
 L(aligned_more):
        /* Align data to VEC_SIZE - 1. This is the same number of
@@ -146,90 +156,92 @@ L(aligned_more):
 L(cross_page_continue):
        /* Check the next 4 * VEC_SIZE.  Only one VEC_SIZE at a time
           since data is only aligned to VEC_SIZE.  */
-       vmovdqa 1(%rdi), %ymm8
-       VPCMPEQ %ymm8, %ymm0, %ymm1
-       VPCMPEQ %ymm8, %ymm9, %ymm2
-       vpor    %ymm1, %ymm2, %ymm1
-       vpmovmskb %ymm1, %eax
+       vmovdqa 1(%rdi), %ymm2
+       VPCMPEQ %ymm2, %ymm0, %ymm3
+       VPCMPEQ %ymm2, %ymm1, %ymm2
+       vpor    %ymm3, %ymm2, %ymm3
+       vpmovmskb %ymm3, %eax
        testl   %eax, %eax
        jnz     L(first_vec_x1)
 
-       vmovdqa (VEC_SIZE + 1)(%rdi), %ymm8
-       VPCMPEQ %ymm8, %ymm0, %ymm1
-       VPCMPEQ %ymm8, %ymm9, %ymm2
-       vpor    %ymm1, %ymm2, %ymm1
-       vpmovmskb %ymm1, %eax
+       vmovdqa (VEC_SIZE + 1)(%rdi), %ymm2
+       VPCMPEQ %ymm2, %ymm0, %ymm3
+       VPCMPEQ %ymm2, %ymm1, %ymm2
+       vpor    %ymm3, %ymm2, %ymm3
+       vpmovmskb %ymm3, %eax
        testl   %eax, %eax
        jnz     L(first_vec_x2)
 
-       vmovdqa (VEC_SIZE * 2 + 1)(%rdi), %ymm8
-       VPCMPEQ %ymm8, %ymm0, %ymm1
-       VPCMPEQ %ymm8, %ymm9, %ymm2
-       vpor    %ymm1, %ymm2, %ymm1
-       vpmovmskb %ymm1, %eax
+       vmovdqa (VEC_SIZE * 2 + 1)(%rdi), %ymm2
+       VPCMPEQ %ymm2, %ymm0, %ymm3
+       VPCMPEQ %ymm2, %ymm1, %ymm2
+       vpor    %ymm3, %ymm2, %ymm3
+       vpmovmskb %ymm3, %eax
        testl   %eax, %eax
        jnz     L(first_vec_x3)
 
-       vmovdqa (VEC_SIZE * 3 + 1)(%rdi), %ymm8
-       VPCMPEQ %ymm8, %ymm0, %ymm1
-       VPCMPEQ %ymm8, %ymm9, %ymm2
-       vpor    %ymm1, %ymm2, %ymm1
-       vpmovmskb %ymm1, %eax
+       vmovdqa (VEC_SIZE * 3 + 1)(%rdi), %ymm2
+       VPCMPEQ %ymm2, %ymm0, %ymm3
+       VPCMPEQ %ymm2, %ymm1, %ymm2
+       vpor    %ymm3, %ymm2, %ymm3
+       vpmovmskb %ymm3, %eax
        testl   %eax, %eax
        jnz     L(first_vec_x4)
-       /* Align data to VEC_SIZE * 4 - 1.      */
-       addq    $(VEC_SIZE * 4 + 1), %rdi
-       andq    $-(VEC_SIZE * 4), %rdi
+       /* Align data to VEC_SIZE * 4 - 1.  */
+       incq    %rdi
+       orq     $(VEC_SIZE * 4 - 1), %rdi
        .p2align 4
 L(loop_4x_vec):
        /* Compare 4 * VEC at a time forward.  */
-       vmovdqa (%rdi), %ymm5
-       vmovdqa (VEC_SIZE)(%rdi), %ymm6
-       vmovdqa (VEC_SIZE * 2)(%rdi), %ymm7
-       vmovdqa (VEC_SIZE * 3)(%rdi), %ymm8
+       vmovdqa 1(%rdi), %ymm6
+       vmovdqa (VEC_SIZE + 1)(%rdi), %ymm7
 
        /* Leaves only CHARS matching esi as 0.  */
-       vpxor   %ymm5, %ymm0, %ymm1
        vpxor   %ymm6, %ymm0, %ymm2
        vpxor   %ymm7, %ymm0, %ymm3
-       vpxor   %ymm8, %ymm0, %ymm4
 
-       VPMINU  %ymm1, %ymm5, %ymm1
        VPMINU  %ymm2, %ymm6, %ymm2
        VPMINU  %ymm3, %ymm7, %ymm3
-       VPMINU  %ymm4, %ymm8, %ymm4
 
-       VPMINU  %ymm1, %ymm2, %ymm5
-       VPMINU  %ymm3, %ymm4, %ymm6
+       vmovdqa (VEC_SIZE * 2 + 1)(%rdi), %ymm6
+       vmovdqa (VEC_SIZE * 3 + 1)(%rdi), %ymm7
+
+       vpxor   %ymm6, %ymm0, %ymm4
+       vpxor   %ymm7, %ymm0, %ymm5
+
+       VPMINU  %ymm4, %ymm6, %ymm4
+       VPMINU  %ymm5, %ymm7, %ymm5
 
-       VPMINU  %ymm5, %ymm6, %ymm6
+       VPMINU  %ymm2, %ymm3, %ymm6
+       VPMINU  %ymm4, %ymm5, %ymm7
 
-       VPCMPEQ %ymm6, %ymm9, %ymm6
-       vpmovmskb %ymm6, %ecx
+       VPMINU  %ymm6, %ymm7, %ymm7
+
+       VPCMPEQ %ymm7, %ymm1, %ymm7
+       vpmovmskb %ymm7, %ecx
        subq    $-(VEC_SIZE * 4), %rdi
        testl   %ecx, %ecx
        jz      L(loop_4x_vec)
 
-
-       VPCMPEQ %ymm1, %ymm9, %ymm1
-       vpmovmskb %ymm1, %eax
+       VPCMPEQ %ymm2, %ymm1, %ymm2
+       vpmovmskb %ymm2, %eax
        testl   %eax, %eax
        jnz     L(last_vec_x0)
 
 
-       VPCMPEQ %ymm5, %ymm9, %ymm2
-       vpmovmskb %ymm2, %eax
+       VPCMPEQ %ymm3, %ymm1, %ymm3
+       vpmovmskb %ymm3, %eax
        testl   %eax, %eax
        jnz     L(last_vec_x1)
 
-       VPCMPEQ %ymm3, %ymm9, %ymm3
-       vpmovmskb %ymm3, %eax
+       VPCMPEQ %ymm4, %ymm1, %ymm4
+       vpmovmskb %ymm4, %eax
        /* rcx has combined result from all 4 VEC. It will only be used
           if the first 3 other VEC all did not contain a match.  */
        salq    $32, %rcx
        orq     %rcx, %rax
        tzcntq  %rax, %rax
-       subq    $(VEC_SIZE * 2), %rdi
+       subq    $(VEC_SIZE * 2 - 1), %rdi
 # ifndef USE_AS_STRCHRNUL
        /* Found CHAR or the null byte.  */
        cmp     (%rdi, %rax), %CHAR_REG
@@ -239,10 +251,11 @@ L(loop_4x_vec):
        VZEROUPPER_RETURN
 
 
-       .p2align 4
+       .p2align 4,, 10
 L(last_vec_x0):
-       tzcntl  %eax, %eax
-       addq    $-(VEC_SIZE * 4), %rdi
+       /* Use bsf to save code size.  */
+       bsfl    %eax, %eax
+       addq    $-(VEC_SIZE * 4 - 1), %rdi
 # ifndef USE_AS_STRCHRNUL
        /* Found CHAR or the null byte.  */
        cmp     (%rdi, %rax), %CHAR_REG
@@ -251,16 +264,11 @@ L(last_vec_x0):
        addq    %rdi, %rax
        VZEROUPPER_RETURN
 
-# ifndef USE_AS_STRCHRNUL
-L(zero_end):
-       xorl    %eax, %eax
-       VZEROUPPER_RETURN
-# endif
 
-       .p2align 4
+       .p2align 4,, 10
 L(last_vec_x1):
        tzcntl  %eax, %eax
-       subq    $(VEC_SIZE * 3), %rdi
+       subq    $(VEC_SIZE * 3 - 1), %rdi
 # ifndef USE_AS_STRCHRNUL
        /* Found CHAR or the null byte.  */
        cmp     (%rdi, %rax), %CHAR_REG
@@ -269,18 +277,23 @@ L(last_vec_x1):
        addq    %rdi, %rax
        VZEROUPPER_RETURN
 
+# ifndef USE_AS_STRCHRNUL
+L(zero_end):
+       xorl    %eax, %eax
+       VZEROUPPER_RETURN
+# endif
 
        /* Cold case for crossing page with first load.  */
-       .p2align 4
+       .p2align 4,, 8
 L(cross_page_boundary):
        movq    %rdi, %rdx
        /* Align rdi to VEC_SIZE - 1.  */
        orq     $(VEC_SIZE - 1), %rdi
-       vmovdqa -(VEC_SIZE - 1)(%rdi), %ymm8
-       VPCMPEQ %ymm8, %ymm0, %ymm1
-       VPCMPEQ %ymm8, %ymm9, %ymm2
-       vpor    %ymm1, %ymm2, %ymm1
-       vpmovmskb %ymm1, %eax
+       vmovdqa -(VEC_SIZE - 1)(%rdi), %ymm2
+       VPCMPEQ %ymm2, %ymm0, %ymm3
+       VPCMPEQ %ymm2, %ymm1, %ymm2
+       vpor    %ymm3, %ymm2, %ymm3
+       vpmovmskb %ymm3, %eax
        /* Remove the leading bytes. sarxl only uses bits [5:0] of COUNT
           so no need to manually mod edx.  */
        sarxl   %edx, %eax, %eax
@@ -291,13 +304,10 @@ L(cross_page_boundary):
        xorl    %ecx, %ecx
        /* Found CHAR or the null byte.  */
        cmp     (%rdx, %rax), %CHAR_REG
-       leaq    (%rdx, %rax), %rax
-       cmovne  %rcx, %rax
-# else
-       addq    %rdx, %rax
+       jne     L(zero_end)
 # endif
-L(return_vzeroupper):
-       ZERO_UPPER_VEC_REGISTERS_RETURN
+       addq    %rdx, %rax
+       VZEROUPPER_RETURN
 
 END (STRCHR)
-# endif
+#endif