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

Add blake2s implementation for SIMD backend #601

Merged
merged 3 commits into from
May 15, 2024

Conversation

andrewmilson
Copy link
Contributor

@andrewmilson andrewmilson commented May 2, 2024

This change is Reviewable

@codecov-commenter
Copy link

codecov-commenter commented May 2, 2024

Codecov Report

Attention: Patch coverage is 80.64516% with 60 lines in your changes are missing coverage. Please review.

Project coverage is 91.02%. Comparing base (24935cf) to head (9f342e8).

Files Patch % Lines
crates/prover/src/core/backend/simd/blake2s.rs 80.64% 60 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #601      +/-   ##
==========================================
- Coverage   91.36%   91.02%   -0.34%     
==========================================
  Files          76       77       +1     
  Lines        9497     9807     +310     
  Branches     9497     9807     +310     
==========================================
+ Hits         8677     8927     +250     
- Misses        751      811      +60     
  Partials       69       69              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@andrewmilson andrewmilson force-pushed the 04-30-Add_bit_reverse_for_SIMD_backend branch from e04a2f6 to 9ccaf1f Compare May 2, 2024 16:29
@andrewmilson andrewmilson force-pushed the 05-02-Add_blake2s_implementation_for_SIMD_backend branch from e84bf9a to 962b3b7 Compare May 2, 2024 16:29
@andrewmilson andrewmilson force-pushed the 04-30-Add_bit_reverse_for_SIMD_backend branch from 9ccaf1f to c13b2e1 Compare May 2, 2024 16:30
@andrewmilson andrewmilson force-pushed the 05-02-Add_blake2s_implementation_for_SIMD_backend branch from 962b3b7 to 4c6a57e Compare May 2, 2024 16:30
@andrewmilson andrewmilson force-pushed the 04-30-Add_bit_reverse_for_SIMD_backend branch from c13b2e1 to 4265dcb Compare May 2, 2024 16:35
@andrewmilson andrewmilson force-pushed the 05-02-Add_blake2s_implementation_for_SIMD_backend branch from 4c6a57e to 436d936 Compare May 2, 2024 16:35
@andrewmilson andrewmilson force-pushed the 04-30-Add_bit_reverse_for_SIMD_backend branch from 4265dcb to b2f874d Compare May 2, 2024 19:09
@andrewmilson andrewmilson force-pushed the 05-02-Add_blake2s_implementation_for_SIMD_backend branch from 436d936 to 64bba16 Compare May 2, 2024 19:09
@andrewmilson andrewmilson force-pushed the 04-30-Add_bit_reverse_for_SIMD_backend branch from b2f874d to 209d27b Compare May 2, 2024 19:24
@andrewmilson andrewmilson force-pushed the 05-02-Add_blake2s_implementation_for_SIMD_backend branch from 64bba16 to 804fd02 Compare May 2, 2024 19:24
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 64bba16 Previous: 3d666a1 Ratio
cpu bit_rev 24bit 425012444 ns/iter (± 16504444) 192851383 ns/iter (± 4615303) 2.20

This comment was automatically generated by workflow using github-action-benchmark.

CC: @spapinistarkware

@andrewmilson andrewmilson force-pushed the 04-30-Add_bit_reverse_for_SIMD_backend branch from 209d27b to 71d3b5e Compare May 2, 2024 20:08
@andrewmilson andrewmilson force-pushed the 05-02-Add_blake2s_implementation_for_SIMD_backend branch from 804fd02 to 1b6c6c1 Compare May 2, 2024 20:09
@andrewmilson andrewmilson force-pushed the 04-30-Add_bit_reverse_for_SIMD_backend branch from 71d3b5e to 1a84c3c Compare May 2, 2024 20:34
@andrewmilson andrewmilson force-pushed the 05-02-Add_blake2s_implementation_for_SIMD_backend branch from 1b6c6c1 to fa27659 Compare May 2, 2024 20:34
@andrewmilson andrewmilson force-pushed the 04-30-Add_bit_reverse_for_SIMD_backend branch from 314685b to f50af06 Compare May 7, 2024 18:50
@andrewmilson andrewmilson force-pushed the 05-02-Add_blake2s_implementation_for_SIMD_backend branch from 9626fbb to 4a7ffb2 Compare May 7, 2024 18:50
@andrewmilson andrewmilson force-pushed the 04-30-Add_bit_reverse_for_SIMD_backend branch from f50af06 to 3b8ca2d Compare May 8, 2024 15:45
@andrewmilson andrewmilson force-pushed the 05-02-Add_blake2s_implementation_for_SIMD_backend branch from 4a7ffb2 to ccb0c2e Compare May 8, 2024 15:45
Copy link
Collaborator

