Skip to content

replace double with bit shift - #50

Open
str4d wants to merge 1 commit into
mainfrom
improve-double
Open

replace double with bit shift#50
str4d wants to merge 1 commit into
mainfrom
improve-double

Conversation

@str4d

@str4d str4d commented Oct 13, 2022

Copy link
Copy Markdown
Contributor

Extracted from #44.

@codecov-commenter

codecov-commenter commented Oct 13, 2022

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.44%. Comparing base (682a0e6) to head (9b4bfa5).
⚠️ Report is 82 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #50      +/-   ##
==========================================
- Coverage   66.10%   60.44%   -5.67%     
==========================================
  Files          12       10       -2     
  Lines        1428     1221     -207     
==========================================
- Hits          944      738     -206     
+ Misses        484      483       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@str4d

str4d commented Oct 13, 2022

Copy link
Copy Markdown
Contributor Author

Hmm, this implementation does not appear to be more efficient. I ran:

$ rustc +stable --version
rustc 1.64.0 (a55dd71d5 2022-09-19)
$ cargo +stable asm --lib "<pasta_curves::fields::fp::Fp as ff::Field>::double"

once on current main and again on this PR, and got the following assembly:

Current main
.section ".text.<pasta_curves::fields::fp::Fp as ff::Field>::double","ax",@progbits
	.globl	<pasta_curves::fields::fp::Fp as ff::Field>::double
	.p2align	4, 0x90
	.type	<pasta_curves::fields::fp::Fp as ff::Field>::double,@function
<pasta_curves::fields::fp::Fp as ff::Field>::double:

	.cfi_startproc
	mov rax, rdi

	mov rdx, qword ptr [rsi]

	mov r11, qword ptr [rsi + 8]

	lea r10, [rdx + rdx]

	mov r9, qword ptr [rsi + 16]

	xor edi, edi
	movabs r8, 7409212017489215487

	add r8, r10

	adc rdi, -1

	shrd rdx, r11, 63
	sar rdi, 63

	add rdx, rdi

	adc rdi, 0
	movabs r10, -2469829653914515739

	add r10, rdx

	adc rdi, -1

	shrd r11, r9, 63

	sar rdi, 63

	add r11, rdi

	adc rdi, 0

	mov rdx, qword ptr [rsi + 24]

	sar rdi, 63

	shld rdx, r9, 1
	add rdx, rdi
	adc rdi, 0
	movabs r9, -4611686018427387904
	add r9, rdx
	adc rdi, -1

	movabs rdx, -7409212017489215487

	and rdx, rdi
	movabs rsi, 2469829653914515739

	and rsi, rdi
	movabs rcx, 4611686018427387904

	and rcx, rdi

	add rdx, r8

	adc rsi, r10

	adc r11, 0

	adc rcx, r9

	mov qword ptr [rax], rdx
	mov qword ptr [rax + 8], rsi
	mov qword ptr [rax + 16], r11
	mov qword ptr [rax + 24], rcx

	ret

	.size	<pasta_curves::fields::fp::Fp as ff::Field>::double, .Lfunc_end50-<pasta_curves::fields::fp::Fp as ff::Field>::double
Current main
.section ".text.<pasta_curves::fields::fp::Fp as ff::Field>::double","ax",@progbits
	.globl	<pasta_curves::fields::fp::Fp as ff::Field>::double
	.p2align	4, 0x90
	.type	<pasta_curves::fields::fp::Fp as ff::Field>::double,@function
<pasta_curves::fields::fp::Fp as ff::Field>::double:

	.cfi_startproc
	push rbx
	.cfi_def_cfa_offset 16
	.cfi_offset rbx, -16
	mov rax, rdi

	mov rdx, qword ptr [rsi]

	mov rbx, qword ptr [rsi + 8]

	mov rcx, rbx
	shld rcx, rdx, 1

	add rdx, rdx

	mov r8, qword ptr [rsi + 16]

	shrd rbx, r8, 63

	mov r9, qword ptr [rsi + 24]

	shld r9, r8, 1

	xor esi, esi

	movabs r8, 7409212017489215487

	add r8, rdx

	adc rsi, -1

	sar rsi, 63

	add rcx, rsi

	adc rsi, 0
	movabs r10, -2469829653914515739
	add r10, rcx

	adc rsi, -1

	sar rsi, 63

	add rbx, rsi

	adc rsi, 0

	sar rsi, 63

	add r9, rsi

	adc rsi, 0
	movabs r11, -4611686018427387904
	add r11, r9
	adc rsi, -1

	movabs rdx, -7409212017489215487

	and rdx, rsi
	movabs rcx, 2469829653914515739

	and rcx, rsi
	movabs rdi, 4611686018427387904

	and rdi, rsi

	add rdx, r8

	adc rcx, r10

	adc rbx, 0

	adc rdi, r11

	mov qword ptr [rax], rdx
	mov qword ptr [rax + 8], rcx
	mov qword ptr [rax + 16], rbx
	mov qword ptr [rax + 24], rdi

	pop rbx

	.cfi_def_cfa_offset 8
	ret

	.size	<pasta_curves::fields::fp::Fp as ff::Field>::double, .Lfunc_end50-<pasta_curves::fields::fp::Fp as ff::Field>::double

It looks to me like rustc and LLVM are able to optimise the self.add(self) implementation down to basically the same assembly as the manual bitshifting implementation, while the latter causes a couple of additional assembly instructions to be generated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants