[PATCH] aarch64: Revert "aarch64: Optimized implementation of strnlen" [BZ #25825]

[PATCH] aarch64: Revert "aarch64: Optimized implementation of strnlen" [BZ #25825]

[zhuyan (M)]

After commit 2911cb68ed3d6c515ad1979237e74e1fefab3674("aarch64:
Optimized implementation of strnlen"), there is a problem when calling the strnlen interface to handle the string length of 9 or more

This reverts commit 2911cb68ed3d6c515ad1979237e74e1fefab3674.
---
 sysdeps/aarch64/strnlen.S | 52 +----------------------------------------------
 1 file changed, 1 insertion(+), 51 deletions(-)

diff --git a/sysdeps/aarch64/strnlen.S b/sysdeps/aarch64/strnlen.S index 5981247dd9..2b55eda6db 100644
--- a/sysdeps/aarch64/strnlen.S
+++ b/sysdeps/aarch64/strnlen.S
@@ -45,11 +45,6 @@
 #define pos		x13
 #define limit_wd	x14
 
-#define dataq		q2
-#define datav		v2
-#define datab2		b3
-#define dataq2		q3
-#define datav2		v3
 #define REP8_01 0x0101010101010101
 #define REP8_7f 0x7f7f7f7f7f7f7f7f
 #define REP8_80 0x8080808080808080
@@ -76,7 +71,7 @@ ENTRY_ALIGN_AND_PAD (__strnlen, 6, 9)
 	   cycle, as we get much better parallelism out of the operations.  */
 
 	/* Start of critial section -- keep to one 64Byte cache line.  */
-
+L(loop):
 	ldp	data1, data2, [src], #16
 L(realigned):
 	sub	tmp1, data1, zeroones
@@ -124,51 +119,6 @@ L(nul_in_data2):
 	csel	len, len, limit, ls		/* Return the lower value.  */
 	RET
 
-L(loop):
-	ldr	dataq, [src], #16
-	uminv	datab2, datav.16b
-	mov	tmp1, datav2.d[0]
-	subs	limit_wd, limit_wd, #1
-	ccmp	tmp1, #0, #4, pl	/* NZCV = 0000  */
-	b.eq	L(loop_end)
-	ldr	dataq, [src], #16
-	uminv	datab2, datav.16b
-	mov	tmp1, datav2.d[0]
-	subs	limit_wd, limit_wd, #1
-	ccmp	tmp1, #0, #4, pl	/* NZCV = 0000  */
-	b.ne	L(loop)
-L(loop_end):
-	/* End of critical section -- keep to one 64Byte cache line.  */
-
-	cbnz	tmp1, L(hit_limit)	/* No null in final Qword.  */
-
-	/* We know there's a null in the final Qword.  The easiest thing
-	   to do now is work out the length of the string and return
-	   MIN (len, limit).  */
-
-#ifdef __AARCH64EB__
-	rev64	datav.16b, datav.16b
-#endif
-	/* Set te NULL byte as 0xff and the rest as 0x00, move the data into a
-	   pair of scalars and then compute the length from the earliest NULL
-	   byte.  */
-
-	cmeq	datav.16b, datav.16b, #0
-	mov	data1, datav.d[0]
-	mov	data2, datav.d[1]
-	cmp	data1, 0
-	csel	data1, data1, data2, ne
-	sub	len, src, srcin
-	sub	len, len, #16
-	rev	data1, data1
-	add	tmp2, len, 8
-	clz	tmp1, data1
-	csel	len, len, tmp2, ne
-	add	len, len, tmp1, lsr 3
-	cmp	len, limit
-	csel	len, len, limit, ls		/* Return the lower value.  */
-	RET
-
 L(misaligned):
 	/* Deal with a partial first word.
 	   We're doing two things in parallel here;
--
2.12.3

Re: [PATCH] aarch64: Revert "aarch64: Optimized implementation of strnlen" [BZ #25825]

[Florian Weimer]

* zhuyan:

> After commit 2911cb68ed3d6c515ad1979237e74e1fefab3674("aarch64:
> Optimized implementation of strnlen"), there is a problem when calling the strnlen interface to handle the string length of 9 or more
>
> This reverts commit 2911cb68ed3d6c515ad1979237e74e1fefab3674.

Do you have a test case for this bug?  Or is it already caught by the
test suite?

Thanks,
Florian

Re: [PATCH] aarch64: Revert "aarch64: Optimized implementation of strnlen" [BZ #25825]

[Andreas Schwab]

On Apr 15 2020, Florian Weimer via Libc-alpha wrote:

> * zhuyan:
>
>> After commit 2911cb68ed3d6c515ad1979237e74e1fefab3674("aarch64:
>> Optimized implementation of strnlen"), there is a problem when calling the strnlen interface to handle the string length of 9 or more
>>
>> This reverts commit 2911cb68ed3d6c515ad1979237e74e1fefab3674.
>
> Do you have a test case for this bug?  Or is it already caught by the
> test suite?

https://build.opensuse.org/package/live_build_log/home:Andreas_Schwab:glibc/glibc:testsuite/a/aarch64
has no related failures.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] aarch64: Revert "aarch64: Optimized implementation of strnlen" [BZ #25825]

[Wilco Dijkstra]

Hi Zhuyan,

I think a much simpler fix is to change ldr into ld1 and remove the rev64 altogether.
The same fix will be needed in strlen_asimd.S and strcpy.S.

Cheers,
Wilco

Re: [PATCH] aarch64: Revert "aarch64: Optimized implementation of strnlen" [BZ #25825]

[zhuyan (M)]

On Apr 15 2020, Andreas wrote:
>On Apr 15 2020, Florian Weimer via Libc-alpha wrote:
>
>> * zhuyan:
>>
>>> After commit 2911cb68ed3d6c515ad1979237e74e1fefab3674("aarch64:
>>> Optimized implementation of strnlen"), there is a problem when 
>>> calling the strnlen interface to handle the string length of 9 or 
>>> more
>>>
>>> This reverts commit 2911cb68ed3d6c515ad1979237e74e1fefab3674.
>>
>> Do you have a test case for this bug?  Or is it already caught by the 
>> test suite?
>
>https://build.opensuse.org/package/live_build_log/home:Andreas_Schwab:glibc/glibc:testsuite/a/aarch64
>has no related failures.

There is a testcase.

#include <stdio.h>
#include <string.h>

void init_base(char *base, int len)
{
	int i;
	for (i = 0; i < len; i++) {
		base[i] = 'x';
	}
	base[len] = '\0';
}

int main(int argc, char *argv[])
{
	int i;
	char base[1000];
	char buff[1000];
	size_t n;
	
	for (i = 1; i < 1000; i++) {
		memset(buff, 0, 1000);
		init_base(base, i);		
		n = strnlen(base, 1000);      
		if ( i == n )
			printf("size=%d is equel\n", i);
		else
			printf("size=%d is not equel%s\n", i);
	}

    return 0;
}


The execution log is as follows：

arm64be /tmp # ./arm64be-test_strnlen
size=1 is equel
size=2 is equel
size=3 is equel
size=4 is equel
size=5 is equel
size=6 is equel
size=7 is equel
size=8 is equel
size=9 is not equel
size=10 is not equel
size=11 is not equel
size=12 is not equel
size=13 is not equel


Thanks,
Yan Zhu