Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
@Pratyush @weikengchen @mmaker I've updated the PR and Issue with more information and the presentation slides: https://andrewzitek.xyz/images/small_fp_slides.pdf We present this to Ale at 17h GMT+1 today. You are very welcome to attend and we are happy to schedule this 15 min presentation with you at another time. Thanks. |
|
I feel that the current shape is okay-enough for a review... let me take a look |
There was a problem hiding this comment.
I am just curious why not just always do
SmallFp::new(other % P::MODULUS)or the like (due to types), since I feel it would be faster than the try_from and many other things that involve function calls.
There was a problem hiding this comment.
We know that other is u16 but the argument to SmallFp::new() is type T in {u8, u16, u32, u64, u128} so the cleanest I can figure out how to make it would be something like:
fn from(other: u16) -> Self {
let modulus_as_u128: u128 = P::MODULUS.into();
let other_as_u128: u128 = other.into();
let reduced = other_as_u128 % modulus_as_u128;
// let reduced_as_t = P::T::try_from(reduced).unwrap(); <-- this fails bc the error doesn't implement debug
let reduced_as_t = P::T::try_from(reduced).unwrap_or_else(|_| panic!("Reduced value should fit in T"));
SmallFp::new(reduced_as_t)
}
But by including the match, being that other is u16 we'd probably fall into the Ok() branch a huge amount of the time (but not all the time) and skip the u128 reduction.
There was a problem hiding this comment.
I think I am in favor of the new one because it offers another advantage --- it seems to be mostly constant-time, while the previous one doesn't.
There was a problem hiding this comment.
Hey, I did another pass I think it's this that's needed to match the behavior of the BigInt impl: benbencik#10
|
(I think I have 6 files left but would love to hear from you on the ones above first) |
Co-authored-by: Weikeng Chen <w.k@berkeley.edu>
Update MODULUS_128 to MODULUS_U128
Use shave bits in sample func
|
We probably will find a way to bypass the nightly compilation failures in another PR... so ignore that for now |
|
oh there is already a PR in the work for that: #1059 (comment) |
Description
The primary motivation of this PR is to create a generalized path toward vectorized/ SIMD instruction optimizations for finite fields in Arkworks.
In the process, we pick up a non-trivial performance boost in serial.
SmallFp, a procedural macro for instantiating prime fields with modulus< 2^127u8,u16,u32,u64,u128)SLIDES: https://andrewzitek.xyz/images/small_fp_slides.pdf
Closes: #1038
This work was done in collaboration with @z-tech