Skip to content

memcpy optimization#4

Open
i3abghany wants to merge 3 commits into
mainfrom
memcpy-enhance
Open

memcpy optimization#4
i3abghany wants to merge 3 commits into
mainfrom
memcpy-enhance

Conversation

@i3abghany

@i3abghany i3abghany commented Mar 19, 2025

Copy link
Copy Markdown
Collaborator

This new version is based on the previous one but has two main
differences:

  • Instead of copying the remainder of 64 bytes one byte at a time, the
    new implementation copies at most one 32-byte block before copying one
    byte at a time.
  • In the case where __riscv_misaligned_avoid is defined, the C
    implementation copies one byte at a time if the src and dest
    alignments differ. The new implementation aligns dest to SZREG, does
    SZREG byte-sized loads, packs them in one register, and uses one
    aligned SZREG-sized store.

The new implementation is 18% smaller than the C version compiled with
-O3 with GCC 14.2.

In terms of performance, gem5 was used to emulate a small, in-order core
to exercise all possible pairs of src & dest alignments, and different
copy sizes. Percentages are the average reduction in execution (number
of cycles). Higher is better.

Average over all alignment pairs:

  • -mtune=thead-c906 -mno-strict-align -O3: 2%
  • -mtune=thead-c906 -mstrict-align -O3: 13%

Average over different sizes (0, 1, 2, 3, 4, 8, 16, 32, 64, 128, 200,
256, 300, 400, 512, 700, 800, 1000, 1024, 2048, 4096, 8192 16384):

  • -mtune=thead-c906 -mno-strict-align -O3: 3%
  • -mtune=thead-c906 -mstrict-align -O3: 7%

@christian-herber-nxp

Copy link
Copy Markdown
Collaborator

your axes would benefit a lot from labels :D.
We need to dig into futher improving the performance. If you are just matching the C performance, it is better to stay with the C version in my opinion.
What flags did you compile the C version with?
Maybe there are some tricks to learn from the C version which could improve the performance at the cost of code size

Comment thread newlib/libc/machine/riscv/memcpy-asm.S Outdated
Comment thread newlib/libc/machine/riscv/memcpy-asm.S Outdated
Comment thread newlib/libc/machine/riscv/memcpy-asm.S Outdated
Comment thread newlib/libc/machine/riscv/memcpy-asm.S Outdated
Comment thread newlib/libc/machine/riscv/memcpy-asm.S Outdated
Comment thread newlib/libc/machine/riscv/memcpy-asm.S Outdated
Comment thread newlib/libc/machine/riscv/memcpy-asm.S Outdated
Comment thread newlib/libc/machine/riscv/memcpy-asm.S Outdated
Comment thread newlib/libc/machine/riscv/memcpy-asm.S Outdated
Comment thread newlib/libc/machine/riscv/memcpy-asm.S Outdated
Comment thread newlib/libc/machine/riscv/memcpy-asm.S Outdated
Comment thread newlib/libc/machine/riscv/memcpy-asm.S Outdated
@christian-herber-nxp

Copy link
Copy Markdown
Collaborator

as the change on the size optimized version is pretty simple, lets have separate PRs on the two.
In fact, when submitting upstream, we should combine all of these changes into one patch.
I think there is enough room for improvement in the assembly version that we will make this faster than the C version

@christian-herber-nxp

Copy link
Copy Markdown
Collaborator

I believe we need to discuss the algorithm in a more general sense.
You have to deal with a number of different scenarios including

  • one of src/dst is not XLEN aligned
  • neither is XLEN aligned, and they have different alignment
  • neither is XLEN aligned, but they have the same alignment
  • both of src/dst are XLEN aligned

When in doubt, you should always prioritize the getting alignment of the src. The is because you actually depend on the result of load, whereas the store can complete in the background.

You will want a main work loop, in which you unroll as much as the registers permit you to. This will use ld/sd for RV64 and RV32-Zilsd, and lw/sw for RV32.

In order to generate alignment, you optimally use a sequence of lb/sb, lh/sh, and optionally lw/sw.
In you notion, these procedures would be .Lcopy1 - .Lcopy6. Which one you need depends on the lsbs of src.
You could use the lsbs of src to index a table from, which you load the address of .Lcopyx and jump to that.

After that, you run the main work loop.
You will need to add a remainder loop to do the remaining possible max sized loads/stores.
When done with this, you may have some remainder to clean up. based on the remaining bytes, you can again use something like .Lcopy1 - .Lcopy6.

This should give you the optimal sequence of loads/store with the least overhead.

@i3abghany

i3abghany commented Mar 19, 2025

Copy link
Copy Markdown
Collaborator Author

