public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] powerpc: Remove power8 strcasestr optimization
@ 2024-03-05 20:13 Adhemerval Zanella
  2024-03-12  2:37 ` DJ Delorie
  2024-03-15 19:57 ` Peter Bergner
  0 siblings, 2 replies; 4+ messages in thread
From: Adhemerval Zanella @ 2024-03-05 20:13 UTC (permalink / raw)
  To: libc-alpha; +Cc: Peter Bergner

Similar to strstr (1e9a550ba4), power8 strcasestr does not show much
improvement compared to the generic implementation.  The geomean
on bench-strcasestr shows:

            __strcasestr_power8  __strcasestr_ppc
  power10                  1159              1120
  power9                   1640              1469
  power8                   1787              1904

The strcasestr uses the same 'trick' as power7 strstr to detect
potential quadradic behavior, which only adds overheads for input
that trigger quadradic behavior and it is really a hack.

Checked on powerpc64le-linux-gnu.
---
 sysdeps/powerpc/powerpc64/multiarch/Makefile  |   2 +-
 .../powerpc64/multiarch/ifunc-impl-list.c     |   9 -
 .../powerpc64/multiarch/strcasestr-power8.S   |  33 --
 .../powerpc64/multiarch/strcasestr-ppc64.c    |  34 --
 .../powerpc/powerpc64/multiarch/strcasestr.c  |  38 --
 sysdeps/powerpc/powerpc64/power8/Makefile     |   3 -
 .../powerpc64/power8/strcasestr-ppc64.c       |  29 -
 sysdeps/powerpc/powerpc64/power8/strcasestr.S | 528 ------------------
 8 files changed, 1 insertion(+), 675 deletions(-)
 delete mode 100644 sysdeps/powerpc/powerpc64/multiarch/strcasestr-power8.S
 delete mode 100644 sysdeps/powerpc/powerpc64/multiarch/strcasestr-ppc64.c
 delete mode 100644 sysdeps/powerpc/powerpc64/multiarch/strcasestr.c
 delete mode 100644 sysdeps/powerpc/powerpc64/power8/Makefile
 delete mode 100644 sysdeps/powerpc/powerpc64/power8/strcasestr-ppc64.c
 delete mode 100644 sysdeps/powerpc/powerpc64/power8/strcasestr.S

diff --git a/sysdeps/powerpc/powerpc64/multiarch/Makefile b/sysdeps/powerpc/powerpc64/multiarch/Makefile
index 5624249fe2..a38ff46448 100644
--- a/sysdeps/powerpc/powerpc64/multiarch/Makefile
+++ b/sysdeps/powerpc/powerpc64/multiarch/Makefile
@@ -26,7 +26,7 @@ sysdep_routines += memcpy-power8-cached memcpy-power7 memcpy-a2 memcpy-power6 \
 		   memmove-power7 memmove-ppc64 wordcopy-ppc64 \
 		   strncpy-power8 \
 		   strspn-power8 strspn-ppc64 strcspn-power8 strcspn-ppc64 \
-		   strlen-power8 strcasestr-power8 strcasestr-ppc64 \
+		   strlen-power8 \
 		   strcasecmp-ppc64 strcasecmp-power8 strncase-ppc64 \
 		   strncase-power8
 
diff --git a/sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c b/sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c
index e3e6f5fd10..30fd89e109 100644
--- a/sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c
+++ b/sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c
@@ -432,14 +432,5 @@ __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
              IFUNC_IMPL_ADD (array, i, strcspn, 1,
                              __strcspn_ppc))
 
-  /* Support sysdeps/powerpc/powerpc64/multiarch/strcasestr.c.  */
-  IFUNC_IMPL (i, name, strcasestr,
-	      IFUNC_IMPL_ADD (array, i, strcasestr,
-			      hwcap2 & PPC_FEATURE2_ARCH_2_07
-			      && hwcap & PPC_FEATURE_HAS_ALTIVEC,
-			      __strcasestr_power8)
-             IFUNC_IMPL_ADD (array, i, strcasestr, 1,
-                             __strcasestr_ppc))
-
   return 0;
 }
