-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Co-Z + effective affine precomputation + tests #174
Conversation
Much appreciated, sipa. Nice catch of the missing normalisation of a->y in dblu. |
9929bdf
to
e76c49e
Compare
Updated. Rebased on top of #176, splitted up the commits, and made some changes to comments and function names. Instead of secp256k1_gej_add_ge_var allowing an "a Z ratio" to be passed in, I've turned it into a separate function secp256k1_gej_add_zinv, which adds a point B whose Z coordinate is given by passing in its inverse (which is effectively equivalent to the old method with an azr passed in, but easier to explain IMHO). |
Benchmark before and after this entire PR (including both of @peterdettman's commits):
|
I don't mind the switch to an equivalent zInv formulation; it corresponds to the first "alternative scheme" given in #41 (comment). It avoids the awkwardness of using a->z*azr, then updating from just a->z. @sipa Did you have any thoughts on #159? All these coz and mixed additions are doing input normalisation that could be avoided... |
@peterdettman Not sure what optimization there you're referring to, the comment is a bit vague. If you have further improvements, feel free to PR them! |
@peterdettman I'm not sure I'm following you. 28f317f makes 0 semantic changes - it only separates the azr==NULL and azr!=NULL case into separate functions, and renames the second one and its explanation/argument name. |
Sorry, the second link was wrong, I was asking about #159. As for the first part, I agree there's no semantic change, I was just noting that I'd previously used the _gej_add_zinv "syntax" in a similar context, so it's arguably a more natural presentation. |
03c27ab
to
80d9287
Compare
Rebased, and restructured the commits significantly (moving the Co-Z part to a separate commit). @peterdettman since the code and ideas were yours, I left you as author, but feel free to complain if you think that's not appropriate. |
@sipa I don't have any problems with that, thanks for asking though. |
@sipa thanks for rebasing |
Rebased again. |
Needs operation counts. |
Done. |
@@ -305,11 +310,16 @@ if test x"$use_endomorphism" = x"yes"; then | |||
AC_DEFINE(USE_ENDOMORPHISM, 1, [Define this symbol to use endomorphism optimization]) | |||
fi | |||
|
|||
if test x"$use_coz" = x"yes"; then | |||
AC_DEFINE(USE_COZ, 1, [Define this symbol to use endomorphism optimization]) |
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.
Copy/paste error for help text here?
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.
Refactored the code further (to facilitate later batch chaining, in order to support batch signature validation). |
@sipa I put together a commit demonstrating how to get half of the speed gain of Co-Z precomp without the Co-Z formulae: peterdettman@f188650 . It basically applies the effective-affine trick so that the precomp additions are mixed. |
@peterdettman Awesome, makes perfect sense. Benchmarks (without GLV, with GMP): Benchmarks (with GLV, with GMP): |
* Make secp256k1_gej_add_var and secp256k1_gej_double return the Z ratio to go from a.z to r.z. * Use these Z ratios to speed up batch point conversion to affine coordinates, and to speed up batch conversion of points to a common Z coordinate. * Add a point addition function that takes a point with a known Z inverse. * Due to secp256k1's endomorphism, all additions in the EC multiplication code can work on affine coordinate (with an implicit common Z coordinate), correcting the Z coordinate of the result afterwards. Refactoring by Pieter Wuille: * Move more global-z logic into the group code. * Separate code for computing the odd multiples from the code to bring it to either storage or globalz format. * Rename functions. * Make all addition operations return Z ratios, and test them. * Make the zr table format compatible with future batch chaining (the first entry in zr becomes the ratio between the input and the first output). Original idea and code by Peter Dettman.
- Selected Co-Z formulas from "Scalar Multiplication on Weierstraß Elliptic Curves from Co-Z Arithmetic" (Goundar, Joye, et. al.) added as group methods with new type sep256k1_coz_t. - Co-Z methods used for A and G point precomputations. - DBLU cost: 3M+4S, ZADDU cost: 5M+2S. Original idea and code by Peter Dettman. Refactored by Pieter Wuille.
Closing this, way too many refactors to follow the discussion; I'll open new pull requests. |
This builds on top of #171 but reworks the group element tests, making them much more extensive (and faster).