Poseidon2 perf improvement #758
Open
+394
−427
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LeonHibnik
reviewed
Feb 10, 2025
| // Multiplication by partial (sparse) matrix. | ||
| partial_matrix_diagonal_m1_mul_by_vector(tmp_fields, partial_matrix_diagonal_m1, tmp_fields); | ||
| S prepared_element; | ||
| #pragma unroll |
There was a problem hiding this comment.
Is T known at compile time? if not, unrolling won't be made anyway
Can unroll the smallest value of T...
| return element * element * element * element * element * element * element * element * element * element * | ||
| element; | ||
| default: { | ||
| // Danny TODO: How to stop here if there is an illegal alpha? |
There was a problem hiding this comment.
IMO return eIcicileError and add the return to the api
| // Note that in order to increase the performance the partial matrix diagonal values | ||
| // are actually partial matrix diagonal values minus 1. | ||
| void partial_matrix_diagonal_m1_mul_by_vector(const S* vec_in, const S* matrix_in, S* result) const | ||
| S sbox_el(S element, const int alpha) const |
There was a problem hiding this comment.
name unclear, sbox_element?
Suggested change
| S sbox_el(S element, const int alpha) const | |
| eIcicleError sbox_element(S element, const int alpha, S to_return) const |
| @@ -145,341 +142,437 @@ namespace icicle { | |||
| break; | |||
| default: | |||
| ICICLE_LOG_ERROR | |||
| << "cpu_poseidon2_init_default_constants: T (width) must be one of [2, 3, 4, 8, 12, 16, 20, 24]\n"; | |||
| << "cuda_poseidon2_init_default_constants: T (width) must be one of [2, 3, 4, 8, 12, 16, 20, 24]"; | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Poseidon2 CPU perf improvement by 17% for sponge and 10% for non-sponge.
Changes are only in CPU backend file and the tests file.
This PR...
Linked Issues
Resolves #
(optional) CUDA backend branch
cuda-backend-branch: main # specify the branch here