diff --git a/sysdeps/powerpc/powerpc64/multiarch/strcasestr-power8.S b/sysdeps/powerpc/powerpc64/multiarch/strcasestr-power8.S
deleted file mode 100644
index 4670d03aaa..0000000000
--- a/sysdeps/powerpc/powerpc64/multiarch/strcasestr-power8.S
+++ /dev/null
@@ -1,33 +0,0 @@
-/* Optimized strcasestr implementation for POWER8.
-   Copyright (C) 2016-2024 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 STRCASESTR __strcasestr_power8
-
-#undef libc_hidden_builtin_def
-#define libc_hidden_builtin_def(name)
-
-/* The following definitions are used in strcasestr optimization.  */
-
-/* strlen is used to calculate len of r4.  */
-#define STRLEN __strlen_power8
-/* strnlen is used to check if len of r3 is more than r4.  */
-#define STRNLEN __strnlen_power8
-/* strchr is used to check if first char of r4 is present in r3.  */
-#define STRCHR __strchr_power8
-
-#include <sysdeps/powerpc/powerpc64/power8/strcasestr.S>
diff --git a/sysdeps/powerpc/powerpc64/multiarch/strcasestr-ppc64.c b/sysdeps/powerpc/powerpc64/multiarch/strcasestr-ppc64.c
deleted file mode 100644
index 073d8beaff..0000000000
--- a/sysdeps/powerpc/powerpc64/multiarch/strcasestr-ppc64.c
+++ /dev/null
@@ -1,34 +0,0 @@
-/* PowerPC64 default implementation of strcasestr.
-   Copyright (C) 2016-2024 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 <string.h>
-
-#define STRCASESTR  __strcasestr_ppc
-#if IS_IN (libc) && defined(SHARED)
-# undef libc_hidden_builtin_def
-# define libc_hidden_builtin_def(name) \
-  __hidden_ver1(__strcasestr_ppc, __GI_strcasestr, __strcasestr_ppc);
-#endif
-
-
-#undef weak_alias
-#define weak_alias(a,b)
-
-extern __typeof (strcasestr) __strcasestr_ppc attribute_hidden;
-
-#include <string/strcasestr.c>
diff --git a/sysdeps/powerpc/powerpc64/multiarch/strcasestr.c b/sysdeps/powerpc/powerpc64/multiarch/strcasestr.c
deleted file mode 100644
index d2bda3f487..0000000000
--- a/sysdeps/powerpc/powerpc64/multiarch/strcasestr.c
+++ /dev/null
@@ -1,38 +0,0 @@
-/* Multiple versions of strcasestr.
-   Copyright (C) 2016-2024 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/>.  */
-
-#if IS_IN (libc)
-# include <string.h>
-# include <shlib-compat.h>
-# include "init-arch.h"
-
-extern __typeof (__strcasestr) __strcasestr_ppc attribute_hidden;
-extern __typeof (__strcasestr) __strcasestr_power8 attribute_hidden;
-
-/* Avoid DWARF definition DIE on ifunc symbol so that GDB can handle
-   ifunc symbol properly.  */
-libc_ifunc (__strcasestr,
-		(hwcap2 & PPC_FEATURE2_ARCH_2_07
-		 && hwcap & PPC_FEATURE_HAS_ALTIVEC)
-		? __strcasestr_power8
-		: __strcasestr_ppc);
-
-weak_alias (__strcasestr, strcasestr)
-#else
-#include <string/strcasestr.c>
-#endif
diff --git a/sysdeps/powerpc/powerpc64/power8/Makefile b/sysdeps/powerpc/powerpc64/power8/Makefile
deleted file mode 100644
index 71a59529f3..0000000000
--- a/sysdeps/powerpc/powerpc64/power8/Makefile
+++ /dev/null
@@ -1,3 +0,0 @@
-ifeq ($(subdir),string)
-sysdep_routines += strcasestr-ppc64
-endif
diff --git a/sysdeps/powerpc/powerpc64/power8/strcasestr-ppc64.c b/sysdeps/powerpc/powerpc64/power8/strcasestr-ppc64.c
deleted file mode 100644
index 188d81a717..0000000000
--- a/sysdeps/powerpc/powerpc64/power8/strcasestr-ppc64.c
+++ /dev/null
@@ -1,29 +0,0 @@
-/* Optimized strcasestr implementation for PowerPC64/POWER8.
-   Copyright (C) 2016-2024 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 <string.h>
-
-#define STRCASESTR __strcasestr_ppc
-#undef libc_hidden_builtin_def
-#define libc_hidden_builtin_def(__name)
-
-#undef weak_alias
-#define weak_alias(a,b)
-extern __typeof (strcasestr) __strcasestr_ppc attribute_hidden;
-
-#include <string/strcasestr.c>
diff --git a/sysdeps/powerpc/powerpc64/power8/strcasestr.S b/sysdeps/powerpc/powerpc64/power8/strcasestr.S
deleted file mode 100644
index fc98574bec..0000000000
--- a/sysdeps/powerpc/powerpc64/power8/strcasestr.S
+++ /dev/null
@@ -1,528 +0,0 @@
-/* Optimized strcasestr implementation for PowerPC64/POWER8.
-   Copyright (C) 2016-2024 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>
-#include <locale-defines.h>
-
-/* Char * [r3] strcasestr (char *s [r3], char * pat[r4])  */
-
-/* The performance gain is obtained by comparing 16 bytes.  */
-
-/* When the first char of r4 is hit ITERATIONS times in r3
-   fallback to default.  */
-#define ITERATIONS	64
-
-#ifndef STRCASESTR
-# define STRCASESTR __strcasestr
-#endif
-
-#ifndef STRLEN
-/* For builds without IFUNC support, local calls should be made to internal
-   GLIBC symbol (created by libc_hidden_builtin_def).  */
-# ifdef SHARED
-#  define STRLEN   __GI_strlen
-# else
-#  define STRLEN   strlen
-# endif
-#endif
-
-#ifndef STRNLEN
-/* For builds without IFUNC support, local calls should be made to internal
-   GLIBC symbol (created by libc_hidden_builtin_def).  */
-# ifdef SHARED
-#  define STRNLEN   __GI_strnlen
-# else
-#  define STRNLEN    __strnlen
-# endif
-#endif
-
-#ifndef STRCHR
-# ifdef SHARED
-#  define STRCHR   __GI_strchr
-# else
-#  define STRCHR   strchr
-# endif
-#endif
-
-/* Convert 16 bytes of v4 and reg to lowercase and compare.  */
-#define TOLOWER(reg)     \
-	vcmpgtub	v6, v4, v1; \
-	vcmpgtub	v7, v2, v4; \
-	vand	v8, v7, v6; \
-	vand	v8, v8, v3; \
-	vor	v4, v8, v4; \
-	vcmpgtub	v6, reg, v1; \
-	vcmpgtub	v7, v2, reg; \
-	vand	v8, v7, v6; \
-	vand	v8, v8, v3; \
-	vor	reg, v8, reg; \
-	vcmpequb.	v6, reg, v4;
-
-#define	FRAMESIZE	(FRAME_MIN_SIZE+48)
-	.machine  power8
-ENTRY (STRCASESTR, 4)
-	CALL_MCOUNT 2
-	mflr	r0			/* Load link register LR to r0.  */
-	std	r31, -8(r1)		/* Save callers register r31.  */
-	std	r30, -16(r1)		/* Save callers register r30.  */
-	std	r29, -24(r1)		/* Save callers register r29.  */
-	std	r28, -32(r1)		/* Save callers register r28.  */
-	std	r27, -40(r1)		/* Save callers register r27.  */
-	std	r0, 16(r1)		/* Store the link register.  */
-	cfi_offset(r31, -8)
-	cfi_offset(r30, -16)
-	cfi_offset(r29, -24)
-	cfi_offset(r28, -32)
-	cfi_offset(r27, -40)
-	cfi_offset(lr, 16)
-	stdu	r1, -FRAMESIZE(r1)	/* Create the stack frame.  */
-	cfi_adjust_cfa_offset(FRAMESIZE)
-
-	dcbt	0, r3
-	dcbt	0, r4
-	cmpdi	cr7, r3, 0		/* Input validation.  */
-	beq	cr7, L(retnull)
-	cmpdi	cr7, r4, 0
-	beq	cr7, L(retnull)
-
-	mr	r29, r3
-	mr	r30, r4
-	/* Load first byte from r4 and check if its null.  */
-	lbz	r6, 0(r4)
-	cmpdi	cr7, r6, 0
-	beq	cr7, L(ret_r3)
-
-	ld	r10, __libc_tsd_LOCALE@got@tprel(r2)
-	add	r9, r10, __libc_tsd_LOCALE@tls
-	ld	r9, 0(r9)
-	ld	r9, LOCALE_CTYPE_TOUPPER(r9)
-	sldi	r10, r6, 2		/* Convert to upper case.  */
-	lwzx	r28, r9, r10
-
-	ld	r10, __libc_tsd_LOCALE@got@tprel(r2)
-	add	r11, r10, __libc_tsd_LOCALE@tls
-	ld	r11, 0(r11)
-	ld	r11, LOCALE_CTYPE_TOLOWER(r11)
-	sldi	r10, r6, 2              /* Convert to lower case.  */
-	lwzx	r27, r11, r10
-
-	/* Check if the first char is present.  */
-	mr	r4, r27
-	bl	STRCHR
-	nop
-	mr	r5, r3
-	mr	r3, r29
-	mr	r29, r5
-	mr	r4, r28
-	bl	STRCHR
-	nop
-	cmpdi	cr7, r29, 0
-	beq	cr7, L(firstpos)
-	cmpdi	cr7, r3, 0
-	beq	cr7, L(skipcheck)
-	cmpw	cr7, r3, r29
-	ble 	cr7, L(firstpos)
-	/* Move r3 to the first occurrence.  */
-L(skipcheck):
-	mr	r3, r29
-L(firstpos):
-	mr	r29, r3
-
-	sldi	r9, r27, 8
-	or	r28, r9, r28
-	/* Reg r27 is used to count the number of iterations.  */
-	li	r27, 0
-	/* If first char of search str is not present.  */
-	cmpdi	cr7, r3, 0
-	ble	cr7, L(end)
-
-	/* Find the length of pattern.  */
-	mr	r3, r30
-	bl	STRLEN
-	nop
-
-	cmpdi	cr7, r3, 0	/* If search str is null.  */
-	beq	cr7, L(ret_r3)
-
-	mr	r31, r3
-	mr	r4, r3
-	mr	r3, r29
-	bl	STRNLEN
-	nop
-
-	cmpd	cr7, r3, r31 	/* If len(r3) < len(r4).  */
-	blt	cr7, L(retnull)
-
-	mr	r3, r29
-
-	/* Locales not matching ASCII for single bytes.  */
-	ld	r10, __libc_tsd_LOCALE@got@tprel(r2)
-	add	r9, r10, __libc_tsd_LOCALE@tls
-	ld	r9, 0(r9)
-	ld	r7, 0(r9)
-	addi	r7, r7, LOCALE_DATA_VALUES+_NL_CTYPE_NONASCII_CASE*SIZEOF_VALUES
-	lwz	r8, 0(r7)
-	cmpdi	cr7, r8, 1
-	beq	cr7, L(bytebybyte)
-
-	/* If len(r4) < 16 handle byte by byte.  */
-	/* For shorter strings we will not use vector registers.  */
-	cmpdi	cr7, r31, 16
-	blt	cr7, L(bytebybyte)
-
-	/* Comparison values used for TOLOWER.  */
-	/* Load v1 = 64('A' - 1), v2 = 91('Z' + 1), v3 = 32 in each byte.  */
-	vspltish	v0, 0
-	vspltisb	v5, 2
-	vspltisb	v4, 4
-	vsl	v3, v5, v4
-	vaddubm	v1, v3, v3
-	vspltisb	v5, 15
-	vaddubm	v2, v5, v5
-	vaddubm	v2, v1, v2
-	vspltisb	v4, -3
-	vaddubm	v2, v2, v4
-
-	/*
-	1. Load 16 bytes from r3 and r4
-	2. Check if there is null, If yes, proceed byte by byte path.
-	3. Else,Convert both to lowercase and compare.
-	4. If they are same proceed to 1.
-	5. If they dont match, find if first char of r4 is present in the
-	   loaded 16 byte of r3.
-	6. If yes, move position, load next 16 bytes of r3 and proceed to 2.
-	*/
-
-	mr	r8, r3		/* Save r3 for future use.  */
-	mr	r4, r30		/* Restore r4.  */
-	clrldi	r10, r4, 60
-	lvx	v5, 0, r4	/* Load 16 bytes from r4.  */
-	cmpdi	cr7, r10, 0
-	beq	cr7, L(begin2)
-	/* If r4 is unaligned, load another 16 bytes.  */
-#ifdef __LITTLE_ENDIAN__
-	lvsr	v7, 0, r4
-#else
-	lvsl	v7, 0, r4
-#endif
-	addi	r5, r4, 16
-	lvx	v9, 0, r5
-#ifdef __LITTLE_ENDIAN__
-	vperm	v5, v9, v5, v7
-#else
-	vperm	v5, v5, v9, v7
-#endif
-L(begin2):
-	lvx	v4, 0, r3
-	vcmpequb.	v7, v0, v4	/* Check for null.  */
-	beq	cr6, L(nullchk6)
-	b	L(trailcheck)
-
-        .align  4
-L(nullchk6):
-	clrldi	r10, r3, 60
-	cmpdi	cr7, r10, 0
-	beq	cr7, L(next16)
-#ifdef __LITTLE_ENDIAN__
-	lvsr	v7, 0, r3
-#else
-	lvsl	v7, 0, r3
-#endif
-	addi	r5, r3, 16
-	/* If r3 is unaligned, load another 16 bytes.  */
-	lvx	v10, 0, r5
-#ifdef __LITTLE_ENDIAN__
-	vperm	v4, v10, v4, v7
-#else
-	vperm	v4, v4, v10, v7
-#endif
-L(next16):
-	vcmpequb.	v6, v0, v5	/* Check for null.  */
-	beq	cr6, L(nullchk)
-	b	L(trailcheck)
-
-	.align	4
-L(nullchk):
-	vcmpequb.	v6, v0, v4
-	beq	cr6, L(nullchk1)
-	b	L(retnull)
-
-	.align	4
-L(nullchk1):
-	/* Convert both v3 and v4 to lower.  */
-	TOLOWER(v5)
-	/* If both are same, branch to match.  */
-	blt	cr6, L(match)
-	/* Find if the first char is present in next 15 bytes.  */
-#ifdef __LITTLE_ENDIAN__
-	vspltb	v6, v5, 15
-	vsldoi	v7, v0, v4, 15
-#else
-	vspltb	v6, v5, 0
-	vspltisb	v7, 8
-	vslo	v7, v4, v7
-#endif
-	vcmpequb	v7, v6, v7
-	vcmpequb.	v6, v0, v7
-	/* Shift r3 by 16 bytes and proceed.  */
-	blt	cr6, L(shift16)
-	vclzd	v8, v7
-#ifdef __LITTLE_ENDIAN__
-	vspltb	v6, v8, 15
-#else
-	vspltb	v6, v8, 7
-#endif
-	vcmpequb.	v6, v6, v1
-	/* Shift r3 by 8  bytes and proceed.  */
-	blt	cr6, L(shift8)
-	b	L(begin)
-
-	.align	4
-L(match):
-	/* There is a match of 16 bytes, check next bytes.  */
-	cmpdi	cr7, r31, 16
-	mr	r29, r3
-	beq	cr7, L(ret_r3)
-
-L(secondmatch):
-	addi	r3, r3, 16
-	addi	r4, r4, 16
-	/* Load next 16 bytes of r3 and r4 and compare.  */
-	clrldi	r10, r4, 60
-	cmpdi	cr7, r10, 0
-	beq	cr7, L(nextload)
-	/* Handle unaligned case.  */
-	vor	v6, v9, v9
-	vcmpequb.	v7, v0, v6
-	beq	cr6, L(nullchk2)
-	b	L(trailcheck)
-
-	.align	4
-L(nullchk2):
-#ifdef __LITTLE_ENDIAN__
-	lvsr	v7, 0, r4
-#else
-	lvsl	v7, 0, r4
-#endif
-	addi	r5, r4, 16
-	/* If r4 is unaligned, load another 16 bytes.  */
-	lvx	v9, 0, r5
-#ifdef __LITTLE_ENDIAN__
-	vperm	v11, v9, v6, v7
-#else
-	vperm	v11, v6, v9, v7
-#endif
-	b	L(compare)
-
-	.align	4
-L(nextload):
-	lvx	v11, 0, r4
-L(compare):
-	vcmpequb.	v7, v0, v11
-	beq	cr6, L(nullchk3)
-	b	L(trailcheck)
-
-	.align	4
-L(nullchk3):
-	clrldi	r10, r3, 60
-	cmpdi 	cr7, r10, 0
-	beq 	cr7, L(nextload1)
-	/* Handle unaligned case.  */
-	vor	v4, v10, v10
-	vcmpequb.	v7, v0, v4
-	beq	cr6, L(nullchk4)
-	b	L(retnull)
-
-	.align	4
-L(nullchk4):
-#ifdef __LITTLE_ENDIAN__
-	lvsr	v7, 0, r3
-#else
-	lvsl	v7, 0, r3
-#endif
-	addi	r5, r3, 16
-	/* If r3 is unaligned, load another 16 bytes.  */
-	lvx	v10, 0, r5
-#ifdef __LITTLE_ENDIAN__
-	vperm	v4, v10, v4, v7
-#else
-	vperm	v4, v4, v10, v7
-#endif
-	b	L(compare1)
-
-	.align	4
-L(nextload1):
-	lvx	v4, 0, r3
-L(compare1):
-	vcmpequb.	v7, v0, v4
-	beq	cr6, L(nullchk5)
-	b	L(retnull)
-
-	.align	4
-L(nullchk5):
-	/* Convert both v3 and v4 to lower.  */
-	TOLOWER(v11)
-	/* If both are same, branch to secondmatch.  */
-	blt 	cr6, L(secondmatch)
-	/* Continue the search.  */
-        b	L(begin)
-
-	.align	4
-L(trailcheck):
-	ld	r10, __libc_tsd_LOCALE@got@tprel(r2)
-	add	r11, r10, __libc_tsd_LOCALE@tls
-	ld	r11, 0(r11)
-	ld	r11, LOCALE_CTYPE_TOLOWER(r11)
-L(loop2):
-	lbz	r5, 0(r3)               /* Load byte from r3.  */
-	lbz	r6, 0(r4)               /* Load next byte from r4.  */
-	cmpdi 	cr7, r6, 0              /* Is it null?  */
-	beq 	cr7, L(updater3)
-	cmpdi 	cr7, r5, 0              /* Is it null?  */
-	beq 	cr7, L(retnull)         /* If yes, return.  */
-	addi	r3, r3, 1
-	addi	r4, r4, 1               /* Increment r4.  */
-	sldi	r10, r5, 2              /* Convert to lower case.  */
-	lwzx	r10, r11, r10
-	sldi	r7, r6, 2               /* Convert to lower case.  */
-	lwzx	r7, r11, r7
-	cmpw	cr7, r7, r10            /* Compare with byte from r4.  */
-	bne	cr7, L(begin)
-	b	L(loop2)
-
-	.align	4
-L(shift8):
-	addi	r8, r8, 7
-	b	L(begin)
-	.align	4
-L(shift16):
-	addi	r8, r8, 15
-	.align	4
-L(begin):
-	addi	r8, r8, 1
-	mr	r3, r8
-	/* When our iterations exceed ITERATIONS,fall back to default.  */
-	addi	r27, r27, 1
-	cmpdi	cr7, r27, ITERATIONS
-	beq	cr7, L(default)
-	mr	r4, r30         /* Restore r4.  */
-	b	L(begin2)
-
-	/* Handling byte by byte.  */
-	.align	4
-L(loop1):
-	mr	r3, r8
-	addi	r27, r27, 1
-	cmpdi	cr7, r27, ITERATIONS
-	beq	cr7, L(default)
-	mr	r29, r8
-	srdi	r4, r28, 8
-	/* Check if the first char is present.  */
-	bl	STRCHR
-	nop
-	mr	r5, r3
-	mr	r3, r29
-	mr	r29, r5
-	sldi	r4, r28, 56
-	srdi	r4, r4, 56
-	bl	STRCHR
-	nop
-	cmpdi	cr7, r29, 0
-	beq	cr7, L(nextpos)
-	cmpdi	cr7, r3, 0
-	beq	cr7, L(skipcheck1)
-	cmpw	cr7, r3, r29
-	ble 	cr7, L(nextpos)
-	/* Move r3 to first occurrence.  */
-L(skipcheck1):
-	mr	r3, r29
-L(nextpos):
-	mr	r29, r3
-	cmpdi 	cr7, r3, 0
-	ble 	cr7, L(retnull)
-L(bytebybyte):
-	ld	r10, __libc_tsd_LOCALE@got@tprel(r2)
-	add	r11, r10, __libc_tsd_LOCALE@tls
-	ld	r11, 0(r11)
-	ld	r11, LOCALE_CTYPE_TOLOWER(r11)
-	mr	r4, r30                 /* Restore r4.  */
-	mr	r8, r3                  /* Save r3.  */
-	addi	r8, r8, 1
-
-L(loop):
-	addi	r3, r3, 1
-	lbz	r5, 0(r3)               /* Load byte from r3.  */
-	addi	r4, r4, 1               /* Increment r4.  */
-	lbz	r6, 0(r4)               /* Load next byte from r4.  */
-	cmpdi 	cr7, r6, 0              /* Is it null?  */
-	beq 	cr7, L(updater3)
-	cmpdi 	cr7, r5, 0              /* Is it null?  */
-	beq 	cr7, L(retnull)         /* If yes, return.  */
-	sldi	r10, r5, 2              /* Convert to lower case.  */
-	lwzx	r10, r11, r10
-	sldi	r7, r6, 2               /* Convert to lower case.  */
-	lwzx	r7, r11, r7
-	cmpw	cr7, r7, r10            /* Compare with byte from r4.  */
-	bne 	cr7, L(loop1)
-	b	L(loop)
-
-	/* Handling return values.  */
-	.align	4
-L(updater3):
-	subf	r3, r31, r3	/* Reduce r31 (len of r4) from r3.  */
-	b	L(end)
-
-	.align	4
-L(ret_r3):
-	mr	r3, r29		/* Return point of match.  */
-	b	L(end)
-
-	.align	4
-L(retnull):
-	li	r3, 0		/* Substring was not found.  */
-	b	L(end)
-
-	.align	4
-L(default):
-	mr	r4, r30
-	bl	__strcasestr_ppc
-	nop
-
-	.align	4
-L(end):
-	addi	r1, r1, FRAMESIZE	/* Restore stack pointer.  */
-	cfi_adjust_cfa_offset(-FRAMESIZE)
-	ld	r0, 16(r1)	/* Restore the saved link register.  */
-	ld	r27, -40(r1)
-	ld	r28, -32(r1)
-	ld	r29, -24(r1)	/* Restore callers save register r29.  */
-	ld	r30, -16(r1)	/* Restore callers save register r30.  */
-	ld	r31, -8(r1)	/* Restore callers save register r31.  */
-	cfi_restore(lr)
-	cfi_restore(r27)
-	cfi_restore(r28)
-	cfi_restore(r29)
-	cfi_restore(r30)
-	cfi_restore(r31)
-	mtlr	r0		/* Branch to link register.  */
-	blr
-END (STRCASESTR)
-
-weak_alias (__strcasestr, strcasestr)
-libc_hidden_def (__strcasestr)
-libc_hidden_builtin_def (strcasestr)
-- 
2.34.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] powerpc: Remove power8 strcasestr optimization
  2024-03-05 20:13 [PATCH] powerpc: Remove power8 strcasestr optimization Adhemerval Zanella
