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

Amortize sha2 compression loop #231

Merged
merged 10 commits into from
May 20, 2024
Merged

Amortize sha2 compression loop #231

merged 10 commits into from
May 20, 2024

Conversation

Nashtare
Copy link
Collaborator

Applies the tricks initially specified in the SHA1 specs for the macros sha2_choice and sha2_majority, as well as reducing u32 addition chain overhead for already reduced inputs.

Yields ~4.6% savings for SHA2 precompile.

@github-actions github-actions bot added the crate: evm_arithmetization Anything related to the evm_arithmetization crate. label May 16, 2024
Copy link
Contributor

@muursh muursh left a comment

Choose a reason for hiding this comment

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

Looks fine to me. I went through the logic on paper and it seems sound but it is 3am so...

@Nashtare
Copy link
Collaborator Author

Nashtare commented May 17, 2024

Commit 45fa5a7 shaves off an additional 1.95%.

Commit af81235 shaves off an additional 9.55%.

%stack (shifted, rot, value) -> (rot, value, shifted)
// stack: value >> rot, value
SWAP1
PUSH $rot
Copy link
Collaborator Author

@Nashtare Nashtare May 17, 2024

Choose a reason for hiding this comment

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

Yes I'm pushing again instead of calling DUP because $rot always fits in 4 bytes at most anyway, and the savings on the # CPU cycles are worth the extra overhead on BytePackingStark.

@Nashtare
Copy link
Collaborator Author

Nashtare commented May 17, 2024

@muursh You may want to review the latest changes, I've pushed some additional optimizations after your review.
Total savings are now at 16.1% 18.3%.

@muursh
Copy link
Contributor

muursh commented May 17, 2024

@muursh You may want to review the latest changes, I've pushed some additional optimizations after your review. Total savings are now at 16.1%.

Will do this evening

@@ -27,6 +27,6 @@
// stack: c, a, b, Sigma_0(a)
%sha2_majority
// stack: Maj(c, a, b), Sigma_0(a)
%add_u32
ADD
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is no need for 32-bit reduction, as the result of this macro is being reduced after (see compression section). Similar reasoning for %sha2_temp_word1.

Copy link
Contributor

@LindaGuiga LindaGuiga left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

Nashtare and others added 2 commits May 20, 2024 19:47
Co-authored-by: Linda Guiga <[email protected]>
Co-authored-by: Linda Guiga <[email protected]>
@Nashtare Nashtare enabled auto-merge (squash) May 20, 2024 10:47
@Nashtare Nashtare merged commit fa632ef into develop May 20, 2024
6 checks passed
@Nashtare Nashtare deleted the sha2 branch May 20, 2024 11:01
Nashtare added a commit that referenced this pull request May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate: evm_arithmetization Anything related to the evm_arithmetization crate.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants