Skip to content

strcmp optimization#6

Open
puranikvinit wants to merge 4 commits into
mainfrom
strcmp-opt
Open

strcmp optimization#6
puranikvinit wants to merge 4 commits into
mainfrom
strcmp-opt

Conversation

@puranikvinit

@puranikvinit puranikvinit commented Mar 24, 2025

Copy link
Copy Markdown
Owner
  1. Use only compressed registers across the function.
  2. Use Zbb extension:
    • use the orc.b instruction for the Hacker's Delight algorithm used to check for the presence of a null byte in a word.
    • use the rev8 instruction for reversing the byte order in case of endianness being little endian.

testsuite/newlib.string/strcmp-1.c was compiled and run successfully on spike with pk, indicating no issues with the functionality.

Size of the size-optimized version could not be reduced, as it was pretty simple.
Size of perf-optimized version reduced from 278b to 174b.

I am currently working on getting an approximate count on the number of retired instructions, but I believe it must be lesser at the end for almost all average cases due to the reduction in the number of instructions itself.

@puranikvinit puranikvinit self-assigned this Mar 24, 2025
Comment thread newlib/libc/machine/riscv/strcmp.S Outdated
sub a0, a4, a5
ret
rev8 a2, a2
rev8 a3, a3

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Had a quick question for this change:

  1. Do I retain this change? Because it adds up an overhead of 2 instructions every time .Lmismatch is executed compared to the already existing code, but it is a significant size reduction.
  2. If I should retain the change, should I reverse byte order for little endian or big endian?

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 like the change, but overall, it does not improve speed or code size. So it is better to keep both versions.

Comment thread newlib/libc/machine/riscv/strcmp.S Outdated
or t0, t0, t1

bne t0, t2, .Lnull\i
orc.b a4, a2

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.

the file still needs to compile for targets without Zbb. Check out e.g. in Erics patch how you can guard this.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Oh yes, sorry I missed that...

.macro check_one_word i n
REG_L a2, \i*SZREG(a0)

and t0, a2, a5

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.

if this was a C function, you could simply call __libc_detect_null(). Eric has already optimized this for Zbb in his patch.
I do not see anything in this function that would not work in C. Maybe this is a candidate to move from asm to C

@christian-herber-nxp

Copy link
Copy Markdown
Collaborator

I could imagine that you will find a faster algorithm for .Lmismatch using bitmanip.
E.g. if you xor the two values, all matching bytes will be all ones. If you invert, matching bytes will be all zero. You can use xnor operation for that.
Then you just count the leading zeros, and you will get the number of bits till the first mismatch. From that, calculate the byte position.

@christian-herber-nxp

christian-herber-nxp commented Mar 24, 2025

Copy link
Copy Markdown
Collaborator

I could imagine that you will find a faster algorithm for .Lmismatch using bitmanip.
E.g. if you xor the two values, all matching bytes will be all ones. If you invert, matching bytes will be all zero. You can use xnor operation for that.
Then you just count the leading zeros, and you will get the number of bits till the first mismatch. From that, calculate the byte position.

I realized that you could even use sub instead of xnor, and then you are back to finding a NULL byte
Edit: Actually not quite, as the byte you want to detect is explicitly not the Null byte.

@thebigclub

Copy link
Copy Markdown
Collaborator

@puranikvinit If you check out the stropt_v5 branch and open newlib/libc/machine/riscv/sys/string.h, you'll see examples on using the intrinsic functions and preprocessor guard macros.

Comment thread newlib/libc/machine/riscv/strcmp.S
@puranikvinit

Copy link
Copy Markdown
Owner Author

I could imagine that you will find a faster algorithm for .Lmismatch using bitmanip. E.g. if you xor the two values, all matching bytes will be all ones. If you invert, matching bytes will be all zero. You can use xnor operation for that. Then you just count the leading zeros, and you will get the number of bits till the first mismatch. From that, calculate the byte position.

I tried this, works perfectly right with the Zbb extension. However for the fallback logic for targets without Zbb, we will have to implement the logic of counting the number of trailing zeroes or leading zeroes (based on the endianness of the target), which will consume a lot of instructions if we attempt to optimize it with a lookup table. And given the fact that there are already intrinsic functions for doing exactly this, I believe I will start looking in the direction of migrating to a simpler C implementation... Please correct me here if I am wrong

@christian-herber-nxp

Copy link
Copy Markdown
Collaborator

I tried this, works perfectly right with the Zbb extension. However for the fallback logic for targets without Zbb, we will have to implement the logic of counting the number of trailing zeroes or leading zeroes (based on the endianness of the target), which will consume a lot of instructions if we attempt to optimize it with a lookup table. And given the fact that there are already intrinsic functions for doing exactly this, I believe I will start looking in the direction of migrating to a simpler C implementation... Please correct me here if I am wrong

There is no need to change the algorithm for the non Zbb version.

@puranikvinit

Copy link
Copy Markdown
Owner Author

I tried this, works perfectly right with the Zbb extension. However for the fallback logic for targets without Zbb, we will have to implement the logic of counting the number of trailing zeroes or leading zeroes (based on the endianness of the target), which will consume a lot of instructions if we attempt to optimize it with a lookup table. And given the fact that there are already intrinsic functions for doing exactly this, I believe I will start looking in the direction of migrating to a simpler C implementation... Please correct me here if I am wrong

There is no need to change the algorithm for the non Zbb version.

Wouldn't just having the same algorithm for both versions ease up things for maintainability and algorithm efficiency in general? Do you want me to try and get improvements comparing both the algorithms for the non Zbb version?

@christian-herber-nxp

Copy link
Copy Markdown
Collaborator

Wouldn't just having the same algorithm for both versions ease up things for maintainability and algorithm efficiency in general? Do you want me to try and get improvements comparing both the algorithms for the non Zbb version?

i cannot judge right now how different it would be. But it clearly does not make sense to count leading zeros if you do not have an instruction for that, so you will have to have both versions if you want the bitmanip acceleration

@puranikvinit

puranikvinit commented Mar 30, 2025

Copy link
Copy Markdown
Owner Author

Here are some metrics I observed for the test cases provided in the testsuite for strcmp(src, dest):

TEST - 1:
src -> (1100 0001) A B C \0
dest -> (0100 0001) A B C \0

Cycle count with Zbb support on target - 29 cycles
Cycle count without Zbb support on target - 31 cycles


TEST - 2:
src -> (0000 0001) A B C \0
dest -> (1000 0010) A B C \0

Cycle count with Zbb support on target - 29 cycles
Cycle count without Zbb support on target - 31 cycles


TEST - 3:
src -> D A B (1100 0001) \0
dest -> D A B (0100 0001) \0

Cycle count with Zbb support on target - 29 cycles
Cycle count without Zbb support on target - 49 cycles


TEST - 4:
src -> D A B (0000 0001) \0
dest -> D A B (1000 0010) \0

Cycle count with Zbb support on target - 29 cycles
Cycle count without Zbb support on target - 49 cycles


TEST - 5 -> Random data filled in src and dest, with MAX_BLOCK_SIZE of 64:

Cycle count with Zbb support on target - 144 cycles
Cycle count without Zbb support on target - 413 cycles
(Above data is an average of various runs)

@puranikvinit

puranikvinit commented Mar 30, 2025

Copy link
Copy Markdown
Owner Author

Code Size comparison:
with Zbb -> 198 bytes
wihtout Zbb -> 308 bytes

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.

4 participants