My approach here is to make the minimal changes from the Bandersnatch repo implementation so we can take advantage of the speedup as soon as possible.
Upstreaming this to gnark is a separate beast, in the sense that from what I saw it might require some refactors in their fft package to extract roots of unity and might not be entirely trivial. Still is a good idea to do it, but I don't want that to block our progress in the VKT speedups.
My strategy was to include this separate "faster" sqrt so that doesn't mess up with generated code from gnark, since that would be painful for us in the near future when we try to (attempt) to not depend on generated code and try using gnark directly. (Again, that's a different topic!)
jsign
commented
1 year ago
This PR includes optimization for sqrt.
My approach here is to make the minimal changes from the Bandersnatch repo implementation so we can take advantage of the speedup as soon as possible.
Upstreaming this to gnark is a separate beast, in the sense that from what I saw it might require some refactors in their
fftpackage to extract roots of unity and might not be entirely trivial. Still is a good idea to do it, but I don't want that to block our progress in the VKT speedups.My strategy was to include this separate "faster" sqrt so that doesn't mess up with generated code from gnark, since that would be painful for us in the near future when we try to (attempt) to not depend on generated code and try using gnark directly. (Again, that's a different topic!)
For more context or info: