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.

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

sha2: add read_volatile workaround for round constants #547

Merged
merged 7 commits into from Jan 12, 2024

Conversation

newpavlov
Copy link
Member

@newpavlov newpavlov commented Jan 12, 2024

Prevents compiler from inlining round constants or spilling them to stack, which can slightly improve performance.

For example, on my x86-64 laptop:

// Before:
test sha512_10    ... bench:          23 ns/iter (+/- 0) = 434 MB/s
test sha512_100   ... bench:         208 ns/iter (+/- 1) = 480 MB/s
test sha512_1000  ... bench:       2,015 ns/iter (+/- 7) = 496 MB/s
test sha512_10000 ... bench:      20,047 ns/iter (+/- 862) = 498 MB/s

// After:
test sha512_10    ... bench:          22 ns/iter (+/- 0) = 454 MB/s
test sha512_100   ... bench:         201 ns/iter (+/- 0) = 497 MB/s
test sha512_1000  ... bench:       1,941 ns/iter (+/- 3) = 515 MB/s
test sha512_10000 ... bench:      19,298 ns/iter (+/- 64) = 518 MB/s

Closes #328

@newpavlov
Copy link
Member Author

@piegamesde
Can you check that this PR indeed helps with performance issues on RISC-V?

@piegamesde
Copy link

Hi, and thank you for the fix. However, I cannot test it as I have quite working on embedded systems over a year ago, and the setup was for a work project.

@tarcieri
Copy link
Member

or spilling them to stack

Interesting. Wonder if we could use that approach in the aes crate as well?

@newpavlov
Copy link
Member Author

Wonder if we could use that approach in the aes crate as well?

It will be a bit harder to do. If there is enough registers, we want keys to stay in them, while read_volatile would always read them. In the sha2 case we know for certain that there is not enough registers, even on register rich targets like RISC-V, so reading round constants on every block processing is fine.

@newpavlov newpavlov merged commit 3020704 into master Jan 12, 2024
22 checks passed
@newpavlov newpavlov deleted the sha2_round_const branch January 12, 2024 14:03
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.

sha2: performance issue on RISC-V
3 participants