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

Modify FFT implementation for SIMD backend #596

Merged
merged 1 commit into from
May 16, 2024

Conversation

andrewmilson
Copy link
Contributor

@andrewmilson andrewmilson commented May 1, 2024

This change is Reviewable

@andrewmilson andrewmilson force-pushed the 04-30-Add_blake2s_implementation_for_SIMD_backend branch from 89b71dc to 05e275f Compare May 1, 2024 04:42
@andrewmilson andrewmilson force-pushed the 05-01-Add_FFT_implementation_for_SIMD_backend branch 2 times, most recently from 1c9274c to 01ce893 Compare May 1, 2024 04:45
@codecov-commenter
Copy link

codecov-commenter commented May 1, 2024

Codecov Report

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

Project coverage is 92.12%. Comparing base (25bf368) to head (3b38213).
Report is 3 commits behind head on dev.

Files Patch % Lines
crates/prover/src/core/backend/simd/fft/ifft.rs 99.00% 1 Missing and 1 partial ⚠️
crates/prover/src/core/backend/simd/fft/rfft.rs 99.09% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #596      +/-   ##
==========================================
+ Coverage   91.98%   92.12%   +0.13%     
==========================================
  Files          80       80              
  Lines       10916    11275     +359     
  Branches    10916    11275     +359     
==========================================
+ Hits        10041    10387     +346     
- Misses        806      821      +15     
+ Partials       69       67       -2     

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

@andrewmilson andrewmilson force-pushed the 04-30-Add_blake2s_implementation_for_SIMD_backend branch from 05e275f to a5463cc Compare May 1, 2024 15:59
@andrewmilson andrewmilson force-pushed the 05-01-Add_FFT_implementation_for_SIMD_backend branch from 01ce893 to ba78073 Compare May 1, 2024 15:59
@andrewmilson andrewmilson force-pushed the 04-30-Add_blake2s_implementation_for_SIMD_backend branch from a5463cc to 1c2de17 Compare May 1, 2024 16:04
@andrewmilson andrewmilson force-pushed the 05-01-Add_FFT_implementation_for_SIMD_backend branch from ba78073 to 0ff0d6e Compare May 1, 2024 16:06
@andrewmilson andrewmilson force-pushed the 04-30-Add_blake2s_implementation_for_SIMD_backend branch from 1c2de17 to 6ffe5a9 Compare May 2, 2024 04:42
@andrewmilson andrewmilson force-pushed the 05-01-Add_FFT_implementation_for_SIMD_backend branch from 0ff0d6e to d3c0c82 Compare May 2, 2024 04:42
Copy link
Contributor

@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.

Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @andrewmilson)

a discussion (no related file):
It would be easier to check this Pr if you first add a commit that only copies the old files, so we can see in Reviewable just what changed.