Thank you for the great and detailed feedback! I will go through it hopefully today.

One more note to myself: I am modifying the original dest (a0) so the return value is wrong.

Comment thread newlib/libc/machine/riscv/memcpy-asm.S Outdated
Comment thread newlib/libc/machine/riscv/memcpy-asm.S Outdated
@christian-herber-nxp

Copy link
Copy Markdown
Collaborator

@i3abghany in your statements on performance, I am currently missing details on how you did the build. E.g. for C, which flags did you enable. Did you use -mno-strict-align?

Comment thread newlib/libc/machine/riscv/memcpy.S
#else
mv a3, a0
li a4, SZREG
addi sp, sp, -2 * SZREG

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there should be an option to use Zcmp

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually this is debatable, as this would include the ra register.
would an additional register be of help anywhere?
Otherwise, only opportunity is use of Zilsd for RV32

Comment thread newlib/libc/machine/riscv/memcpy.S
Comment thread newlib/libc/machine/riscv/memcpy.S
Comment thread newlib/libc/machine/riscv/memcpy.S Outdated
Comment thread newlib/libc/machine/riscv/memcpy.S Outdated
Comment thread newlib/libc/machine/riscv/memcpy.S Outdated
Comment thread newlib/libc/machine/riscv/memcpy.S Outdated
Comment thread newlib/libc/machine/riscv/memcpy.S Outdated
Comment thread newlib/libc/machine/riscv/memcpy.S Outdated
@i3abghany i3abghany force-pushed the memcpy-enhance branch 2 times, most recently from 6db0086 to f1552ad Compare April 6, 2025 05:56
This new version is based on the previous one but has two main
differences:

In the following, BLOCK size is 16 * SZREG

- Instead of copying the remainder, BLOCK-misaligned bytes one byte at a
  time, the new implementation copies at most one half BLOCK, one SZREG
  at a time before copying one byte at a time.
- In the case where __riscv_misaligned_avoid is defined, the C
  implementation copies one byte at a time if the src and dest
  alignments differ. The new implementation aligns dest to SZREG, does
  SZREG byte-sized loads, packs them in one register, and uses one
  aligned SZREG-sized store.

The new implementation is 14% smaller than the C version compiled with
-O3 with GCC 14.2.

In terms of performance, gem5 was used to emulate a small, in-order core
to exercise all possible pairs of src & dest alignments, and different
copy sizes. Percentages are the average reduction in execution (number
of cycles). Higher is better.

Average over all alignment pairs:
- `-mtune=thead-c906 -mno-strict-align -O3`: 2%
- `-mtune=thead-c906 -mstrict-align -O3`: 13%

Average over different sizes (0, 1, 2, 3, 4, 8, 16, 32, 64, 128, 200,
256, 300, 400, 512, 700, 800, 1000, 1024, 2048, 4096, 8192 16384):
- `-mtune=thead-c906 -mno-strict-align -O3`: 3%
- `-mtune=thead-c906 -mstrict-align -O3`: 7%

#if __riscv_xlen == 32
/* 32-bit + zbkb */
#define PACK4(r1, r2, r3, r4) \

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as this is assembly, use a .marco/.endm definition rather than a C macro.

lbu s1, 7(a1)

PACK4(t0, a5, s0, s1)
pack a4, a4, t0

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would move this inside the macro.
Also, I believe splat is the more fitting name.
llvm libc has a similar library function:
https://github.com/llvm/llvm-project/blob/be6ccc98f38227db02164f17bfaf0ac86d800e4a/libc/src/string/memory_utils/op_generic.h#L129

PACK4(a4, a5, s0, s1)
#if __riscv_xlen == 64

lbu t0, 4(a1)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you need to use a different register for rv64 over rv32?

REG_S t6, 10 * SZREG(a3)
#endif

#ifdef __riscv_e

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the performance difference between rv32e and rv32i?
It should be significant to justify this

@christian-herber-nxp

Copy link
Copy Markdown
Collaborator

Average over all alignment pairs:

  • -mtune=thead-c906 -mno-strict-align -O3: 2%
  • -mtune=thead-c906 -mstrict-align -O3: 13%

Average over different sizes (0, 1, 2, 3, 4, 8, 16, 32, 64, 128, 200, 256, 300, 400, 512, 700, 800, 1000, 1024, 2048, 4096, 8192 16384):

  • -mtune=thead-c906 -mno-strict-align -O3: 3%
  • -mtune=thead-c906 -mstrict-align -O3: 7%

I would also like to see the numbers on spike, i.e. difference in instret (you can of course take this out of gem5 also).
Also, please mention the isa string used here. Include results for rv32e/rv32i/rv64i.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants