diff mbox series

[v1,1/3] arm64: lib: Import latest version of Arm Optimized Routines' strcmp

Message ID 20220215170723.21266-2-joey.gouly@arm.com (mailing list archive)
State New, archived
Headers show
Series Import Arm Optimized Routines str{n}cmp functions | expand

Commit Message

Joey Gouly Feb. 15, 2022, 5:07 p.m. UTC
Import the latest version of the Arm Optimized Routines strcmp function based
on the upstream code of string/aarch64/strcmp.S at commit 189dfefe37d5 from:
  https://github.com/ARM-software/optimized-routines

This latest version includes MTE support.

Signed-off-by: Joey Gouly <joey.gouly@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/lib/strcmp.S | 238 +++++++++++++++++++++-------------------
 1 file changed, 126 insertions(+), 112 deletions(-)

Comments

Russell King (Oracle) Feb. 16, 2022, 4:44 p.m. UTC | #1
On Tue, Feb 15, 2022 at 05:07:21PM +0000, Joey Gouly wrote:
> Import the latest version of the Arm Optimized Routines strcmp function based
> on the upstream code of string/aarch64/strcmp.S at commit 189dfefe37d5 from:
>   https://github.com/ARM-software/optimized-routines
> 
> This latest version includes MTE support.
> 
> Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> ---
>  arch/arm64/lib/strcmp.S | 238 +++++++++++++++++++++-------------------
>  1 file changed, 126 insertions(+), 112 deletions(-)
> 
> diff --git a/arch/arm64/lib/strcmp.S b/arch/arm64/lib/strcmp.S
> index 83bcad72ec97..758de77afd2f 100644
> --- a/arch/arm64/lib/strcmp.S
> +++ b/arch/arm64/lib/strcmp.S
> @@ -1,9 +1,9 @@
>  /* SPDX-License-Identifier: GPL-2.0-only */

Looking at the LICENSE file in the above repository, it appears that
this code is licensed as "MIT OR Apache-2.0 WITH LLVM-exception".
Shouldn't the SPDX line be updated to reflect the origin license of
this code?
Robin Murphy Feb. 16, 2022, 6:36 p.m. UTC | #2
On 2022-02-16 16:44, Russell King (Oracle) wrote:
> On Tue, Feb 15, 2022 at 05:07:21PM +0000, Joey Gouly wrote:
>> Import the latest version of the Arm Optimized Routines strcmp function based
>> on the upstream code of string/aarch64/strcmp.S at commit 189dfefe37d5 from:
>>    https://github.com/ARM-software/optimized-routines
>>
>> This latest version includes MTE support.
>>
>> Signed-off-by: Joey Gouly <joey.gouly@arm.com>
>> Cc: Robin Murphy <robin.murphy@arm.com>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> ---
>>   arch/arm64/lib/strcmp.S | 238 +++++++++++++++++++++-------------------
>>   1 file changed, 126 insertions(+), 112 deletions(-)
>>
>> diff --git a/arch/arm64/lib/strcmp.S b/arch/arm64/lib/strcmp.S
>> index 83bcad72ec97..758de77afd2f 100644
>> --- a/arch/arm64/lib/strcmp.S
>> +++ b/arch/arm64/lib/strcmp.S
>> @@ -1,9 +1,9 @@
>>   /* SPDX-License-Identifier: GPL-2.0-only */
> 
> Looking at the LICENSE file in the above repository, it appears that
> this code is licensed as "MIT OR Apache-2.0 WITH LLVM-exception".
> Shouldn't the SPDX line be updated to reflect the origin license of
> this code?

This is noted in the commits which first imported implementations from 
Arm Optimized Routines (020b199bc70d and earlier):

  "Note that for simplicity Arm have chosen to contribute this code
   to Linux under GPLv2 rather than the original MIT license."

Apologies for the confusion - I should have mentioned that to Joey 
beforehand, if I hadn't completely forgotten about it. I think it's just 
been implicit that we'd continue to follow the same approach going forward.

