mbox series

[0/3] riscv: optimize memcpy/memmove/memset

Message ID 20240128111013.2450-1-jszhang@kernel.org (mailing list archive)
Headers show
Series riscv: optimize memcpy/memmove/memset | expand

Message

Jisheng Zhang Jan. 28, 2024, 11:10 a.m. UTC
This series is to renew Matteo's "riscv: optimized mem* functions"
sereies.

Compared with Matteo's original series, Jisheng made below changes:
1. adopt Emil's change to fix boot failure when build with clang
2. add corresponding changes to purgatory
3. always build optimized string.c rather than only build when optimize
for performance
4. implement unroll support when src & dst are both aligned to keep
the same performance as assembly version. After disassembling, I found
that the unroll version looks something like below, so it acchieves
the "unroll" effect as asm version but in C programming language:
	ld	t2,0(a5)
	ld	t0,8(a5)
	ld	t6,16(a5)
	ld	t5,24(a5)
	ld	t4,32(a5)
	ld	t3,40(a5)
	ld	t1,48(a5)
	ld	a1,56(a5)
	sd	t2,0(a6)
	sd	t0,8(a6)
	sd	t6,16(a6)
	sd	t5,24(a6)
	sd	t4,32(a6)
	sd	t3,40(a6)
	sd	t1,48(a6)
	sd	a1,56(a6)
And per my testing, unrolling more doesn't help performance, so
the "c" version only unrolls by using 8 GP regs rather than 16
ones as asm version.
5. Add proper __pi_memcpy and __pi___memcpy alias
6. more performance numbers.

Per my benchmark with [1] on TH1520, CV1800B and JH7110 platforms,
the unaligned medium memcpy performance is running about 3.5x ~ 8.6x
speed of the unpatched versions's! Check patch1 for more details and
performance numbers.

Link:https://github.com/ARM-software/optimized-routines/blob/master/string/bench/memcpy.c [1]

Here is the original cover letter msg from Matteo:
Replace the assembly mem{cpy,move,set} with C equivalent.

Try to access RAM with the largest bit width possible, but without
doing unaligned accesses.

A further improvement could be to use multiple read and writes as the
assembly version was trying to do.

Tested on a BeagleV Starlight with a SiFive U74 core, where the
improvement is noticeable.


Matteo Croce (3):
  riscv: optimized memcpy
  riscv: optimized memmove
  riscv: optimized memset

 arch/riscv/include/asm/string.h |  14 +-
 arch/riscv/kernel/riscv_ksyms.c |   6 -
 arch/riscv/lib/Makefile         |   9 +-
 arch/riscv/lib/memcpy.S         | 110 -----------
 arch/riscv/lib/memmove.S        | 317 --------------------------------
 arch/riscv/lib/memset.S         | 113 ------------
 arch/riscv/lib/string.c         | 187 +++++++++++++++++++
 arch/riscv/purgatory/Makefile   |  13 +-
 8 files changed, 206 insertions(+), 563 deletions(-)
 delete mode 100644 arch/riscv/lib/memcpy.S
 delete mode 100644 arch/riscv/lib/memmove.S
 delete mode 100644 arch/riscv/lib/memset.S
 create mode 100644 arch/riscv/lib/string.c

Comments

Conor Dooley Jan. 29, 2024, 6:16 p.m. UTC | #1
On Sun, Jan 28, 2024 at 07:10:10PM +0800, Jisheng Zhang wrote:
> This series is to renew Matteo's "riscv: optimized mem* functions"
> sereies.
> 
> Compared with Matteo's original series, Jisheng made below changes:
> 1. adopt Emil's change to fix boot failure when build with clang
> 2. add corresponding changes to purgatory
> 3. always build optimized string.c rather than only build when optimize
> for performance
> 4. implement unroll support when src & dst are both aligned to keep
> the same performance as assembly version. After disassembling, I found
> that the unroll version looks something like below, so it acchieves
> the "unroll" effect as asm version but in C programming language:
> 	ld	t2,0(a5)
> 	ld	t0,8(a5)
> 	ld	t6,16(a5)
> 	ld	t5,24(a5)
> 	ld	t4,32(a5)
> 	ld	t3,40(a5)
> 	ld	t1,48(a5)
> 	ld	a1,56(a5)
> 	sd	t2,0(a6)
> 	sd	t0,8(a6)
> 	sd	t6,16(a6)
> 	sd	t5,24(a6)
> 	sd	t4,32(a6)
> 	sd	t3,40(a6)
> 	sd	t1,48(a6)
> 	sd	a1,56(a6)
> And per my testing, unrolling more doesn't help performance, so
> the "c" version only unrolls by using 8 GP regs rather than 16
> ones as asm version.
> 5. Add proper __pi_memcpy and __pi___memcpy alias
> 6. more performance numbers.
> 
> Per my benchmark with [1] on TH1520, CV1800B and JH7110 platforms,
> the unaligned medium memcpy performance is running about 3.5x ~ 8.6x
> speed of the unpatched versions's! Check patch1 for more details and
> performance numbers.
> 
> Link:https://github.com/ARM-software/optimized-routines/blob/master/string/bench/memcpy.c [1]
> 
> Here is the original cover letter msg from Matteo:
> Replace the assembly mem{cpy,move,set} with C equivalent.
> 
> Try to access RAM with the largest bit width possible, but without
> doing unaligned accesses.
> 
> A further improvement could be to use multiple read and writes as the
> assembly version was trying to do.
> 
> Tested on a BeagleV Starlight with a SiFive U74 core, where the
> improvement is noticeable.