@ 2024-03-12  2:37 ` DJ Delorie
  2024-03-15 19:57 ` Peter Bergner
  1 sibling, 0 replies; 4+ messages in thread
From: DJ Delorie @ 2024-03-12  2:37 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha, bergner

Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
> diff --git a/sysdeps/powerpc/powerpc64/multiarch/Makefile b/sysdeps/powerpc/powerpc64/multiarch/Makefile
> -		   strlen-power8 strcasestr-power8 strcasestr-ppc64 \
> +		   strlen-power8 \

Removing strcasestr-power8 and strcasestr-ppc64, ok.

> diff --git a/sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c b/sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c
> index e3e6f5fd10..30fd89e109 100644
> --- a/sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c
> +++ b/sysdeps/powerpc/powerpc64/multiarch/ifunc-impl-list.c
> @@ -432,14 +432,5 @@ __libc_ifunc_impl_list (const char *name, struct libc_ifunc_impl *array,
>               IFUNC_IMPL_ADD (array, i, strcspn, 1,
>                               __strcspn_ppc))
>  
> -  /* Support sysdeps/powerpc/powerpc64/multiarch/strcasestr.c.  */
> -  IFUNC_IMPL (i, name, strcasestr,
> -	      IFUNC_IMPL_ADD (array, i, strcasestr,
> -			      hwcap2 & PPC_FEATURE2_ARCH_2_07
> -			      && hwcap & PPC_FEATURE_HAS_ALTIVEC,
> -			      __strcasestr_power8)
> -             IFUNC_IMPL_ADD (array, i, strcasestr, 1,
> -                             __strcasestr_ppc))
> -