Thanks,
Robin.
Joey Gouly Feb. 17, 2022, 10:23 a.m. UTC | #3
On Wed, Feb 16, 2022 at 06:36:12PM +0000, Robin Murphy wrote:
> On 2022-02-16 16:44, Russell King (Oracle) wrote:
> > On Tue, Feb 15, 2022 at 05:07:21PM +0000, Joey Gouly wrote:
> > > Import the latest version of the Arm Optimized Routines strcmp function based
> > > on the upstream code of string/aarch64/strcmp.S at commit 189dfefe37d5 from:
> > >    https://github.com/ARM-software/optimized-routines
> > > 
> > > This latest version includes MTE support.
> > > 
> > > Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> > > Cc: Robin Murphy <robin.murphy@arm.com>
> > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > Cc: Will Deacon <will@kernel.org>
> > > ---
> > >   arch/arm64/lib/strcmp.S | 238 +++++++++++++++++++++-------------------
> > >   1 file changed, 126 insertions(+), 112 deletions(-)
> > > 
> > > diff --git a/arch/arm64/lib/strcmp.S b/arch/arm64/lib/strcmp.S
> > > index 83bcad72ec97..758de77afd2f 100644
> > > --- a/arch/arm64/lib/strcmp.S
> > > +++ b/arch/arm64/lib/strcmp.S
> > > @@ -1,9 +1,9 @@
> > >   /* SPDX-License-Identifier: GPL-2.0-only */
> > 
> > Looking at the LICENSE file in the above repository, it appears that
> > this code is licensed as "MIT OR Apache-2.0 WITH LLVM-exception".
> > Shouldn't the SPDX line be updated to reflect the origin license of
> > this code?
> 
> This is noted in the commits which first imported implementations from Arm
> Optimized Routines (020b199bc70d and earlier):
> 
>  "Note that for simplicity Arm have chosen to contribute this code
>   to Linux under GPLv2 rather than the original MIT license."
> 
> Apologies for the confusion - I should have mentioned that to Joey
> beforehand, if I hadn't completely forgotten about it. I think it's just
> been implicit that we'd continue to follow the same approach going forward.
> 

Yes, I didn't mention it because I was just being implicit.

I've added a note about it in the commit message, and will send out a v2 after
deciding what we should do about the conflict with
https://lore.kernel.org/linux-arm-kernel/20220216162229.1076788-1-mark.rutland@arm.com/
(since I may have to rebase the patches onto a different base).

Thanks,
Joey
Will Deacon Feb. 25, 2022, 2:21 p.m. UTC | #4
On Thu, Feb 17, 2022 at 10:23:09AM +0000, Joey Gouly wrote:
> On Wed, Feb 16, 2022 at 06:36:12PM +0000, Robin Murphy wrote:
> > On 2022-02-16 16:44, Russell King (Oracle) wrote:
> > > On Tue, Feb 15, 2022 at 05:07:21PM +0000, Joey Gouly wrote:
> > > > Import the latest version of the Arm Optimized Routines strcmp function based
> > > > on the upstream code of string/aarch64/strcmp.S at commit 189dfefe37d5 from:
> > > >    https://github.com/ARM-software/optimized-routines
> > > > 
> > > > This latest version includes MTE support.
> > > > 
> > > > Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> > > > Cc: Robin Murphy <robin.murphy@arm.com>
> > > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > > Cc: Will Deacon <will@kernel.org>
> > > > ---
> > > >   arch/arm64/lib/strcmp.S | 238 +++++++++++++++++++++-------------------
> > > >   1 file changed, 126 insertions(+), 112 deletions(-)
> > > > 
> > > > diff --git a/arch/arm64/lib/strcmp.S b/arch/arm64/lib/strcmp.S
> > > > index 83bcad72ec97..758de77afd2f 100644
> > > > --- a/arch/arm64/lib/strcmp.S
> > > > +++ b/arch/arm64/lib/strcmp.S
> > > > @@ -1,9 +1,9 @@
> > > >   /* SPDX-License-Identifier: GPL-2.0-only */
> > > 
> > > Looking at the LICENSE file in the above repository, it appears that
> > > this code is licensed as "MIT OR Apache-2.0 WITH LLVM-exception".
> > > Shouldn't the SPDX line be updated to reflect the origin license of
> > > this code?
> > 
> > This is noted in the commits which first imported implementations from Arm
> > Optimized Routines (020b199bc70d and earlier):
> > 
> >  "Note that for simplicity Arm have chosen to contribute this code
> >   to Linux under GPLv2 rather than the original MIT license."
> > 
> > Apologies for the confusion - I should have mentioned that to Joey
> > beforehand, if I hadn't completely forgotten about it. I think it's just
> > been implicit that we'd continue to follow the same approach going forward.
> > 
> 
> Yes, I didn't mention it because I was just being implicit.
> 
> I've added a note about it in the commit message, and will send out a v2 after
> deciding what we should do about the conflict with
> https://lore.kernel.org/linux-arm-kernel/20220216162229.1076788-1-mark.rutland@arm.com/
> (since I may have to rebase the patches onto a different base).

