x86-64: Update strlen.S to support wcslen/wcsnlen

Message ID CAMe9rOrik_jJkM+vd9n7LuPEUR=xM7T8MC1cMiFXgnPsYUiTEw@mail.gmail.com
State New, archived
Headers

Commit Message

H.J. Lu May 30, 2017, 10:14 p.m. UTC
  On Sat, May 20, 2017 at 6:56 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> The difference between strlen and wcslen is byte vs int.  We can
> replace pminub and pcmpeqb with pminud and pcmpeqd to turn strlen
> into wcslen.  Tested on Ivy Bridge with benchtests/bench-wcslen.c,
> the new strlen based wcslen is as fast as the old wcslen.
>
> OK for master?
>
> H.J.
> ---
>         * sysdeps/x86_64/strlen.S (PMINU): New.
>         (PCMPEQ): Likewise.
>         (SHIFT_RETURN): Likewise.
>         (FIND_ZERO): Replace pcmpeqb with PCMPEQ.
>         (strlen): Add SHIFT_RETURN before ret.  Replace pcmpeqb and
>         pminub with PCMPEQ and PMINU.
>         * sysdeps/x86_64/wcslen.S: Define AS_WCSLEN and strlen.
>         Include "strlen.S".
>         * sysdeps/x86_64/wcsnlen.S: New file.

Here is the updated patch only with SSE2 wcsnlen.S.  Any
comments?
  

Comments

H.J. Lu June 5, 2017, 2:24 p.m. UTC | #1
On Tue, May 30, 2017 at 3:14 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Sat, May 20, 2017 at 6:56 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> The difference between strlen and wcslen is byte vs int.  We can
>> replace pminub and pcmpeqb with pminud and pcmpeqd to turn strlen
>> into wcslen.  Tested on Ivy Bridge with benchtests/bench-wcslen.c,
>> the new strlen based wcslen is as fast as the old wcslen.
>>
>> OK for master?
>>
>> H.J.
>> ---
>>         * sysdeps/x86_64/strlen.S (PMINU): New.
>>         (PCMPEQ): Likewise.
>>         (SHIFT_RETURN): Likewise.
>>         (FIND_ZERO): Replace pcmpeqb with PCMPEQ.
>>         (strlen): Add SHIFT_RETURN before ret.  Replace pcmpeqb and
>>         pminub with PCMPEQ and PMINU.
>>         * sysdeps/x86_64/wcslen.S: Define AS_WCSLEN and strlen.
>>         Include "strlen.S".
>>         * sysdeps/x86_64/wcsnlen.S: New file.
>
> Here is the updated patch only with SSE2 wcsnlen.S.  Any
> comments?
>

I will check it in today.
  
Markus Trippelsdorf June 6, 2017, 5:37 a.m. UTC | #2
On 2017.06.05 at 07:24 -0700, H.J. Lu wrote:
> On Tue, May 30, 2017 at 3:14 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> > On Sat, May 20, 2017 at 6:56 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> >> The difference between strlen and wcslen is byte vs int.  We can
> >> replace pminub and pcmpeqb with pminud and pcmpeqd to turn strlen
> >> into wcslen.  Tested on Ivy Bridge with benchtests/bench-wcslen.c,
> >> the new strlen based wcslen is as fast as the old wcslen.
> >>
> >> OK for master?
> >>
> >> H.J.
> >> ---
> >>         * sysdeps/x86_64/strlen.S (PMINU): New.
> >>         (PCMPEQ): Likewise.
> >>         (SHIFT_RETURN): Likewise.
> >>         (FIND_ZERO): Replace pcmpeqb with PCMPEQ.
> >>         (strlen): Add SHIFT_RETURN before ret.  Replace pcmpeqb and
> >>         pminub with PCMPEQ and PMINU.
> >>         * sysdeps/x86_64/wcslen.S: Define AS_WCSLEN and strlen.
> >>         Include "strlen.S".
> >>         * sysdeps/x86_64/wcsnlen.S: New file.
> >
> > Here is the updated patch only with SSE2 wcsnlen.S.  Any
> > comments?
> >
> 
> I will check it in today.

