Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

strcmp optimization for riscv #20

Merged
merged 3 commits into from
Aug 18, 2024
Merged

strcmp optimization for riscv #20

merged 3 commits into from
Aug 18, 2024

Conversation

M-Moawad
Copy link
Collaborator

Optimize strcmp by calling orc.b (OR-Combine) instruction when Zbb extension is supported instead of 4 instructions to detect if zero byte is exist in word

keith-packard
keith-packard reviewed Jul 21, 2024
View reviewed changes
newlib/libc/machine/riscv/strcmp.S Outdated
@@ -46,12 +46,14 @@ strcmp:
.macro check_one_word i n
REG_L a2, \i*SZREG(a0)
REG_L a3, \i*SZREG(a1)

#ifdef __riscv_zbb
orc.b t2, t0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Reading the adjacent code, it seems like this should be

    orc.b t0, a2

that is, t0 should contain 0xff in each non-zero byte of a2.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Edited on the same commit

@keith-packard
Copy link
Collaborator

I also hacked up one of the CI configurations to enable Zbb for testing:

diff --git a/scripts/cross-rv32imac_zicsr.txt b/scripts/cross-rv32imac_zicsr.txt
index ef1b2751ee..6e12ac283d 100644
--- a/scripts/cross-rv32imac_zicsr.txt
+++ b/scripts/cross-rv32imac_zicsr.txt
@@ -14,8 +14,8 @@ cpu = 'riscv32'
 endian = 'little'
 
 [properties]
-c_args = ['-msave-restore', '-fshort-enums', '-march=rv32imac_zicsr', '-mabi=ilp32']
-c_link_args = ['-msave-restore', '-fshort-enums', '-march=rv32imac', '-mabi=ilp32']
+c_args = ['-msave-restore', '-fshort-enums', '-march=rv32imac_zicsr_zbb', '-mabi=ilp32']
+c_link_args = ['-msave-restore', '-fshort-enums', '-march=rv32imac_zicsr_zbb', '-mabi=ilp32']
 skip_sanity_check = true
 default_flash_addr = '0x80000000'
 default_flash_size = '0x00200000'

@keith-packard
Copy link
Collaborator

It also looks like the mask value stored in 'a5' is not needed any more, so we can remove initializing that register and the related 'mask' value at the bottom of the file.

@M-Moawad M-Moawad force-pushed the moawad-optimize-strcmp-for-riscv branch from ef3347f to 09c010f Compare July 22, 2024 07:17
@M-Moawad
Copy link
Collaborator Author

It also looks like the mask value stored in 'a5' is not needed any more, so we can remove initializing that register and the related 'mask' value at the bottom of the file.

Check the current change, I moved the value assigned to a5 to be used when zbb extension is disabled, also the related mask at the bottom of the file will be stored only in this case.

@M-Moawad M-Moawad mentioned this pull request Jul 22, 2024
Merged
@M-Moawad M-Moawad force-pushed the moawad-optimize-strcmp-for-riscv branch 2 times, most recently from 528915c to 6a7ca3d Compare July 23, 2024 07:48
@arichardson
semihosting: reduce code duplication for calls with indirect arguments
1098984 
These functions make it easier to call the semihosting functions without
having to declare a local struct in each caller.
Since there is only one call with 4 indirect parameters, I did not add
a sys_semihost4 wrapper.
@M-Moawad M-Moawad force-pushed the moawad-optimize-strcmp-for-riscv branch from 18d3e29 to c415d89 Compare July 30, 2024 07:57
@keith-packard keith-packard force-pushed the moawad-optimize-strcmp-for-riscv branch from c415d89 to 23892e4 Compare July 30, 2024 17:58
@keith-packard
Copy link
Collaborator

I updated your last commit to add _zbb to do-rv32imac-configure which is what CI uses to run that test.

@M-Moawad @keith-packard
strcmp optimization for riscv
e71c602 
Optimize strcmp by calling orc.b (OR-Combine) instruction when Zbb extension
is supported instead of 4 instructions to detect if zero byte is exist in word

Signed-off-by: Mohamed Moawad <moawad@synopsys.com>
@M-Moawad @keith-packard
Enable zbb extension in CI configuration
61560a1 
Enable zbb extension in CI for RISC-V to test optimizations using bit manipulation instructions like `orc.b`. 
This ensures that the strcmp optimization and other features using zbb are tested and validated

Signed-off-by: Mohamed Moawad <moawad@synopsys.com>
@keith-packard keith-packard force-pushed the moawad-optimize-strcmp-for-riscv branch from 23892e4 to 61560a1 Compare July 30, 2024 19:36
@mostafa-salmaan mostafa-salmaan merged commit 61560a1 into main Aug 18, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@keith-packard keith-packard keith-packard left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@M-Moawad @keith-packard @mostafa-salmaan @arichardson