We're removing all of the three implementations, so ok.

> diff --git a/sysdeps/powerpc/powerpc64/multiarch/strcasestr-power8.S b/sysdeps/powerpc/powerpc64/multiarch/strcasestr-power8.S
> deleted file mode 100644
> diff --git a/sysdeps/powerpc/powerpc64/multiarch/strcasestr-ppc64.c b/sysdeps/powerpc/powerpc64/multiarch/strcasestr-ppc64.c
> deleted file mode 100644
> diff --git a/sysdeps/powerpc/powerpc64/multiarch/strcasestr.c b/sysdeps/powerpc/powerpc64/multiarch/strcasestr.c
> deleted file mode 100644

Ok.

> diff --git a/sysdeps/powerpc/powerpc64/power8/Makefile b/sysdeps/powerpc/powerpc64/power8/Makefile
> deleted file mode 100644
> diff --git a/sysdeps/powerpc/powerpc64/power8/strcasestr-ppc64.c b/sysdeps/powerpc/powerpc64/power8/strcasestr-ppc64.c
> deleted file mode 100644
> diff --git a/sysdeps/powerpc/powerpc64/power8/strcasestr.S b/sysdeps/powerpc/powerpc64/power8/strcasestr.S
> deleted file mode 100644

Ok.

LGTM

