Re: git: 61f4c4d3dd38 - main - lib/libc/amd64/string: add strchrnul implementations (scalar, baseline)

In reply to: Robert Clausecker : "git: 61f4c4d3dd38 - main - lib/libc/amd64/string: add strchrnul implementations (scalar, baseline)"
Go to: [ bottom of page ] [ top of archives ] [ this month ]
From: Konstantin Belousov <kostikbel_at_gmail.com>
Date: Sun, 06 Aug 2023 23:14:57 UTC
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 <fuz@FreeBSD.org>
> AuthorDate: 2023-06-30 14:45:11 +0000
> Commit:     Robert Clausecker <fuz@FreeBSD.org>
> 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 <fuz@FreeBSD.org>
> + * 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?