-
Notifications
You must be signed in to change notification settings - Fork 157
Faster bn254 syscalls #414
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
base: master
Are you sure you want to change the base?
Conversation
joncinque
left a comment
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.
Thanks for your contribution, looks great to me! Just the one comment, @samkim-crypto can you also take a look?
|
Noting for anyone watching, that the |
Co-authored-by: Jon C <[email protected]>
samkim-crypto
left a comment
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 one small clarification below, but everything looks fine to me!
Inspired by #408 , we can
stealadapt these ideas to optimize the bn254 syscalls as well, since those all overwrite the full memory buffer as well. For the compression syscalls that return a fixed size array, we can utilize the sameMaybeUninittrick. For the addition, multiplication, and pairing syscalls we can alloc a vec once at the beginning instead of allocing an array and then converting that to a vec. This results in the following CU diffs:Note: I didn't apply this to the deprecated bn254 syscalls, but can do as well.
Note 2: Technically these optimizations could be applied inside the actual syscall implementation too instead of the wrapper functions interfacing with them for a couple extra CUs. However, this seemed more low risk, and idk if a SIMD would apply in that case so I went with this for now.