It doesn't work on old machines without SSE4.1:

FAIL: stdio-common/tstdiomisc               
FAIL: wcsmbs/test-wcpncpy                   
FAIL: wcsmbs/test-wcsncmp                   
FAIL: wcsmbs/test-wcsncpy                   
FAIL: wcsmbs/test-wcsnlen                   
FAIL: wcsmbs/wcsatcliff

markus@x4 tmp % gdb --args /var/tmp/glibc-build/elf/ld-linux-x86-64.so.2 --library-path /var/tmp/glibc-build:/var/tmp/glibc-build/math:/var/tmp/glibc-build/elf:/var/tmp/gl[31/150]d/dlfcn:/var/tmp/glibc-build/nss:/var/tmp/glibc-build/nis:/var/tmp/glibc-build/rt:/var/tmp/glibc-build/resolv:/var/tmp/glibc-build/crypt:/var/tmp/glibc-build/mathvec:/var/tmp/glib
c-build/support:/var/tmp/glibc-build/nptl /var/tmp/glibc-build/stdio-common/tstdiomisc   
Reading symbols from /var/tmp/glibc-build/elf/ld-linux-x86-64.so.2...done.               
(gdb) run                                                                                
Starting program: /home/markus/tmp/glibc-build/elf/ld-linux-x86-64.so.2 --library-path /var/tmp/glibc-build:/var/tmp/glibc-build/math:/var/tmp/glibc-build/elf:/var/tmp/glibc-build/dlfcn:/var/tmp/glibc-build/nss:/var/tmp/glibc-build/nis:/var/tmp/glibc-build/rt:/var/tmp/glibc-build/resolv:/var/tmp/glibc-build/crypt:/var/tmp/glibc-build/mathvec:/var/tmp/glibc-build/support:/var/tmp/glibc-build/nptl /var/tmp/glibc-build/stdio-common/tstdiomisc    
t1: count=5                                                                              
sscanf ("12345", "%ld", &x) => 1, x = 12345                                              
sscanf ("12345", "%llllld", &x) => 0, x = -1                                             
sscanf ("12345", "%LLLLLd", &x) => 0, x = -1                                             
sscanf ("test ", "%*s%n", &x) => 0, x = 4                                                
sscanf ("test ", "%2*s%n", &x) => 0, x = -1                                              
sscanf ("12 ", "%l2d", &x) => 0, x = -1                                                  
sscanf ("12 ", "%2ld", &x) => 1, x = 12                                                  
sscanf ("1 1", "%d %Z", &n, &N) => 1, n = 1, N = -1                                      
expected "nan NAN nan NAN nan NAN nan NAN", got "nan NAN nan NAN nan NAN nan NAN"        
expected "-nan -NAN -nan -NAN -nan -NAN -nan -NAN", got "-nan -NAN -nan -NAN -nan -NAN -nan -NAN"                                                                                  
expected "nan NAN nan NAN nan NAN nan NAN", got "nan NAN nan NAN nan NAN nan NAN"        
expected "-nan -NAN -nan -NAN -nan -NAN -nan -NAN", got "-nan -NAN -nan -NAN -nan -NAN -nan -NAN"                                                                                  expected "inf INF inf INF inf INF inf INF", got "inf INF inf INF inf INF inf INF"        
expected "-inf -INF -inf -INF -inf -INF -inf -INF", got "-inf -INF -inf -INF -inf -INF -inf -INF"                                                                                                                              
Program received signal SIGILL, Illegal instruction.                                     
wcsnlen () at ../sysdeps/x86_64/strlen.S:180                                             
180             PMINU   16(%rax), %xmm0
  

Patch

From 5798d0fbf03c531dfcd8d145217a5fbfd2319d95 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sat, 20 May 2017 06:48:04 -0700
Subject: [PATCH] x86-64: Update strlen.S to support wcslen/wcsnlen

