[prev in list] [next in list] [prev in thread] [next in thread] 

List:       glibc-alpha
Subject:    Re: [PATCH] powerpc64: Remove old strncmp optimization
From:       Rajalakshmi Srinivasaraghavan via Libc-alpha <libc-alpha () sourceware ! org>
Date:       2022-11-30 16:50:53
Message-ID: 3c74512e-c26b-9167-da31-a3ad5101b83d () linux ! vnet ! ibm ! com
[Download RAW message or body]


On 11/29/22 4:37 PM, Paul E Murphy wrote:
>
>
> 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.

Compared __strncmp_power4 and __strncmp_ppc benchtest numbers on POWER8 
and POWER10 systems.

>
>>
>> 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?

Ack.



[prev in list] [next in list] [prev in thread] [next in thread]