Skip to content

memmove speed optimized#13

Draft
m-fally wants to merge 5 commits into
mainfrom
memmove_speed_opt
Draft

memmove speed optimized#13
m-fally wants to merge 5 commits into
mainfrom
memmove_speed_opt

Conversation

@m-fally

@m-fally m-fally commented May 5, 2025

Copy link
Copy Markdown
Collaborator

Here is the current state of my optimization-suggestion for memmove().

I have used the common implementation of the function as a base, which first checks if at least one of the addresses is unaligned, and only uses an optimized algorithm if both addresses are aligned. Otherwhise it copies byte by byte.

I have added an unrolled loop for the destructive overlap case as well as increased the unroll-factor of the loop in the non-destructive overlap case. I chose an unroll factor of 9*SZREG since that is what is also used in memcpy(), and according to my tests it seems like a reasonable choice.

The other bigger change is aligning the source address when one of the addresses is unaligned. In that case, one word (or doubleword for rv64) is loaded at a time, and individual bytes are stored to the destination. For the vast majority of cases I get better or equally as good results when aligning the source than when aligning the destination.
This change also affects the case when source and destination are aligned, there is a destructive overlap and length is not a multiple of SZREG, because in this scenario the length is added to both source and destination, which causes them to become unaligned.

I have also tried always aligning the source and then checking whether source and destination have the same alignment. This performs better than checking for misalignment when at least one address is unaligned (especially when the addresses already have the same alignment). However, when the addresses are both aligned (or when -mno-strict-align is used) in a lot of cases this approach performs worse than just checking for misalignment of one of the addresses (in the case of -mno-strict-align performance is even slightly worse than in the original implementation).
Maybe it would be an idea to use the check for matching alignment only in the case of a destructive overlap, since having two aligned addresses and an "unaligned" length would produce the scenario where that approach performs well, and in the case of two aligned addresses and an aligned length it does not do too much worse than the other approach. But that would favour the case where length is not a multiple of SZREG, so I am not sure if it is good to do that.

The following tables compare the data for both approaches I have tried (checking for misalignment of at least one address and only aligning if one is misaligned vs. always aligning the source and then checking whether the alignments of source and destination match) and the original algorithm for rv32 and rv64:

memmove_speed_opt_comparison_rv32.ods
memmove_speed_opt_comparison_rv64.ods

With regards to incorporating zilsd I haven't had any success yet, since there are additional checks and alignment operations needed to be sure the addresses are aligned by 8 bytes.
I think I will try to also unroll the misaligned-copy-loop and see if in that case the extra overhead is worth it.

@m-fally m-fally marked this pull request as draft May 5, 2025 08:13
@m-fally m-fally changed the base branch from main to size_opt May 5, 2025 08:47
@m-fally m-fally changed the base branch from size_opt to main May 5, 2025 08:47
Comment thread newlib/libc/machine/riscv/memmove-stub.c
Comment thread newlib/libc/machine/riscv/memmove-stub.c Outdated
Comment thread newlib/libc/machine/riscv/memmove-stub.c Outdated
Comment thread newlib/libc/machine/riscv/memmove-stub.c Outdated
Comment thread newlib/libc/machine/riscv/memmove-stub.c Outdated
aligned_src = (uintxlen_t*)src;

