-
Notifications
You must be signed in to change notification settings - Fork 9
SIMD packing for tweak hash tree #21
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
Merged
Merged
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
3585dad
implement variant of tree layer computation using SIMD packing
Vindaar c6c16fc
[tests] add test cases for SIMD tree construction
Vindaar 3cc78ea
address clippy errors
Vindaar c9e234f
avoid temporary arrays entirely during packing
Vindaar c1dfc58
use `#[inline]` without (always)
Vindaar 6a182fd
remove `unpack_to_array`, `pack_column` and `pack_into` & their tests
Vindaar 026bf3b
annotate `unpack_array` for loop with needless_range_loop
Vindaar 6faeaff
put input / output layout doc comment for `unpack_array` back
Vindaar d062347
remove comment about test section
Vindaar 83a8a3c
use clippy too_many_lines for `compute_tree_leaves`
Vindaar 755292b
use chunked iterator for assignment of packed chains
Vindaar 99cbaf5
fix `level` passed to `tree_tweak` in default impl
Vindaar 9571be3
add doc comments for compute_tree_layer and poseidon override
Vindaar File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this
level+1correct? If I compare with the old code and unpack the function call, this would mean that we actually add 1 to the level twice now, whereas we did only once in the old code? Maybe this did not show up in tests as this default implementation is not used. Can you maybe add some test that also checks this?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just checked and above you seem to have just
levelin the scalar implementation that you use for testing.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, great catch! Yeah, I copied the code for the default implementation before I decided to just change the API to pass
level + 1directly and didn't update the default.