On Sun, Aug 06, 2023 at 02:10:06PM +0000, Robert Clausecker wrote:
> The branch main has been updated by fuz:
>
> URL:
> https://cgit.FreeBSD.org/src/commit/?id=61f4c4d3dd38c5b79e04fff1dedb02071ebb33f2
>
> commit 61f4c4d3dd38c5b79e04fff1dedb02071ebb33f2
> Author: Robert Clausecker <[email protected]>
> AuthorDate: 2023-06-30 14:45:11 +0000
> Commit: Robert Clausecker <[email protected]>
> CommitDate: 2023-08-06 13:58:27 +0000
>
> lib/libc/amd64/string: add strchrnul implementations (scalar, baseline)
>
> A lot better than the generic (pre) implementaion. We do not beat glibc
> for long strings, likely due to glibc switching to AVX once the input is
> sufficiently long. X86-64-v3 and v4 implementations may be added at a
> future time.
>
> os: FreeBSD
> arch: amd64
> cpu: 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz
> │ strchrnul_pre.out │ strchrnul_scalar.out │
> strchrnul_baseline.out │
> │ sec/op │ sec/op vs base │
> sec/op vs base │
> Short 129.68µ ± 3% 59.91µ ± 1% -53.80% (p=0.000 n=20)
> 44.37µ ± 1% -65.79% (p=0.000 n=20)
> Mid 21.15µ ± 0% 19.30µ ± 0% -8.76% (p=0.000 n=20)
> 12.30µ ± 0% -41.85% (p=0.000 n=20)
> Long 13.772µ ± 0% 11.028µ ± 0% -19.92% (p=0.000 n=20)
> 3.285µ ± 0% -76.15% (p=0.000 n=20)
> geomean 33.55µ 23.36µ -30.37%
> 12.15µ -63.80%
>
> │ strchrnul_pre.out │ strchrnul_scalar.out │
> strchrnul_baseline.out │
> │ B/s │ B/s vs base │
> B/s vs base │
> Short 919.3Mi ± 3% 1989.7Mi ± 1% +116.45% (p=0.000 n=20)
> 2686.8Mi ± 1% +192.28% (p=0.000 n=20)
> Mid 5.505Gi ± 0% 6.033Gi ± 0% +9.60% (p=0.000 n=20)
> 9.466Gi ± 0% +71.97% (p=0.000 n=20)
> Long 8.453Gi ± 0% 10.557Gi ± 0% +24.88% (p=0.000 n=20)
> 35.441Gi ± 0% +319.26% (p=0.000 n=20)
> geomean 3.470Gi 4.983Gi +43.62%
> 9.584Gi +176.22%
>
> For comparison, glibc on the same machine:
>
> │ strchrnul_glibc.out │
> │ sec/op │
> Short 49.73µ ± 0%
> Mid 14.60µ ± 0%
> Long 1.237µ ± 0%
> geomean 9.646µ
>
> │ strchrnul_glibc.out │
> │ B/s │
> Short 2.341Gi ± 0%
> Mid 7.976Gi ± 0%
> Long 94.14Gi ± 0%
> geomean 12.07Gi
>
> Sponsored by: The FreeBSD Foundation
> Approved by: mjg
> Differential Revision: https://reviews.freebsd.org/D41333
> ---
> lib/libc/amd64/string/Makefile.inc | 1 +
> lib/libc/amd64/string/strchrnul.S | 167
> +++++++++++++++++++++++++++++++++++++
> 2 files changed, 168 insertions(+)
>
> diff --git a/lib/libc/amd64/string/Makefile.inc
> b/lib/libc/amd64/string/Makefile.inc
> index 1bfefa7be98c..b5260cfd78d1 100644
> --- a/lib/libc/amd64/string/Makefile.inc
> +++ b/lib/libc/amd64/string/Makefile.inc
> @@ -8,6 +8,7 @@ MDSRCS+= \
> memmove.S \
> memset.S \
> strcat.S \
> + strchrnul.S \
> strcmp.S \
> strlen.S \
> stpcpy.S
> diff --git a/lib/libc/amd64/string/strchrnul.S
> b/lib/libc/amd64/string/strchrnul.S
> new file mode 100644
> index 000000000000..6d22f31850f8
> --- /dev/null
> +++ b/lib/libc/amd64/string/strchrnul.S
> @@ -0,0 +1,167 @@
> +/*-
> + * Copyright (c) 2023 The FreeBSD Foundation
> + *
> + * This software was developed by Robert Clausecker <[email protected]>
> + * under sponsorship from the FreeBSD Foundation.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ''AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE
> + */
> +
> +#include <machine/asm.h>
> +
> +#include "amd64_archlevel.h"
> +
> +#define ALIGN_TEXT .p2align 4,0x90 # 16-byte alignment, nop-filled
> +
> + .weak strchrnul
> + .set strchrnul, __strchrnul
> +
> +ARCHFUNCS(__strchrnul)
> + ARCHFUNC(__strchrnul, scalar)
> + ARCHFUNC(__strchrnul, baseline)
> +ENDARCHFUNCS(__strchrnul)
...
> +1: sub $8, %rdi # undo advance past buffer
> +2: tzcnt %rax, %rax # first NUL or c byte match
tzcnt is the _V3 feature, while __strchrnul_scalar seems to be used for _V2.
Am I missing something?