@andrewmilson andrewmilson changed the title Add FFT implementation for SIMD backend Modify FFT implementation for SIMD backend May 2, 2024
@andrewmilson andrewmilson force-pushed the 04-30-Add_blake2s_implementation_for_SIMD_backend branch from 6ffe5a9 to 948ab4d Compare May 2, 2024 15:53
@andrewmilson andrewmilson force-pushed the 05-01-Add_FFT_implementation_for_SIMD_backend branch from d3c0c82 to 84a70a2 Compare May 2, 2024 15:53
@andrewmilson andrewmilson force-pushed the 04-30-Add_blake2s_implementation_for_SIMD_backend branch from 948ab4d to 91c40c5 Compare May 2, 2024 16:22
@andrewmilson andrewmilson force-pushed the 05-01-Add_FFT_implementation_for_SIMD_backend branch from 84a70a2 to 0123ac9 Compare May 2, 2024 16:22
@andrewmilson andrewmilson force-pushed the 04-30-Add_blake2s_implementation_for_SIMD_backend branch from 91c40c5 to 705a536 Compare May 2, 2024 16:29
@andrewmilson andrewmilson force-pushed the 05-01-Add_FFT_implementation_for_SIMD_backend branch from 0123ac9 to 7fb3601 Compare May 2, 2024 16:29
@andrewmilson andrewmilson force-pushed the 04-30-Add_blake2s_implementation_for_SIMD_backend branch from 705a536 to 005b77b Compare May 2, 2024 16:30
@andrewmilson andrewmilson force-pushed the 05-01-Add_FFT_implementation_for_SIMD_backend branch from 7fb3601 to 1e963c5 Compare May 2, 2024 16:30
@andrewmilson andrewmilson force-pushed the 05-01-Add_FFT_implementation_for_SIMD_backend branch from b5a1cdc to 1a083e5 Compare May 6, 2024 19:50
@andrewmilson andrewmilson force-pushed the 05-02-Add_FFT_implementation_for_SIMD_backend branch from 347d446 to df21476 Compare May 7, 2024 18:50
@andrewmilson andrewmilson force-pushed the 05-01-Add_FFT_implementation_for_SIMD_backend branch from 1a083e5 to 8ec20f3 Compare May 7, 2024 18:50
@andrewmilson andrewmilson force-pushed the 05-02-Add_FFT_implementation_for_SIMD_backend branch from df21476 to 4979fb3 Compare May 8, 2024 15:45
@andrewmilson andrewmilson force-pushed the 05-01-Add_FFT_implementation_for_SIMD_backend branch from 8ec20f3 to 2c1927c Compare May 8, 2024 15:45
@andrewmilson andrewmilson force-pushed the 05-02-Add_FFT_implementation_for_SIMD_backend branch 2 times, most recently from 941402f to 0fdf4c7 Compare May 11, 2024 00:35
@andrewmilson andrewmilson force-pushed the 05-01-Add_FFT_implementation_for_SIMD_backend branch from 2c1927c to f644aa6 Compare May 11, 2024 00:35
@andrewmilson andrewmilson force-pushed the 05-02-Add_FFT_implementation_for_SIMD_backend branch from 0fdf4c7 to d68543b Compare May 11, 2024 02:41
@andrewmilson andrewmilson force-pushed the 05-01-Add_FFT_implementation_for_SIMD_backend branch from f644aa6 to c88d1ef Compare May 11, 2024 02:42
@andrewmilson andrewmilson force-pushed the 05-02-Add_FFT_implementation_for_SIMD_backend branch from d68543b to 3cb07a8 Compare May 15, 2024 04:34
@andrewmilson andrewmilson force-pushed the 05-01-Add_FFT_implementation_for_SIMD_backend branch from c88d1ef to 86625b8 Compare May 15, 2024 04:34
@andrewmilson andrewmilson force-pushed the 05-02-Add_FFT_implementation_for_SIMD_backend branch from 3cb07a8 to 16f412d Compare May 15, 2024 19:37
@andrewmilson andrewmilson force-pushed the 05-01-Add_FFT_implementation_for_SIMD_backend branch from 86625b8 to 993fee6 Compare May 15, 2024 19:37
@andrewmilson andrewmilson force-pushed the 05-02-Add_FFT_implementation_for_SIMD_backend branch 2 times, most recently from 83fd68d to 1ade01d Compare May 16, 2024 03:56
@andrewmilson andrewmilson force-pushed the 05-01-Add_FFT_implementation_for_SIMD_backend branch from 993fee6 to b09665e Compare May 16, 2024 03:56
@andrewmilson andrewmilson force-pushed the 05-02-Add_FFT_implementation_for_SIMD_backend branch from 1ade01d to 42df366 Compare May 16, 2024 12:18
Base automatically changed from 05-02-Add_FFT_implementation_for_SIMD_backend to dev May 16, 2024 12:22
@andrewmilson andrewmilson force-pushed the 05-01-Add_FFT_implementation_for_SIMD_backend branch from b09665e to 2d73436 Compare May 16, 2024 12:23
Copy link
Contributor

@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 2 of 4 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @andrewmilson)


crates/prover/src/core/backend/simd/fft/mod.rs line 40 at r4 (raw file):

                    continue;
                }
                let val0 = load(values.add(i << 4).cast_const());

Consider inlining load() and store()


crates/prover/src/core/backend/simd/fft/rfft.rs line 343 at r4 (raw file):

/// unreduced form, [0, P] including P. twiddle_dbl holds 16 values, each is a *double* of a twiddle
/// factor, in unreduced form, [0, 2*P].
pub fn butterfly(

butterfly() is a generic function that exists already, I want to not confuse them

Suggestion:

simd_butterfly

@andrewmilson andrewmilson force-pushed the 05-01-Add_FFT_implementation_for_SIMD_backend branch from 2d73436 to 3b38213 Compare May 16, 2024 12:34
@andrewmilson andrewmilson merged commit c3b650b into dev May 16, 2024
21 of 22 checks passed
@andrewmilson andrewmilson deleted the 05-01-Add_FFT_implementation_for_SIMD_backend branch May 16, 2024 12:38
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.

3 participants