-
Notifications
You must be signed in to change notification settings - Fork 49
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
improve double and refactoring #44
base: main
Are you sure you want to change the base?
improve double and refactoring #44
Conversation
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! This reduces code duplication between the Fp
and Fq
implementations.
I want to take the first commit from this branch but not (yet) the second. I don't really want to maintain a second macro-based field implementation (the other being the proc-macro-based |
I've opened #49 for discussing code de-duplication approaches. In the meantime, I will open another PR with the first commit from this branch, so we can get it reviewed. |
Per #50 (comment), the manual bitshift-based doubling does not appear to be more efficient than the |
Hi @str4d I think we can speed up if we implement as following.
I am going to implement these if you think it's good idea. |
e150cc5
to
3036891
Compare
2adbb01
to
bcdbef9
Compare
double Before➜ pasta_curves git:(main) rustc +stable --version
rustc 1.66.1 (90743e729 2023-01-10)
➜ pasta_curves git:(main) cargo +stable asm --lib "<pasta_curves::fields::fp::Fp as ff::Field>::double"
<pasta_curves::fields::fp::Fp as ff::Field>::double:
push rbp
mov rbp, rsp
push rbx
mov rax, qword, ptr, [rsi]
mov r11, qword, ptr, [rsi, +, 8]
lea r10, [rax, +, rax]
mov r9, qword, ptr, [rsi, +, 16]
mov rsi, qword, ptr, [rsi, +, 24]
xor edx, edx
movabs r8, 7409212017489215487
add r8, r10
adc rdx, -1
shrd rax, r11, 63
sar rdx, 63
add rax, rdx
adc rdx, 0
movabs r10, -2469829653914515739
add r10, rax
adc rdx, -1
shrd r11, r9, 63
sar rdx, 63
add r11, rdx
adc rdx, 0
shld rsi, r9, 1
sar rdx, 63
add rsi, rdx
adc rdx, 0
movabs r9, -4611686018427387904
add r9, rsi
adc rdx, -1
movabs rsi, -7409212017489215487
and rsi, rdx
movabs rcx, 2469829653914515739
and rcx, rdx
movabs rbx, 4611686018427387904
and rbx, rdx
add rsi, r8
adc rcx, r10
adc r11, 0
mov rax, rdi
adc rbx, r9
mov qword, ptr, [rdi], rsi
mov qword, ptr, [rdi, +, 8], rcx
mov qword, ptr, [rdi, +, 16], r11
mov qword, ptr, [rdi, +, 24], rbx
pop rbx
pop rbp
ret double After➜ pasta_curves git:(feat/improve-double-and-refactoring) rustc +stable --version
rustc 1.66.1 (90743e729 2023-01-10)
➜ pasta_curves git:(feat/improve-double-and-refactoring) cargo +stable asm --lib "<pasta_curves::fields::fp::Fp as ff::Field>::double"
<pasta_curves::fields::fp::Fp as ff::Field>::double:
push rbp
mov rbp, rsp
mov rax, qword, ptr, [rsi]
mov r9, qword, ptr, [rsi, +, 8]
mov rcx, r9
shld rcx, rax, 1
add rax, rax
mov rdx, qword, ptr, [rsi, +, 16]
shrd r9, rdx, 63
mov rsi, qword, ptr, [rsi, +, 24]
shld rsi, rdx, 1
xor edx, edx
movabs r8, 7409212017489215487
add r8, rax
adc rdx, -1
sar rdx, 63
add rcx, rdx
adc rdx, 0
movabs rax, -2469829653914515739
add rax, rcx
adc rdx, -1
sar rdx, 63
add r9, rdx
adc rdx, 0
sar rdx, 63
add rsi, rdx
adc rdx, 0
movabs r10, -4611686018427387904
add r10, rsi
adc rdx, -1
movabs r11, -7409212017489215487
and r11, rdx
movabs rcx, 2469829653914515739
and rcx, rdx
movabs rsi, 4611686018427387904
and rsi, rdx
add r11, r8
adc rcx, rax
adc r9, 0
mov rax, rdi
adc rsi, r10
mov qword, ptr, [rdi], r11
mov qword, ptr, [rdi, +, 8], rcx
mov qword, ptr, [rdi, +, 16], r9
mov qword, ptr, [rdi, +, 24], rsi
pop rbp
ret
soooo slightly improved 😅 |
Abstract
Hi there.
I improve the
double
method with bit shift and commonalize the field operation.What I did
add
anddouble
methoddouble
arithmetic with bit shiftI would appreciate it if you could confirm.
Thank you!