Reviewed-by: DJ Delorie <dj@redhat.com>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] powerpc: Remove power8 strcasestr optimization
  2024-03-05 20:13 [PATCH] powerpc: Remove power8 strcasestr optimization Adhemerval Zanella
  2024-03-12  2:37 ` DJ Delorie
@ 2024-03-15 19:57 ` Peter Bergner
  2024-03-18 12:52   ` Adhemerval Zanella Netto
  1 sibling, 1 reply; 4+ messages in thread
From: Peter Bergner @ 2024-03-15 19:57 UTC (permalink / raw)
  To: Adhemerval Zanella, libc-alpha

I'm sorry I haven't replied earlier, but I just got back from vacation.
I also see you've pushed this already.  That said...

As with the power7 patch, I'm all for cleanups, especially if they
simplify things, remove more code than they add, and make things
faster.


On 3/5/24 2:13 PM, Adhemerval Zanella wrote:
> Similar to strstr (1e9a550ba4), power8 strcasestr does not show much
> improvement compared to the generic implementation.  The geomean
> on bench-strcasestr shows:
> 
>             __strcasestr_power8  __strcasestr_ppc
>   power10                  1159              1120
>   power9                   1640              1469
>   power8                   1787              1904

The generic implementation being the one in string/strcasestr.c,
correct?  Then how do I read the performance numbers above?

When Raji first added the power8 optimized routine, it was
showing big speedups.  I see that was before Wilco's changes
to the generic routine.  Do you think that was the main
reason why the generic implementation is better now?


Even though it's already pushed...

LGTM.

Reviewed-by: Peter Bergner <bergner@linux.ibm.com>  

Peter



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] powerpc: Remove power8 strcasestr optimization
  2024-03-15 19:57 ` Peter Bergner