The difference between strlen and wcslen is byte vs int.  We can replace
pminub and pcmpeqb with pminud and pcmpeqd to turn strlen/strnlen into
wcslen/wcsnlen.  The SSE2 wcsnlen is up to 4X faster on Haswell.

	* sysdeps/x86_64/strlen.S (PMINU): New.
	(PCMPEQ): Likewise.
	(SHIFT_RETURN): Likewise.
	(FIND_ZERO): Replace pcmpeqb with PCMPEQ.
	(strlen): Add SHIFT_RETURN before ret.  Replace pcmpeqb and
	pminub with PCMPEQ and PMINU.
	* sysdeps/x86_64/wcsnlen.S: New file.
---
 sysdeps/x86_64/strlen.S  | 61 +++++++++++++++++++++++++++++++-----------------
 sysdeps/x86_64/wcsnlen.S |  7 ++++++
 2 files changed, 47 insertions(+), 21 deletions(-)
 create mode 100644 sysdeps/x86_64/wcsnlen.S

diff --git a/sysdeps/x86_64/strlen.S b/sysdeps/x86_64/strlen.S
index 5896e6b9..b5ab117 100644
--- a/sysdeps/x86_64/strlen.S
+++ b/sysdeps/x86_64/strlen.S
@@ -1,4 +1,4 @@ 
-/* SSE2 version of strlen.
+/* SSE2 version of strlen/wcslen.
    Copyright (C) 2012-2017 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
@@ -18,6 +18,16 @@ 
 
 #include <sysdep.h>
 
+#ifdef AS_WCSLEN
+# define PMINU		pminud
+# define PCMPEQ		pcmpeqd
+# define SHIFT_RETURN	shrq $2, %rax
+#else
+# define PMINU		pminub
+# define PCMPEQ		pcmpeqb
+# define SHIFT_RETURN
+#endif
+
 /* Long lived register in strlen(s), strnlen(s, n) are:
 
 	%xmm3 - zero
@@ -32,10 +42,10 @@  ENTRY(strlen)
 
 /* Test 64 bytes from %rax for zero. Save result as bitmask in %rdx.  */
 #define FIND_ZERO	\
-	pcmpeqb	(%rax), %xmm0;	\
-	pcmpeqb	16(%rax), %xmm1;	\
-	pcmpeqb	32(%rax), %xmm2;	\
-	pcmpeqb	48(%rax), %xmm3;	\
+	PCMPEQ	(%rax), %xmm0;	\
+	PCMPEQ	16(%rax), %xmm1;	\
+	PCMPEQ	32(%rax), %xmm2;	\
+	PCMPEQ	48(%rax), %xmm3;	\
 	pmovmskb	%xmm0, %esi;	\
 	pmovmskb	%xmm1, %edx;	\
 	pmovmskb	%xmm2, %r8d;	\
@@ -54,6 +64,9 @@  ENTRY(strlen)
 	xor	%rax, %rax
 	ret
 L(n_nonzero):
+# ifdef AS_WCSLEN
+	shlq	$2, %rsi
+# endif
 
 /* Initialize long lived registers.  */
 
@@ -96,6 +109,7 @@  L(n_nonzero):
 	test	%rdx, %rdx;	\
 	je	L(lab);	\
 	bsfq	%rdx, %rax;	\
+	SHIFT_RETURN;		\
 	ret
 
 #ifdef AS_STRNLEN
@@ -104,19 +118,20 @@  L(n_nonzero):
 #else
 	/* Test first 16 bytes unaligned.  */
 	movdqu	(%rax), %xmm4
-	pcmpeqb	%xmm0, %xmm4
+	PCMPEQ	%xmm0, %xmm4
 	pmovmskb	%xmm4, %edx
 	test	%edx, %edx
 	je 	L(next48_bytes)
 	bsf	%edx, %eax /* If eax is zeroed 16bit bsf can be used.  */
+	SHIFT_RETURN
 	ret
 
 L(next48_bytes):
 /* Same as FIND_ZERO except we do not check first 16 bytes.  */
 	andq	$-16, %rax
-	pcmpeqb 16(%rax), %xmm1
-	pcmpeqb 32(%rax), %xmm2
-	pcmpeqb 48(%rax), %xmm3
+	PCMPEQ 16(%rax), %xmm1
+	PCMPEQ 32(%rax), %xmm2
+	PCMPEQ 48(%rax), %xmm3
 	pmovmskb	%xmm1, %edx
 	pmovmskb	%xmm2, %r8d
 	pmovmskb	%xmm3, %ecx
@@ -145,6 +160,7 @@  L(strnlen_ret):
 	test	%rdx, %rdx
 	je	L(loop_init)
 	bsfq	%rdx, %rax
+	SHIFT_RETURN
 	ret
 #endif
 	.p2align 4
@@ -161,10 +177,10 @@  L(loop):
 	je	L(exit_end)
 
 	movdqa	(%rax), %xmm0
-	pminub	16(%rax), %xmm0
-	pminub	32(%rax), %xmm0
-	pminub	48(%rax), %xmm0
-	pcmpeqb	%xmm3, %xmm0
+	PMINU	16(%rax), %xmm0
+	PMINU	32(%rax), %xmm0
+	PMINU	48(%rax), %xmm0
+	PCMPEQ	%xmm3, %xmm0
 	pmovmskb	%xmm0, %edx
 	testl	%edx, %edx
 	jne	L(exit)
@@ -182,6 +198,7 @@  L(first):
 	bsfq	%rdx, %rdx
 	addq	%rdx, %rax
 	subq	%rdi, %rax
+	SHIFT_RETURN
 	ret
 
 	.p2align 4
@@ -192,6 +209,7 @@  L(exit):
 	bsfq	%rdx, %rdx
 	addq	%rdx, %rax
 	subq	%rdi, %rax
+	SHIFT_RETURN
 	ret
 
 #else
@@ -201,10 +219,10 @@  L(exit):
 L(loop):
 
 	movdqa	64(%rax), %xmm0
-	pminub	80(%rax), %xmm0
-	pminub	96(%rax), %xmm0
-	pminub	112(%rax), %xmm0
-	pcmpeqb	%xmm3, %xmm0
+	PMINU	80(%rax), %xmm0
+	PMINU	96(%rax), %xmm0
+	PMINU	112(%rax), %xmm0
+	PCMPEQ	%xmm3, %xmm0
 	pmovmskb	%xmm0, %edx
 	testl	%edx, %edx
 	jne	L(exit64)
@@ -212,10 +230,10 @@  L(loop):
 	subq	$-128, %rax
 
 	movdqa	(%rax), %xmm0
-	pminub	16(%rax), %xmm0
-	pminub	32(%rax), %xmm0
-	pminub	48(%rax), %xmm0
-	pcmpeqb	%xmm3, %xmm0
+	PMINU	16(%rax), %xmm0
+	PMINU	32(%rax), %xmm0
+	PMINU	48(%rax), %xmm0
+	PCMPEQ	%xmm3, %xmm0
 	pmovmskb	%xmm0, %edx
 	testl	%edx, %edx
 	jne	L(exit0)
@@ -231,6 +249,7 @@  L(exit0):
 	bsfq	%rdx, %rdx
 	addq	%rdx, %rax
 	subq	%rdi, %rax
+	SHIFT_RETURN
 	ret
 
 #endif
diff --git a/sysdeps/x86_64/wcsnlen.S b/sysdeps/x86_64/wcsnlen.S
new file mode 100644
index 0000000..968bb69
--- /dev/null
+++ b/sysdeps/x86_64/wcsnlen.S
@@ -0,0 +1,7 @@ 
+#define AS_WCSLEN
+#define AS_STRNLEN
+#define strlen	__wcsnlen
+
+#include "strlen.S"
+
+weak_alias(__wcsnlen, wcsnlen)
-- 
2.9.4