@shaharsamocha7 shaharsamocha7 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @andrewmilson)


crates/prover/src/core/backend/simd/blake2s.rs line 270 at r2 (raw file):

/// representing 16 packed instances of a message word.
/// # Safety
pub unsafe fn transpose_msgs(mut data: [u32x16; 16]) -> [u32x16; 16] {

rename

Suggestion:

pub unsafe fn transpose_states(mut data: [u32x16; 16]) -> [u32x16; 16] {

crates/prover/src/core/backend/simd/blake2s.rs line 306 at r2 (raw file):

    for _ in 0..3 {
        states = [
            LoLoInterleaveHiLo::concat_swizzle(states[0], states[4]),

Fix as we discussed.

Code quote:

LoLoInterleaveHiLo::concat_swizzle(states[0], states[4]),

Copy link
Contributor Author

@andrewmilson andrewmilson left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @shaharsamocha7)


crates/prover/src/core/backend/simd/blake2s.rs line 270 at r2 (raw file):

Previously, shaharsamocha7 wrote…

rename

transpose_states is a different function


crates/prover/src/core/backend/simd/blake2s.rs line 306 at r2 (raw file):

Previously, shaharsamocha7 wrote…

Fix as we discussed.

Is already updated.

Copy link
Collaborator

@spapinistarkware spapinistarkware left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 3 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @shaharsamocha7)

@andrewmilson andrewmilson force-pushed the 04-30-Add_bit_reverse_for_SIMD_backend branch from 3b8ca2d to 501b9ea Compare May 10, 2024 21:11
@andrewmilson andrewmilson force-pushed the 05-02-Add_blake2s_implementation_for_SIMD_backend branch from ccb0c2e to c693113 Compare May 10, 2024 21:11
@andrewmilson andrewmilson force-pushed the 04-30-Add_bit_reverse_for_SIMD_backend branch from 501b9ea to 6e55592 Compare May 11, 2024 02:41
@andrewmilson andrewmilson force-pushed the 05-02-Add_blake2s_implementation_for_SIMD_backend branch from c693113 to 120b1c5 Compare May 11, 2024 02:41
@andrewmilson andrewmilson force-pushed the 04-30-Add_bit_reverse_for_SIMD_backend branch from 6e55592 to 2719965 Compare May 15, 2024 04:34
@andrewmilson andrewmilson force-pushed the 05-02-Add_blake2s_implementation_for_SIMD_backend branch from 120b1c5 to 6182cfc Compare May 15, 2024 04:34
Copy link
Collaborator

@spapinistarkware spapinistarkware left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r5.
Reviewable status: all files reviewed (commit messages unreviewed), 1 unresolved discussion (waiting on @shaharsamocha7)

@andrewmilson andrewmilson force-pushed the 04-30-Add_bit_reverse_for_SIMD_backend branch from 2719965 to e4f0cbb Compare May 15, 2024 14:57
Base automatically changed from 04-30-Add_bit_reverse_for_SIMD_backend to dev May 15, 2024 15:01
@andrewmilson andrewmilson force-pushed the 05-02-Add_blake2s_implementation_for_SIMD_backend branch from 6182cfc to 9f342e8 Compare May 15, 2024 15:03
@andrewmilson andrewmilson requested review from shaharsamocha7 and removed request for shaharsamocha7 May 15, 2024 15:08
Copy link
Collaborator

@shaharsamocha7 shaharsamocha7 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: all files reviewed (commit messages unreviewed), 1 unresolved discussion (waiting on @spapinistarkware)

@andrewmilson andrewmilson merged commit f2c90cd into dev May 15, 2024
21 of 22 checks passed
@andrewmilson andrewmilson deleted the 05-02-Add_blake2s_implementation_for_SIMD_backend branch May 15, 2024 15:16
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.

None yet

4 participants