Skip to content
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.

Sign up for GitHub

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

Jump to bottom

addc/subc optimizations #253

Merged
merged 2 commits into from
Mar 15, 2022
Merged

addc/subc optimizations #253

merged 2 commits into from
Mar 15, 2022

Conversation

chfast
Copy link
Owner

@chfast chfast commented Mar 9, 2022 •
edited
Loading

Use compiler builtins to implement addc and subc primitives. This mostly benefits GCC (clang is able to match respective patterns without builtins).

@chfast chfast force-pushed the addc_optimization branch from 8db47df to fa22289 Compare March 9, 2022 12:28
@chfast chfast marked this pull request as ready for review March 9, 2022 12:29
@codecov-commenter
Copy link

codecov-commenter commented Mar 9, 2022 •
edited
Loading

Codecov Report

Merging #253 (c348241) into master (b05e8a1) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #253   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines         1911      1958   +47     
=========================================
+ Hits          1911      1958   +47
Flag Coverage Δ
32bit 100.00% <100.00%> (ø)
gcc 99.48% <92.42%> (-0.52%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
test/unittests/test_builtins.cpp 100.00% <ø> (ø)
test/unittests/test_uint256.cpp 100.00% <ø> (ø)
include/intx/intx.hpp 100.00% <100.00%> (ø)
test/experimental/addmod.hpp 100.00% <100.00%> (ø)

@chfast chfast force-pushed the addc_optimization branch 11 times, most recently from 0e8efec to 10df349 Compare March 14, 2022 22:23
@chfast chfast requested review from gumb0 and yperbasis March 15, 2022 07:30
@yperbasis
Copy link
Collaborator

According to Wiki, there are 2 different conventions for subtraction: carry & borrow. x86 uses borrow & ARM uses carry. I'm not sure which convention __builtin_subcll uses. Is there good documentation for it anywhere?

@chfast
Copy link
Owner Author

chfast commented Mar 15, 2022

According to Wiki, there are 2 different conventions for subtraction: carry & borrow. x86 uses borrow & ARM uses carry. I'm not sure which convention __builtin_subcll uses. Is there good documentation for it anywhere?

Oh, interesting. This is "sub with borrow" then although the name may suggest otherwise. And looks this is not very well optimized on ARM maybe because of the reversed carry bit.

https://godbolt.org/z/sPsahE4f9

chfast and others added 2 commits March 15, 2022 18:16
@chfast
Optimize addc() and subc() with builtins
08179ca
@chfast
Tune fast path in addmod()
04128be 
The algorithm remains the same, but the changes help low-level
compiler optimizations (e.g. by limiting lifetime of result_vith_carry
objects).
@chfast chfast force-pushed the addc_optimization branch from 10df349 to 04128be Compare March 15, 2022 17:16
@chfast chfast merged commit 29cabb9 into master Mar 15, 2022
@chfast chfast deleted the addc_optimization branch March 15, 2022 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gumb0 gumb0 Awaiting requested review from gumb0

@yperbasis yperbasis Awaiting requested review from yperbasis

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@chfast @codecov-commenter @yperbasis