Skip to content

arithmetic: Rewrite limbs_reduce_once.#2435

Open
briansmith wants to merge 1 commit intomainfrom
b/am-1-2
Open

arithmetic: Rewrite limbs_reduce_once.#2435
briansmith wants to merge 1 commit intomainfrom
b/am-1-2

Conversation

@briansmith
Copy link
Copy Markdown
Owner

Move the function to arithmetic from limb. This is step towards moving all arithmetic out of limb.

Change the signature so that the reduction is done separately instead of in-place. It was already being done separately in bigint and it costs very little, if anything, to do the same in the other caller in ec.

Optimize the implementation to take advantage of the fact that r and a do not alias each other. To do so, replace LIMBS_reduce_once with two separate helper functions, LIMBS_sub and LIMBS_cmov.

As part of doing this, ensure we're not passing any empty slices to the relevant C code.

git difftool HEAD^1:src/limb.rs src/arithmetic/limbs/reduce_once.rs

@briansmith briansmith self-assigned this Mar 3, 2025
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 3, 2025

Codecov Report

❌ Patch coverage is 95.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.47%. Comparing base (56ac0fb) to head (a189aed).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/arithmetic/limbs/fallback/cmov.rs 76.47% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2435      +/-   ##
==========================================
- Coverage   96.49%   96.47%   -0.02%     
==========================================
  Files         207      210       +3     
  Lines       20430    20479      +49     
  Branches      520      524       +4     
==========================================
+ Hits        19713    19758      +45     
- Misses        599      601       +2     
- Partials      118      120       +2     

☔ View full report in Codecov by Sentry.
📢 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.

@briansmith briansmith added this to the 0.17.15 milestone Mar 11, 2025
@georglauterbach georglauterbach mentioned this pull request Jun 4, 2025
@briansmith briansmith force-pushed the b/am-1-2 branch 2 times, most recently from 25c11d2 to dab9c38 Compare March 24, 2026 18:53
Move the function to `arithmetic` from `limb`. This is step towards
moving all arithmetic out of `limb`.

Change the signature so that the reduction is done separately instead
of in-place. It was already being done separately in `bigint` and it
costs very little, if anything, to do the same in the other caller in
`ec`.

Optimize the implementation to take advantage of the fact that `r`
and `a` do not alias each other. To do so, replace
`LIMBS_reduce_once` with two separate helper functions, `LIMBS_sub`
and `LIMBS_cmov`.

As part of doing this, ensure we're not passing any empty slices to
the relevant C code.

```
git difftool HEAD^1:src/limb.rs src/arithmetic/limbs/reduce_once.rs
```
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.

1 participant