-
Notifications
You must be signed in to change notification settings - Fork 4
Add 62 bit limb implementation, script and update benches #66
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
Conversation
WIP Add 62 bit limb implementation, script and update benches
giarc3
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.
LGTM
| y.const_lt(self).not() | ||
| } | ||
|
|
||
| #[inline] |
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.
This doesn't cover the case where they are equal.
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.
I think it does. In both const_gt and const_lt if they're equal the value 0 is produced, which is correct.
Do you see something I'm missing?
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.
I'm sorry - I had a brain cramp.
Co-authored-by: Bob Wall <bob.wall@ironcorelabs.com>
| y.const_lt(self).not() | ||
| } | ||
|
|
||
| #[inline] |
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.
I'm sorry - I had a brain cramp.
This is a full implementation of the 62 bit limb version of gridiron. In the process I discovered that the ff31 version doesn't port directly. There are some bitshifts that are different because of the limb sizes changing. The property based tests give us a high confidence in the changes.
This is a breaking change because the default has changed to 62 bit. The only time you wouldn't want this is WASM (or some other niche 32 bit arch). The README has been updated to give all this info.
I've also included a python script that will calculate the values for the macro invocation. This used to be done only in sage, but it can be done with normal python as well.
This new implementation is about twice as fast as the old one on 64 bit archs.
Closes #12.