@ 2024-03-18 12:52   ` Adhemerval Zanella Netto
  0 siblings, 0 replies; 4+ messages in thread
From: Adhemerval Zanella Netto @ 2024-03-18 12:52 UTC (permalink / raw)
  To: Peter Bergner, libc-alpha



On 15/03/24 16:57, Peter Bergner wrote:
> I'm sorry I haven't replied earlier, but I just got back from vacation.
> I also see you've pushed this already.  That said...
> 
> As with the power7 patch, I'm all for cleanups, especially if they
> simplify things, remove more code than they add, and make things
> faster.
> 
> 
> On 3/5/24 2:13 PM, Adhemerval Zanella wrote:
>> Similar to strstr (1e9a550ba4), power8 strcasestr does not show much
>> improvement compared to the generic implementation.  The geomean
>> on bench-strcasestr shows:
>>
>>             __strcasestr_power8  __strcasestr_ppc
>>   power10                  1159              1120
>>   power9                   1640              1469
>>   power8                   1787              1904
> 
> The generic implementation being the one in string/strcasestr.c,
> correct?  Then how do I read the performance numbers above?

Sorry about that, in fact I created these numbers with an improved
bench-strcasestr.c [1] benchmark based on bench-strstr.c and I forgot
to send it along the patch to remove the power implementation.
With this patch you should be able to use benchtests/scripts/compare_strings.py
to get the numbers.

> 
> When Raji first added the power8 optimized routine, it was
> showing big speedups.  I see that was before Wilco's changes
> to the generic routine.  Do you think that was the main
> reason why the generic implementation is better now?

Yes, but the change was not only motivated by this.  Wilco added
a set of improvements (3ae725dfb6d7f6144 which on aarch64 improved
performance by about 4.3% and 284f42bc778e487dfd5 which improved
by 3-4%). But the main problem with previous implementation was it
used a non-linear vector scan and added a hack (by counting the
iteration on the search) to fallback to generic version.  

This turns to waste a lot of cycles for this tests and it also make
the generic implementation to not take the improvement in the generic
one (not without rewrite it anyway).

We are trying to avoid such 'optimization', since they are really
hard to evaluate if they are not quadratic and we are trying to avoid
such pitfalls in the code.

However, I think there are still room for improvement on powerpc64le
since it uses ifunc for some functions that are not really required
since the minimum ISA is power8 (strnlen, strncat for instance).
We can just setup the built system to use the power8 version and
not provide ifunc variants for LE (as zseries and x86_64-vN are
doing).

> 
> 
> Even though it's already pushed...
> 
> LGTM.
> 
> Reviewed-by: Peter Bergner <bergner@linux.ibm.com>  

Thanks!

> 
> Peter
> 
> 

[1] https://sourceware.org/pipermail/libc-alpha/2024-March/155413.html

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-03-18 12:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-05 20:13 [PATCH] powerpc: Remove power8 strcasestr optimization Adhemerval Zanella
2024-03-12  2:37 ` DJ Delorie
2024-03-15 19:57 ` Peter Bergner
2024-03-18 12:52   ` Adhemerval Zanella Netto

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).