Re: [PATCH] powerpc64: Remove old strncmp optimization

[Paul E Murphy <murphyp@linux.ibm.com>]

On 11/29/22 10:02 AM, Rajalakshmi Srinivasaraghavan via Libc-alpha wrote:
> This patch removes the power4 strncmp optimization for powerpc64 and uses
> __strncmp_ppc implementation instead.  Currently, both power4 and ppc
> IFUNC variants result in similar performance.
What cpu was used to test performance? I'd be curious if this holds for 
something like the venerable ppc970.

> 
> Tested on ppc64le with and without --disable-multi-arch flag.
> ---
>   sysdeps/powerpc/powerpc64/multiarch/Makefile  |   3 +-
>   .../powerpc64/multiarch/ifunc-impl-list.c     |   2 -
>   .../powerpc64/multiarch/strncmp-power4.S      |  23 --
>   sysdeps/powerpc/powerpc64/multiarch/strncmp.c |   3 -
>   sysdeps/powerpc/powerpc64/power4/strncmp.S    | 225 ------------------
>   5 files changed, 1 insertion(+), 255 deletions(-)
>   delete mode 100644 sysdeps/powerpc/powerpc64/multiarch/strncmp-power4.S
>   delete mode 100644 sysdeps/powerpc/powerpc64/power4/strncmp.S
> 
> diff --git a/sysdeps/powerpc/powerpc64/multiarch/Makefile b/sysdeps/powerpc/powerpc64/multiarch/Makefile
> index 6f2436b660..ed25e234ba 100644
> --- a/sysdeps/powerpc/powerpc64/multiarch/Makefile
> +++ b/sysdeps/powerpc/powerpc64/multiarch/Makefile
> @@ -12,8 +12,7 @@ sysdep_routines += memcpy-power8-cached memcpy-power7 memcpy-a2 memcpy-power6 \
>   		   strnlen-power8 strnlen-power7 strnlen-ppc64 \
>   		   strcasecmp-power7 strcasecmp_l-power7 \
>   		   strncase-power7 strncase_l-power7 \
> -		   strncmp-power8 strncmp-power7 \
> -		   strncmp-power4 strncmp-ppc64 \
> +		   strncmp-power8 strncmp-power7 strncmp-ppc64 \
>   		   strchr-power8 strchr-power7 strchr-ppc64 \
>   		   strchrnul-power8 strchrnul-power7 strchrnul-ppc64 \
>   		   strcpy-power8 strcpy-power7 strcpy-ppc64 stpcpy-power8 \
> diff --git a/sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c b/sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c
> index 5a3c7a5886..238f784146 100644
> --- a/sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c
> +++ b/sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c
> @@ -171,8 +171,6 @@ __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
>   			      __strncmp_power8)
>   	      IFUNC_IMPL_ADD (array, i, strncmp, hwcap & PPC_FEATURE_ARCH_2_06,
>   			      __strncmp_power7)
> -	      IFUNC_IMPL_ADD (array, i, strncmp, hwcap & PPC_FEATURE_POWER4,
> -			      __strncmp_power4)
>   	      IFUNC_IMPL_ADD (array, i, strncmp, 1,
>   			      __strncmp_ppc))
> 
> diff --git a/sysdeps/powerpc/powerpc64/multiarch/strncmp-power4.S b/sysdeps/powerpc/powerpc64/multiarch/strncmp-power4.S
> deleted file mode 100644
> index df6ad6ae9c..0000000000
> --- a/sysdeps/powerpc/powerpc64/multiarch/strncmp-power4.S
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -/* Copyright (C) 2013-2022 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 STRNCMP __strncmp_power4
> -
> -#undef libc_hidden_builtin_def
> -#define libc_hidden_builtin_def(name)
> -
> -#include <sysdeps/powerpc/powerpc64/power4/strncmp.S>
> diff --git a/sysdeps/powerpc/powerpc64/multiarch/strncmp.c b/sysdeps/powerpc/powerpc64/multiarch/strncmp.c
> index da3de73c27..7400ed3b9a 100644
> --- a/sysdeps/powerpc/powerpc64/multiarch/strncmp.c
> +++ b/sysdeps/powerpc/powerpc64/multiarch/strncmp.c
> @@ -26,7 +26,6 @@
>   # include "init-arch.h"
> 
>   extern __typeof (strncmp) __strncmp_ppc attribute_hidden;
> -extern __typeof (strncmp) __strncmp_power4 attribute_hidden;
>   extern __typeof (strncmp) __strncmp_power7 attribute_hidden;
>   extern __typeof (strncmp) __strncmp_power8 attribute_hidden;
>   # ifdef __LITTLE_ENDIAN__
> @@ -46,7 +45,5 @@ libc_ifunc_redirected (__redirect_strncmp, strncmp,
>   		       ? __strncmp_power8
>   		       : (hwcap & PPC_FEATURE_ARCH_2_06)
>   			 ? __strncmp_power7
> -			 : (hwcap & PPC_FEATURE_POWER4)
> -			   ? __strncmp_power4
>   			   : __strncmp_ppc);
Minor nit, does the spacing of __strcncmp_ppc need updating here too?