Re: [PATCH 2/2] AArch64: Add SVE2 strchrnul

On 03/08/26 12:49, Wilco Dijkstra wrote:
> 
> Add an SVE2 strchrnul implementation and ifunc support based on strchr-sve.S.
> Use MATCH to check for both the input char and NUL.  Performance of
> bench-strchrnul improves ~34% on Neoverse V2.

Patch looks good, thanks.  Minor comments below, and as for strchr I think it
would good to update SHARED-FILES for this one.

> 
> ---
> 
> diff --git a/sysdeps/aarch64/multiarch/Makefile b/sysdeps/aarch64/multiarch/Makefile
> index 9616231c042c03088384b07ea8c5465d413048d1..8c279a81f7bd53f07bcab49ba17680355f57b5ab 100644
> --- a/sysdeps/aarch64/multiarch/Makefile
> +++ b/sysdeps/aarch64/multiarch/Makefile
> @@ -18,6 +18,8 @@ sysdep_routines += \
>    memset_zva64 \
>    strchr_generic \
>    strchr_sve2 \
> +  strchrnul_generic \
> +  strchrnul_sve2 \
>    strlen_asimd \
>    strlen_generic \
>    # sysdep_routines
> diff --git a/sysdeps/aarch64/multiarch/dl-symbol-redir-ifunc.h b/sysdeps/aarch64/multiarch/dl-symbol-redir-ifunc.h
> index 50b2581455c94dc2bd38f8e5ba9a357d1b79e89f..74fdca91baa562316f8f0a4d2450e9dd81aabb9c 100644
> --- a/sysdeps/aarch64/multiarch/dl-symbol-redir-ifunc.h
> +++ b/sysdeps/aarch64/multiarch/dl-symbol-redir-ifunc.h
> @@ -23,6 +23,8 @@ asm ("memset = __memset_generic");
>  asm ("strlen = __strlen_generic");
>  #ifndef SHARED
>  asm ("strchr = __strchr_generic");
> +asm ("strchrnul = __strchrnul_generic");
> +asm ("__strchrnul = __strchrnul_generic");
>  asm ("memcpy = __memcpy_generic");
>  asm ("memmove = __memmove_generic");
>  asm ("memcmp = __memcmp_generic");
> diff --git a/sysdeps/aarch64/multiarch/ifunc-impl-list.c b/sysdeps/aarch64/multiarch/ifunc-impl-list.c
> index a0c93147c532d935d562a66ac67ebaa5326dd3bd..5f5350850dd5d8270c2675fdbdc3be0f110f7116 100644
> --- a/sysdeps/aarch64/multiarch/ifunc-impl-list.c
> +++ b/sysdeps/aarch64/multiarch/ifunc-impl-list.c
> @@ -63,5 +63,8 @@ __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
>    IFUNC_IMPL (i, name, strchr,
>  	      IFUNC_IMPL_ADD (array, i, strchr, sve2 && !mte, __strchr_sve2)
>  	      IFUNC_IMPL_ADD (array, i, strchr, 1, __strchr_generic))
> +  IFUNC_IMPL (i, name, strchrnul,
> +	      IFUNC_IMPL_ADD (array, i, strchrnul, sve2 && !mte, __strchrnul_sve2)
> +	      IFUNC_IMPL_ADD (array, i, strchrnul, 1, __strchrnul_generic))
>    return 0;
>  }
> diff --git a/sysdeps/aarch64/multiarch/strchrnul.c b/sysdeps/aarch64/multiarch/strchrnul.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..d09508880655bada4ca9185a5cedf28714d10a31
> --- /dev/null
> +++ b/sysdeps/aarch64/multiarch/strchrnul.c
> @@ -0,0 +1,41 @@
> +/* Multiple versions of strchrnul. AArch64 version.
> +   Copyright (C) 2026 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +/* Define multiple versions only for the definition in libc.  */
> +
> +#if IS_IN (libc)
> +/* Redefine strchrnul so that the compiler won't complain about the type
> +   mismatch with the IFUNC selector in weak_alias, below.  */
> +# undef strchrnul
> +# undef __strchrnul
> +# define strchrnul __redirect_strchrnul
> +# define __strchrnul __redirect___strchrnul
> +# include <string.h>
> +# include <init-arch.h>
> +# undef strchrnul
> +# undef __strchrnul
> +
> +extern __typeof (__redirect_strchrnul) __strchrnul attribute_hidden;
> +
> +extern __typeof (__redirect_strchrnul) __strchrnul_generic attribute_hidden;
> +extern __typeof (__redirect_strchrnul) __strchrnul_sve2 attribute_hidden;
> +
> +libc_ifunc (__strchrnul, (sve2 && !mte ? __strchrnul_sve2 : __strchrnul_generic));
> +
> +weak_alias (__strchrnul, strchrnul);
> +#endif
> diff --git a/sysdeps/aarch64/multiarch/strchrnul_generic.S b/sysdeps/aarch64/multiarch/strchrnul_generic.S
> new file mode 100644
> index 0000000000000000000000000000000000000000..e5a6380e775eee3b97db575c1e1ea99860922f11
> --- /dev/null
> +++ b/sysdeps/aarch64/multiarch/strchrnul_generic.S
> @@ -0,0 +1,38 @@
> +/* A generic optimized strchrnul implementation for AArch64.
> +   Copyright (C) 2026 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <sysdep.h>
> +
> +#if IS_IN (libc)
> +
> +# define STRCHRNUL __strchrnul_generic
> +
> +/* Do not hide the generic version of strchr, we use it internally.  */
> +# undef hidden_def
> +# define hidden_def(name)
> +
> +# undef weak_alias
> +# define weak_alias(a, b)
> +
> +# ifdef SHARED
> +/* It doesn't make sense to send libc-internal strchrnul calls through a PLT.  */
> +	.globl __GI___strchrnul; __GI___strchrnul = __strchrnul_generic
> +# endif
> +#endif
> +
> +#include "../strchrnul.S"
> diff --git a/sysdeps/aarch64/multiarch/strchrnul_sve2.S b/sysdeps/aarch64/multiarch/strchrnul_sve2.S
> new file mode 100644
> index 0000000000000000000000000000000000000000..3a037446d851e060790369e7ea78be8573fed56b
> --- /dev/null
> +++ b/sysdeps/aarch64/multiarch/strchrnul_sve2.S
> @@ -0,0 +1,22 @@
> +/* Optimized strchrnul for SVE2.
> +   Copyright (C) 2026 Free Software Foundation, Inc.
> +
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library.  If not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#define BUILD_STRCHRNUL
> +
> +#include "strchr_sve2.S"
> diff --git a/sysdeps/aarch64/strchrnul.S b/sysdeps/aarch64/strchrnul.S
> index c80807ea73d85493982e60d023c4c137b6c9b311..2bc535fc4f77bed5d558719fad2837a9d9791eef 100644
> --- a/sysdeps/aarch64/strchrnul.S
> +++ b/sysdeps/aarch64/strchrnul.S
> @@ -26,6 +26,10 @@
>   * MTE compatible.
>   */
>  
> +#ifndef STRCHRNUL
> +# define STRCHRNUL __strchrnul
> +#endif
> +
>  #define srcin		x0
>  #define chrin		w1
>  #define result		x0
> @@ -50,7 +54,7 @@
>     which things occur in the original string, counting leading zeros identifies
>     exactly which byte matched.  */
>  
> -ENTRY (__strchrnul)
> +ENTRY (STRCHRNUL)
>  	bic	src, srcin, 15
>  	dup	vrepchr.16b, chrin
>  	ld1	{vdata.16b}, [src]
> @@ -93,6 +97,6 @@ L(end):
>  	add	result, src, tmp1, lsr 2
>  	ret
>  
> -END(__strchrnul)
> -libc_hidden_def (__strchrnul)
> -weak_alias (__strchrnul, strchrnul)
> +END(STRCHRNUL)
> +hidden_def (__strchrnul)
> +weak_alias (STRCHRNUL, strchrnul)
> diff --git a/sysdeps/aarch64/strspn.S b/sysdeps/aarch64/strspn.S
> index 42f3b82a944503aa726bc8bdb61105721d0b959a..f70047bec04fe601a942895140e7cd98cdb6a522 100644
> --- a/sysdeps/aarch64/strspn.S
> +++ b/sysdeps/aarch64/strspn.S
> @@ -51,7 +51,7 @@
>  #define table_b	v5
>  #define sevens	v7.16b
>  
> -ENTRY(STRSPN)
> +ENTRY (STRSPN)

This extra whitespace seems superflous.

>  	ldrb	w2, [set]
>  	cbz	w2, L(early)
>  #ifdef USE_AS_STRCSPN
> @@ -128,7 +128,7 @@ L(early):
>  	mov	w1, w2
>  	mov	fp, sp
>  	mov	x19, x0
> -	bl	__strchrnul
> +	bl	HIDDEN_JUMPTARGET (__strchrnul)
>  	sub	x0, x0, x19
>  	ldr	x19, [sp, 16]
>  	ldp	fp, lr, [sp], 32
> @@ -140,7 +140,7 @@ L(early):
>  	mov	w0, 0
>  #endif
>  	ret
> -END(STRSPN)
> +END (STRSPN)

Same as before.

>  
>  #undef set
> -libc_hidden_def(STRSPN)
> +libc_hidden_def (STRSPN)
>