Please just send a new version based on -rc3 and we can figure out the
conflicts when I merge it together.

Will
diff mbox series

Patch

diff --git a/arch/arm64/lib/strcmp.S b/arch/arm64/lib/strcmp.S
index 83bcad72ec97..758de77afd2f 100644
--- a/arch/arm64/lib/strcmp.S
+++ b/arch/arm64/lib/strcmp.S
@@ -1,9 +1,9 @@ 
 /* SPDX-License-Identifier: GPL-2.0-only */
 /*
- * Copyright (c) 2012-2021, Arm Limited.
+ * Copyright (c) 2012-2022, Arm Limited.
  *
  * Adapted from the original at:
- * https://github.com/ARM-software/optimized-routines/blob/afd6244a1f8d9229/string/aarch64/strcmp.S
+ * https://github.com/ARM-software/optimized-routines/blob/189dfefe37d54c5b/string/aarch64/strcmp.S
  */
 
 #include <linux/linkage.h>
@@ -11,161 +11,175 @@ 
 
 /* Assumptions:
  *
- * ARMv8-a, AArch64
+ * ARMv8-a, AArch64.
+ * MTE compatible.
  */
 
 #define L(label) .L ## label
 
 #define REP8_01 0x0101010101010101
 #define REP8_7f 0x7f7f7f7f7f7f7f7f
-#define REP8_80 0x8080808080808080
 
-/* Parameters and result.  */
 #define src1		x0
 #define src2		x1
 #define result		x0
 
-/* Internal variables.  */
 #define data1		x2
 #define data1w		w2
 #define data2		x3
 #define data2w		w3
 #define has_nul		x4
 #define diff		x5
+#define off1		x5
 #define syndrome	x6
-#define tmp1		x7
-#define tmp2		x8
-#define tmp3		x9
-#define zeroones	x10
-#define pos		x11
-
-	/* Start of performance-critical section  -- one 64B cache line.  */
-	.align 6
+#define tmp		x6
+#define data3		x7
+#define zeroones	x8
+#define shift		x9
+#define off2		x10
+
+/* On big-endian early bytes are at MSB and on little-endian LSB.
+   LS_FW means shifting towards early bytes.  */
+#ifdef __AARCH64EB__
+# define LS_FW lsl
+#else
+# define LS_FW lsr
+#endif
+
+/* NUL detection works on the principle that (X - 1) & (~X) & 0x80
+   (=> (X - 1) & ~(X | 0x7f)) is non-zero iff a byte is zero, and
+   can be done in parallel across the entire word.
+   Since carry propagation makes 0x1 bytes before a NUL byte appear
+   NUL too in big-endian, byte-reverse the data before the NUL check.  */
+
+
 SYM_FUNC_START_WEAK_PI(strcmp)
-	eor	tmp1, src1, src2
-	mov	zeroones, #REP8_01
-	tst	tmp1, #7
+	sub	off2, src2, src1
+	mov	zeroones, REP8_01
+	and	tmp, src1, 7
+	tst	off2, 7
 	b.ne	L(misaligned8)
-	ands	tmp1, src1, #7
-	b.ne	L(mutual_align)
-	/* NUL detection works on the principle that (X - 1) & (~X) & 0x80
-	   (=> (X - 1) & ~(X | 0x7f)) is non-zero iff a byte is zero, and
-	   can be done in parallel across the entire word.  */
+	cbnz	tmp, L(mutual_align)
+
+	.p2align 4
+
 L(loop_aligned):
-	ldr	data1, [src1], #8
-	ldr	data2, [src2], #8
+	ldr	data2, [src1, off2]
+	ldr	data1, [src1], 8
 L(start_realigned):
-	sub	tmp1, data1, zeroones
-	orr	tmp2, data1, #REP8_7f
-	eor	diff, data1, data2	/* Non-zero if differences found.  */
-	bic	has_nul, tmp1, tmp2	/* Non-zero if NUL terminator.  */
+#ifdef __AARCH64EB__
+	rev	tmp, data1
+	sub	has_nul, tmp, zeroones
+	orr	tmp, tmp, REP8_7f
+#else
+	sub	has_nul, data1, zeroones
+	orr	tmp, data1, REP8_7f
+#endif
+	bics	has_nul, has_nul, tmp	/* Non-zero if NUL terminator.  */
+	ccmp	data1, data2, 0, eq
+	b.eq	L(loop_aligned)
+#ifdef __AARCH64EB__
+	rev	has_nul, has_nul
+#endif
+	eor	diff, data1, data2
 	orr	syndrome, diff, has_nul
-	cbz	syndrome, L(loop_aligned)
-	/* End of performance-critical section  -- one 64B cache line.  */
-
 L(end):
-#ifndef	__AARCH64EB__
+#ifndef __AARCH64EB__
 	rev	syndrome, syndrome
 	rev	data1, data1
-	/* The MS-non-zero bit of the syndrome marks either the first bit
-	   that is different, or the top bit of the first zero byte.
-	   Shifting left now will bring the critical information into the
-	   top bits.  */
-	clz	pos, syndrome
 	rev	data2, data2
-	lsl	data1, data1, pos
-	lsl	data2, data2, pos
-	/* But we need to zero-extend (char is unsigned) the value and then
-	   perform a signed 32-bit subtraction.  */
-	lsr	data1, data1, #56
-	sub	result, data1, data2, lsr #56
-	ret
-#else
-	/* For big-endian we cannot use the trick with the syndrome value
-	   as carry-propagation can corrupt the upper bits if the trailing
-	   bytes in the string contain 0x01.  */
-	/* However, if there is no NUL byte in the dword, we can generate
-	   the result directly.  We can't just subtract the bytes as the
-	   MSB might be significant.  */
-	cbnz	has_nul, 1f
-	cmp	data1, data2
-	cset	result, ne
-	cneg	result, result, lo
-	ret
-1:
-	/* Re-compute the NUL-byte detection, using a byte-reversed value.  */
-	rev	tmp3, data1
-	sub	tmp1, tmp3, zeroones
-	orr	tmp2, tmp3, #REP8_7f
-	bic	has_nul, tmp1, tmp2
-	rev	has_nul, has_nul
-	orr	syndrome, diff, has_nul
-	clz	pos, syndrome
-	/* The MS-non-zero bit of the syndrome marks either the first bit
-	   that is different, or the top bit of the first zero byte.
+#endif
+	clz	shift, syndrome
+	/* The most-significant-non-zero bit of the syndrome marks either the
+	   first bit that is different, or the top bit of the first zero byte.
 	   Shifting left now will bring the critical information into the
 	   top bits.  */
-	lsl	data1, data1, pos
-	lsl	data2, data2, pos
+	lsl	data1, data1, shift
+	lsl	data2, data2, shift
 	/* But we need to zero-extend (char is unsigned) the value and then
 	   perform a signed 32-bit subtraction.  */
-	lsr	data1, data1, #56
-	sub	result, data1, data2, lsr #56
+	lsr	data1, data1, 56
+	sub	result, data1, data2, lsr 56
 	ret
