Page MenuHomePhabricator

[SECP256K1] Make scalar/field choice depend on C-detected __int128 availability
ClosedPublic

Authored by Fabien on Sep 29 2020, 07:44.

Details

Summary
This PR does two things:

    It removes the ability to select the 5x52 field with a 8x32 scalar,
or the 10x26 field with a 4x64 scalar. It's both 128-bit wide versions,
or neither.
    The choice is made automatically by the C code, unless overridden by
a USE_FORCE_WIDEMUL_INT{64,128} define (which is available through
configure with a hidden option
--with-test-override-wide-multiplication={auto,int64,int128}).

This reduces the reliance on autoconf for this performance-critical
configuration option, and also reduces the number of different
combinations to test.

This removes one theoretically useful combination: if you had x86_64 asm
but no __int128 support in your compiler, it was possible to use the
64-bit field before but the 32-bit scalar. I think this doesn't matter
as all compilers/systems that support (our) x86_64 asm also support
__int128. Furthermore, #767 will break this.

As an unexpected side effect, this also means the gen_context static
precomputation tool will now use __int128 based implementations when
available (which required an addition to the 5x52 field; see first
commit).

Backport of secp2561k PR793.

Depends on D7610.

Test Plan
cmake -GNinja ..
ninja check-secp256k1

cmake -GNinja .. -DSECP256K1_TEST_OVERRIDE_WIDE_MULTIPLY=int64
ninja check-secp256k1

cmake -GNinja .. -DSECP256K1_TEST_OVERRIDE_WIDE_MULTIPLY=int128
ninja check-secp256k1

../configure
make -j4 check

../configure --with-test-override-wide-multiply=int64
make -j4 check

../configure --with-test-override-wide-multiply=int128
make -j4 check

Run the Travis build.
https://travis-ci.org/github/Fabcien/secp256k1/builds/731196575

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Fabien requested review of this revision.Sep 29 2020, 07:44
Fabien planned changes to this revision.

[Bot Message]
One or more PR numbers were detected in the summary.
Links to those PRs have been inserted into the summary for reference.

This revision is now accepted and ready to land.Sep 29 2020, 09:19