However, with allmodconfig it doesn't compile:
  Redirect to /build/tmp.zzMIlhgQQo and /build/tmp.vxnoxu8G5e
  Tree base:
  0c526539d432 ("riscv: optimized memcpy")
  Building the whole tree with the patch
  ../arch/riscv/lib/string.c:118:7: error: expected identifier or '('
  ../arch/riscv/lib/string.c:118:7: error: expected ')'
  ../arch/riscv/lib/string.c:143:7: error: expected identifier or '('
  ../arch/riscv/lib/string.c:143:7: error: expected ')'
  ../arch/riscv/lib/string.c:118:7: error: expected identifier or '('
  ../arch/riscv/lib/string.c:118:7: error: expected ')'
  ../arch/riscv/lib/string.c:143:7: error: expected identifier or '('
  ../arch/riscv/lib/string.c:143:7: error: expected ')'

Seems to be the case both with llvm and gcc.

Cheers,
Conor.
Jisheng Zhang Jan. 30, 2024, 2:28 a.m. UTC | #2
On Mon, Jan 29, 2024 at 06:16:13PM +0000, Conor Dooley wrote:
> On Sun, Jan 28, 2024 at 07:10:10PM +0800, Jisheng Zhang wrote:
> > This series is to renew Matteo's "riscv: optimized mem* functions"
> > sereies.
> > 
> > Compared with Matteo's original series, Jisheng made below changes:
> > 1. adopt Emil's change to fix boot failure when build with clang
> > 2. add corresponding changes to purgatory
> > 3. always build optimized string.c rather than only build when optimize
> > for performance
> > 4. implement unroll support when src & dst are both aligned to keep
> > the same performance as assembly version. After disassembling, I found
> > that the unroll version looks something like below, so it acchieves
> > the "unroll" effect as asm version but in C programming language:
> > 	ld	t2,0(a5)
> > 	ld	t0,8(a5)
> > 	ld	t6,16(a5)
> > 	ld	t5,24(a5)
> > 	ld	t4,32(a5)
> > 	ld	t3,40(a5)
> > 	ld	t1,48(a5)
> > 	ld	a1,56(a5)
> > 	sd	t2,0(a6)
> > 	sd	t0,8(a6)
> > 	sd	t6,16(a6)
> > 	sd	t5,24(a6)
> > 	sd	t4,32(a6)
> > 	sd	t3,40(a6)
> > 	sd	t1,48(a6)
> > 	sd	a1,56(a6)
> > And per my testing, unrolling more doesn't help performance, so
> > the "c" version only unrolls by using 8 GP regs rather than 16
> > ones as asm version.
> > 5. Add proper __pi_memcpy and __pi___memcpy alias
> > 6. more performance numbers.
> > 
> > Per my benchmark with [1] on TH1520, CV1800B and JH7110 platforms,
> > the unaligned medium memcpy performance is running about 3.5x ~ 8.6x
> > speed of the unpatched versions's! Check patch1 for more details and
> > performance numbers.
> > 
> > Link:https://github.com/ARM-software/optimized-routines/blob/master/string/bench/memcpy.c [1]
> > 
> > Here is the original cover letter msg from Matteo:
> > Replace the assembly mem{cpy,move,set} with C equivalent.
> > 
> > Try to access RAM with the largest bit width possible, but without
> > doing unaligned accesses.
> > 
> > A further improvement could be to use multiple read and writes as the
> > assembly version was trying to do.
> > 
> > Tested on a BeagleV Starlight with a SiFive U74 core, where the
> > improvement is noticeable.
> 
> However, with allmodconfig it doesn't compile:
>   Redirect to /build/tmp.zzMIlhgQQo and /build/tmp.vxnoxu8G5e
>   Tree base:
>   0c526539d432 ("riscv: optimized memcpy")
>   Building the whole tree with the patch
>   ../arch/riscv/lib/string.c:118:7: error: expected identifier or '('
>   ../arch/riscv/lib/string.c:118:7: error: expected ')'
>   ../arch/riscv/lib/string.c:143:7: error: expected identifier or '('
>   ../arch/riscv/lib/string.c:143:7: error: expected ')'
>   ../arch/riscv/lib/string.c:118:7: error: expected identifier or '('
>   ../arch/riscv/lib/string.c:118:7: error: expected ')'
>   ../arch/riscv/lib/string.c:143:7: error: expected identifier or '('
>   ../arch/riscv/lib/string.c:143:7: error: expected ')'
> 
> Seems to be the case both with llvm and gcc.

Hi Conor,

This is due to missing proper FORTIFY_SOURCE handling.
Below trival patch can fix it :)
I'm waiting for more comments before sending out v2.

diff --git a/arch/riscv/lib/string.c b/arch/riscv/lib/string.c
index 022edda68f1c..bfaab058f2cb 100644
--- a/arch/riscv/lib/string.c
+++ b/arch/riscv/lib/string.c
@@ -6,6 +6,7 @@
  * Copyright (C) 2021 Matteo Croce
  */

+#define __NO_FORTIFY
 #include <linux/types.h>
 #include <linux/module.h>
> 
> Cheers,
> Conor.
>