-#endif
+
+	.p2align 4
 
 L(mutual_align):
 	/* Sources are mutually aligned, but are not currently at an
 	   alignment boundary.  Round down the addresses and then mask off
-	   the bytes that preceed the start point.  */
-	bic	src1, src1, #7
-	bic	src2, src2, #7
-	lsl	tmp1, tmp1, #3		/* Bytes beyond alignment -> bits.  */
-	ldr	data1, [src1], #8
-	neg	tmp1, tmp1		/* Bits to alignment -64.  */
-	ldr	data2, [src2], #8
-	mov	tmp2, #~0
-#ifdef __AARCH64EB__
-	/* Big-endian.  Early bytes are at MSB.  */
-	lsl	tmp2, tmp2, tmp1	/* Shift (tmp1 & 63).  */
-#else
-	/* Little-endian.  Early bytes are at LSB.  */
-	lsr	tmp2, tmp2, tmp1	/* Shift (tmp1 & 63).  */
-#endif
-	orr	data1, data1, tmp2
-	orr	data2, data2, tmp2
+	   the bytes that precede the start point.  */
+	bic	src1, src1, 7
+	ldr	data2, [src1, off2]
+	ldr	data1, [src1], 8
+	neg	shift, src2, lsl 3	/* Bits to alignment -64.  */
+	mov	tmp, -1
+	LS_FW	tmp, tmp, shift
+	orr	data1, data1, tmp
+	orr	data2, data2, tmp
 	b	L(start_realigned)
 
 L(misaligned8):
 	/* Align SRC1 to 8 bytes and then compare 8 bytes at a time, always
-	   checking to make sure that we don't access beyond page boundary in
-	   SRC2.  */
-	tst	src1, #7
-	b.eq	L(loop_misaligned)
+	   checking to make sure that we don't access beyond the end of SRC2.  */
+	cbz	tmp, L(src1_aligned)
 L(do_misaligned):
-	ldrb	data1w, [src1], #1
-	ldrb	data2w, [src2], #1
-	cmp	data1w, #1
-	ccmp	data1w, data2w, #0, cs	/* NZCV = 0b0000.  */
+	ldrb	data1w, [src1], 1
+	ldrb	data2w, [src2], 1
+	cmp	data1w, 0
+	ccmp	data1w, data2w, 0, ne	/* NZCV = 0b0000.  */
 	b.ne	L(done)
-	tst	src1, #7
+	tst	src1, 7
 	b.ne	L(do_misaligned)
 
-L(loop_misaligned):
-	/* Test if we are within the last dword of the end of a 4K page.  If
-	   yes then jump back to the misaligned loop to copy a byte at a time.  */
-	and	tmp1, src2, #0xff8
-	eor	tmp1, tmp1, #0xff8
-	cbz	tmp1, L(do_misaligned)
-	ldr	data1, [src1], #8
-	ldr	data2, [src2], #8
-
-	sub	tmp1, data1, zeroones
-	orr	tmp2, data1, #REP8_7f
-	eor	diff, data1, data2	/* Non-zero if differences found.  */
-	bic	has_nul, tmp1, tmp2	/* Non-zero if NUL terminator.  */
+L(src1_aligned):
+	neg	shift, src2, lsl 3
+	bic	src2, src2, 7
+	ldr	data3, [src2], 8
+#ifdef __AARCH64EB__
+	rev	data3, data3
+#endif
+	lsr	tmp, zeroones, shift
+	orr	data3, data3, tmp
+	sub	has_nul, data3, zeroones
+	orr	tmp, data3, REP8_7f
+	bics	has_nul, has_nul, tmp
+	b.ne	L(tail)
+
+	sub	off1, src2, src1
+
+	.p2align 4
+
+L(loop_unaligned):
+	ldr	data3, [src1, off1]
+	ldr	data2, [src1, off2]
+#ifdef __AARCH64EB__
+	rev	data3, data3
+#endif
+	sub	has_nul, data3, zeroones
+	orr	tmp, data3, REP8_7f
+	ldr	data1, [src1], 8
+	bics	has_nul, has_nul, tmp
+	ccmp	data1, data2, 0, eq
+	b.eq	L(loop_unaligned)
+
+	lsl	tmp, has_nul, shift
+#ifdef __AARCH64EB__
+	rev	tmp, tmp
+#endif
+	eor	diff, data1, data2
+	orr	syndrome, diff, tmp
+	cbnz	syndrome, L(end)
+L(tail):
+	ldr	data1, [src1]
+	neg	shift, shift
+	lsr	data2, data3, shift
+	lsr	has_nul, has_nul, shift
+#ifdef __AARCH64EB__
+	rev     data2, data2
+	rev	has_nul, has_nul
+#endif
+	eor	diff, data1, data2
 	orr	syndrome, diff, has_nul
-	cbz	syndrome, L(loop_misaligned)
 	b	L(end)
 
 L(done):