/* Copy 9X SZREG bytes at a time if possible. */
while (length >= (SZREG*9)) {

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.

when i see such relatively long code segments that i have seen before, I will usually ask you to make an inline function.
Makes the code more readable and maintainable.
As a rule of thumb, if your function does not fit on one screen, you should probably structure it better

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 see that this is forward vs. backwards, but this could be an option to the compiler (similar to how the assembly version does it). I would strongly assume that the compiler then inlines two different version of the function.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

i'm not sure i understand what you mean by "option to the compiler" – do you mean to have a variable that contains either -1 or 1 depending on the kind of overlap, so then the same code can be used for both scenarios?

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'm not sure i understand what you mean by "option to the compiler" – do you mean to have a variable that contains either -1 or 1 depending on the kind of overlap, so then the same code can be used for both scenarios?

I formulated this weirdly but you got the point.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

just pushed a new suggestion for this

m-fally added 3 commits May 16, 2025 13:42
Copy the common implementation of memmove() to the RISC-V port.
Rename memmove.S to memmove-asm.S to keep naming of files
consistent between functions. Update Makefile.inc with the changed
filenames.
…types

Remove macros or replace them with static inline functions or RISC-V
specific macros where applicable.
Change data types to fixed-width and/or RISC-V specific types.
Add loop-unrolling for the case where both source and destination
address are aligned in the case of a destructive overlap, and
increase the unroll factor from four to nine for the word-by-word
copy loop in the non-destructive case.
This matches the loop-unrolling done in memcpy() and increases
performance for lenghts >= SZREG*9 while not degrading performance
for shorter lengths.
@m-fally m-fally force-pushed the memmove_speed_opt branch from e775909 to 31ed784 Compare May 16, 2025 11:45
@m-fally

m-fally commented May 16, 2025

Copy link
Copy Markdown
Collaborator Author

@christian-herber-nxp I've pushed an updated version of the implementation with the changes you suggested and tried to organize the data better/made some plots to visualize the differences between implementations:

For rv32ic -mstrict-align, there were 9 cases where the new version was slower than the original with a maximum difference of 10 retired instructions. For rv64ic -mstrict-align there was 1 case where the new version was slower with a difference of 2 retired instructions.
For rv32ic -mno-strict-align and rv64ic -mno-strict-align, the new version was faster or equally as fast as the original implementation in all test cases.
In the tables below, this new version is indicated as "Check for misalignment".

I have also tested a version based on the memcpy() algorithm, where I just added code to handle the destructive-overlap-case and made smaller adjustments so that some code can be reused in both overlap-scenarios (This implementation is indicated as "Check for matching alignment" in the table below).

All tests were run on spike. I have run tests with both source and destination address aligned, both addresses unaligned, source aligned and destination unaligned, and destination aligned and source unaligned.

Graphical plots for all scenarios tested can be found here: https://cloud.servus.at/s/a7Gqnoigxak7y4q


-march=rv32ic -mabi=ilp32 -mtune=thead-c906 -mstrict-align -O3

Number of Cases Highest difference in instructions retired
Check for misalignment was faster than check for matching alignment: 5152 -1541
Check for misalignment was slower than check for matching alignment: 3022 4321
Check for misalignment was equally as fast as check for matching alignment: 0
Check for misalignment was faster than original: 8162 -4583
Check for misalignment was slower than original: 9 10
Check for misalignment was equally as fast as original: 3
Check for matching alignment was faster than original: 8054 -8836
Check for matching alignment was slower than original: 115 115
Check for matching alignment was equally as fast as original: 5

Total number of tests: 8174


-march=rv32ic -mabi=ilp32 -mtune=thead-c906 -mno-strict-align -O3

Number of Cases Highest difference in instructions retired
Check for misalignment was faster than check for matching alignment: 8174 -176
Check for misalignment was slower than check for matching alignment: 0
Check for misalignment was equally as fast as check for matching alignment: 0
Check for misalignment was faster than original: 8171 -1298
Check for misalignment was slower than original: 0
Check for misalignment was equally as fast as original: 3
Check for matching alignment was faster than original: 7680 -1293
Check for matching alignment was slower than original: 492 142
Check for matching alignment was equally as fast as original: 2

Total number of tests: 8174


-march=rv64ic -mabi=lp64 -mtune=thead-c906 -mstrict-align -O3

Number of Cases Highest difference in instructions retired
Check for misalignment was faster than check for matching alignment: 4712 -1801
Check for misalignment was slower than check for matching alignment: 3453 4185
Check for misalignment was equally as fast as check for matching alignment: 1
Check for misalignment was faster than original: 8164 -5343
Check for misalignment was slower than original: 1 2
Check for misalignment was equally as fast as original: 1
Check for matching alignment was faster than original: 7926 -9452
Check for matching alignment was slower than original: 239 275
Check for matching alignment was equally as fast as original: 1

Total number of tests: 8166


-march=rv64ic -mabi=lp64 -mtune=thead-c906 -mno-strict-align -O3

Number of Cases Highest difference in instructions retired
Check for misalignment was faster than check for matching alignment: 8166 -360
Check for misalignment was slower than check for matching alignment: 0
Check for misalignment was equally as fast as check for matching alignment: 0
Check for misalignment was faster than original: 8151 -654
Check for misalignment was slower than original: 0
Check for misalignment was equally as fast as original: 15
Check for matching alignment was faster than original: 6058 -649
Check for matching alignment was slower than original: 2088 326
Check for matching alignment was equally as fast as original: 20

Total number of tests: 8166

@m-fally

m-fally commented May 16, 2025

Copy link
Copy Markdown
Collaborator Author

Since there are less cases where the "check for misalignment"-version is slower than the original memmove() implementation, I thought to stick with that one, but there are also cases where the "check for matching"-alignment version is a lot faster. So I was curious what you think?

@m-fally

m-fally commented May 16, 2025

Copy link
Copy Markdown
Collaborator Author

Forgot to add the information about code sizes, here it is:

Original Implementation

Code Size in Bytes
rv32ic -mstrict-align 326
rv32ic -mno-strict-align 300
rv64ic -mstrict-align 336
rv64ic -mno-strict-align 426

New Implementation (Check for misalignment)

Code Size in Bytes
rv32ic -mstrict-align 1082
rv32ic -mno-strict-align 580
rv64ic -mstrict-align 1370
rv64ic -mno-strict-align 692

New Implementation (Check for matching alignment)

Code Size in Bytes
rv32ic -mstrict-align 1082
rv32ic -mno-strict-align 614
rv64ic -mstrict-align 1284
rv64ic -mno-strict-align 676

m-fally added 2 commits May 17, 2025 10:27
If misaligned accesses are slow or prohibited, either
source or destination address are unaligned and the number
of bytes to be copied is > SZREG*2, align the source address
to xlen. This speeds up the function in the case where at
least one address is unaligned, since now one word (or doubleword
for rv64) is loaded at a time, therefore reducing the amount of
memory accesses necessary.
We still need to store back individual bytes since the
destination address might (still) be unaligned after aligning
the source.
The threshold of SZREG*2 was chosen because the additional
overhead caused by aligning the source would decrease performance
for shorter lengths.
Regenerate the configuration files since a stub-file was
replaced with a new implementation in the RISC-V port
(memmove.c), and one other file has been renamed
(memmove.S => memmove-asm.S).
@m-fally m-fally force-pushed the memmove_speed_opt branch from 31ed784 to dcc2e1d Compare May 17, 2025 08:56
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.

2 participants