Skip to content

crypto: Drop the runtime-modulus Montgomery arithmetic - #1664

Open
chfast wants to merge 1 commit into
masterfrom
crypto/modarith-compile-time-mod
Open

crypto: Drop the runtime-modulus Montgomery arithmetic#1664
chfast wants to merge 1 commit into
masterfrom
crypto/modarith-compile-time-mod

Conversation

@chfast

@chfast chfast commented Aug 19, 2026

Copy link
Copy Markdown
Member

Every modulus in the codebase is a compile-time constant. Accepting one at runtime was needed only
for EVMMAX, which is gone. ModArith becomes MontgomeryArith<Mod> with the modulus as a template
parameter, so the modulus, R² and N′ are compile-time constants instead of members, and the
operations become static. The multiplication keeps its body — nothing is extracted, because nothing
outside the class would call it yet.

to_mont()/from_mont() become to_internal()/from_internal() in the same commit: they touch the
same call sites, and the Montgomery form is an implementation detail of this one arithmetic rather
than something its users need to name. The next arithmetic in the series keeps values in the plain
representation, where the conversions are identity.

Effect on generated code

The curve code already used a static constexpr ModArith, so the compiler had the modulus as a
constant there already and the field arithmetic itself is unchanged. Two things move: p256verify
stops computing R² by a 576-by-256-bit division on every call (the intx::udivrem instantiation,
udivrem_knuth and the reciprocal table are now absent from secp256r1.o), and in the inner pairing
function multiply_by_lin_func_value() GCC stops inlining one of the multiplications.

Instructions per call, hardware counters with two-point subtraction to cancel process startup
(GCC 15.2, -O3, x86-64-v2):

master this PR
p256verify 1,595,954 1,563,289 −2.05%
ecadd 21,993 21,780 −0.97%
ecrecover 1,588,150 1,575,664 −0.79%
ecmul 787,626 789,380 +0.22%
ecpairing 20,877,721 20,931,957 +0.26%

The micro-benchmark also constructed the arithmetic at runtime, so its multiplication gets up to
~30% faster now that the modulus is a constant (secp256k1's all-ones limbs turn multiplies into
shifts). That does not carry over to the library, for the reason above.

On the CodSpeed report

CodSpeed reports the opposite for four of these — ecrecover −10.3%, p256verify −5.3%, ecpairing
−3.9%, ecmul −3.7%, ecadd +3.2% — and warns that those comparisons crossed runtime environments. The
disagreement is in sign, not just size, for ecrecover and p256verify. An interleaved wall-clock A/B on
ecrecover put the two builds within ~1% of each other, spread ±4% by machine load, so a 10%
regression is not there on this hardware. Their simulation mode cannot be reproduced locally
(Valgrind does not run in my environment), and the toolchain differs (GCC 15 here, GCC 14 in CI).

Re-running the benchmark workflow on master would refresh the baseline and settle whether anything
survives.

Notes

  • MOD_INV is computed during constant evaluation, so an even modulus is now a compile-time error
    instead of a failed assert at runtime.
  • The typed test suite drops its ModA wrapper: MontgomeryArith<Mod> is directly the test type.
  • expmod.tmpl still generated ModArith-based code, while the checked-in generated code has been
    Curve::Fp-based for a while. Updated, so regenerating produces what is committed.

@codspeed-hq

codspeed-hq Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will degrade performance by 4.12%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
❌ 4 regressed benchmarks
✅ 124 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
precompile<ecrecover, ecrecover_execute_evmone> 11.1 ms 12.3 ms -10.32%
precompile<p256verify, p256verify_execute> 12.2 ms 12.9 ms -5.34%
precompile<ecpairing, ecpairing_execute> 327.6 ms 340.9 ms -3.9%
precompile<ecmul, ecmul_execute> 6.1 ms 6.4 ms -3.72%
precompile<ecadd, ecadd_execute> 162.7 µs 157.8 µs +3.15%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing crypto/modarith-compile-time-mod (7eb0a63) with master (9344010)

Open in CodSpeed

@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.01124% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.70%. Comparing base (9344010) to head (7eb0a63).

Files with missing lines Patch % Lines
test/unittests/modarith_test.cpp 84.00% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1664      +/-   ##
==========================================
- Coverage   97.72%   97.70%   -0.02%     
==========================================
  Files         171      171              
  Lines       15636    15631       -5     
  Branches     3616     3616              
==========================================
- Hits        15280    15272       -8     
- Misses        269      272       +3     
  Partials       87       87              
Flag Coverage Δ
eest-develop 88.54% <100.00%> (-0.06%) ⬇️
eest-develop-gmp 26.46% <30.33%> (-0.11%) ⬇️
eest-legacy 17.14% <0.00%> (+<0.01%) ⬆️
eest-libsecp256k1 28.81% <30.33%> (-0.03%) ⬇️
eest-stable 88.54% <100.00%> (-0.06%) ⬇️
evmone-unittests 93.42% <91.01%> (-0.03%) ⬇️

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

Components Coverage Δ
core 96.05% <100.00%> (-0.07%) ⬇️
tooling 91.92% <ø> (ø)
tests 99.80% <85.45%> (-0.01%) ⬇️
Files with missing lines Coverage Δ
lib/evmone_precompiles/ecc.hpp 97.24% <100.00%> (ø)
lib/evmone_precompiles/modarith.hpp 96.25% <100.00%> (-3.75%) ⬇️
lib/evmone_precompiles/secp256r1.cpp 100.00% <100.00%> (ø)
test/unittests/secp256k1_test.cpp 100.00% <100.00%> (ø)
test/unittests/modarith_test.cpp 88.73% <84.00%> (-0.31%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Every modulus in the codebase is a compile-time constant, and accepting
one at runtime was needed only for EVMMAX. Replace ModArith with
MontgomeryArith taking the modulus as a template parameter, which turns
the modulus, R² and N' into compile-time constants and the operations
into static functions.

The conversions are renamed to to_internal() and from_internal(),
because the Montgomery form is an implementation detail of this one
arithmetic rather than something its users need to name.

The curve code already used a constexpr ModArith, so its generated code
barely moves: GCC stops inlining one multiplication in the inner pairing
function, worth a fraction of a percent of its instructions. The
p256verify path did more work: it computed R² by division on every call.
The micro-benchmark also built the arithmetic at runtime, and its
modular multiplication gets up to ~30% faster.
@chfast
chfast force-pushed the crypto/modarith-compile-time-mod branch from 2cb6b33 to 7eb0a63 Compare August 19, 2026 11:45
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