Skip to content

Commit

Permalink
Merge bitcoin-core/secp256k1#1000: Synthetic int128 type.
Browse files Browse the repository at this point in the history
a340d95 ci: add int128_struct tests (Jonas Nick)
dceaa1f int128: Tidy #includes of int128.h and int128_impl.h (Tim Ruffing)
2914bcc Simulated int128 type. (Russell O'Connor)

Pull request description:

  Abstracts the int128 type and provides an native version, if available, or a implements it using a pair of int64_t's.

  This is activated by setting the configuration flag `--with-test-override-wide-multiply=int128_struct`.

  The primary purpose of this PR is to take advantage of MSVC's [umulh](https://docs.microsoft.com/en-us/cpp/intrinsics/umulh?view=msvc-170) intrinsic that we can use to simulate an int128 type which MSVC does not have (AFAIU). This PR lays out the groundwork for this level of MSVC support, but doesn't include the configuration logic to enable it yet.

  For completeness, and implementation of `umulh` and `mulh` are also provided for compilers that support neither the intrinsic nor the int128 type (such as CompCert?).  This also opens up the possibility of removing the 32-bit field and scalar implementations should that ever be desired.

ACKs for top commit:
  sipa:
    ACK a340d95
  jonasnick:
    ACK a340d95

Tree-SHA512: b4f2853fa3ab60ce9d77b4eaee1fd20c4b612850e19fcb3179d7e36986f420c6c4589ff72f0cf844f989584ace49a1cd23cca3f4e405dabefc8da647a0df679d
  • Loading branch information
real-or-random committed Nov 16, 2022
2 parents 86e3b38 + a340d95 commit ddf2b29
Show file tree
Hide file tree
Showing 18 changed files with 814 additions and 297 deletions.
17 changes: 10 additions & 7 deletions .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ task:
- env: {WIDEMUL: int64, RECOVERY: yes}
- env: {WIDEMUL: int64, ECDH: yes, SCHNORRSIG: yes}
- env: {WIDEMUL: int128}
- env: {WIDEMUL: int128_struct}
- env: {WIDEMUL: int128, RECOVERY: yes, SCHNORRSIG: yes}
- env: {WIDEMUL: int128, ECDH: yes, SCHNORRSIG: yes}
- env: {WIDEMUL: int128, ASM: x86_64}
Expand Down Expand Up @@ -271,20 +272,22 @@ task:
EXPERIMENTAL: yes
SCHNORRSIG: yes
CTIMETEST: no
# Use a MinGW-w64 host to tell ./configure we're building for Windows.
# This will detect some MinGW-w64 tools but then make will need only
# the MSVC tools CC, AR and NM as specified below.
HOST: x86_64-w64-mingw32
CC: /opt/msvc/bin/x64/cl
AR: /opt/msvc/bin/x64/lib
NM: /opt/msvc/bin/x64/dumpbin -symbols -headers
# Set non-essential options that affect the CLI messages here.
# (They depend on the user's taste, so we don't want to set them automatically in configure.ac.)
CFLAGS: -nologo -diagnostics:caret
LDFLAGS: -XCClinker -nologo -XCClinker -diagnostics:caret
# Use a MinGW-w64 host to tell ./configure we're building for Windows.
# This will detect some MinGW-w64 tools but then make will need only
# the MSVC tools CC, AR and NM as specified below.
matrix:
- name: "x86_64 (MSVC): Windows (Debian stable, Wine)"
- name: "x86_64 (MSVC): Windows (Debian stable, Wine, int128_struct)"
env:
HOST: x86_64-w64-mingw32
CC: /opt/msvc/bin/x64/cl
AR: /opt/msvc/bin/x64/lib
NM: /opt/msvc/bin/x64/dumpbin -symbols -headers
WIDEMUL: int128_struct
- name: "i686 (MSVC): Windows (Debian stable, Wine)"
env:
HOST: i686-w64-mingw32
Expand Down
6 changes: 6 additions & 0 deletions Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ noinst_HEADERS += src/precomputed_ecmult.h
noinst_HEADERS += src/precomputed_ecmult_gen.h
noinst_HEADERS += src/assumptions.h
noinst_HEADERS += src/util.h
noinst_HEADERS += src/int128.h
noinst_HEADERS += src/int128_impl.h
noinst_HEADERS += src/int128_native.h
noinst_HEADERS += src/int128_native_impl.h
noinst_HEADERS += src/int128_struct.h
noinst_HEADERS += src/int128_struct_impl.h
noinst_HEADERS += src/scratch.h
noinst_HEADERS += src/scratch_impl.h
noinst_HEADERS += src/selftest.h
Expand Down
9 changes: 8 additions & 1 deletion configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,11 @@ AC_ARG_ENABLE(external_default_callbacks,
[SECP_SET_DEFAULT([enable_external_default_callbacks], [no], [no])])

# Test-only override of the (autodetected by the C code) "widemul" setting.
# Legal values are int64 (for [u]int64_t), int128 (for [unsigned] __int128), and auto (the default).
# Legal values are:
# * int64 (for [u]int64_t),
# * int128 (for [unsigned] __int128),
# * int128_struct (for int128 implemented as a structure),
# * and auto (the default).
AC_ARG_WITH([test-override-wide-multiply], [] ,[set_widemul=$withval], [set_widemul=auto])

AC_ARG_WITH([asm], [AS_HELP_STRING([--with-asm=x86_64|arm|no|auto],
Expand Down Expand Up @@ -285,6 +289,9 @@ fi

# Select wide multiplication implementation
case $set_widemul in
int128_struct)
AC_DEFINE(USE_FORCE_WIDEMUL_INT128_STRUCT, 1, [Define this symbol to force the use of the structure for simulating (unsigned) int128 based wide multiplication])
;;
int128)
AC_DEFINE(USE_FORCE_WIDEMUL_INT128, 1, [Define this symbol to force the use of the (unsigned) __int128 based wide multiplication implementation])
;;
Expand Down
7 changes: 5 additions & 2 deletions src/assumptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
#include <limits.h>

#include "util.h"
#if defined(SECP256K1_INT128_NATIVE)
#include "int128_native.h"
#endif

/* This library, like most software, relies on a number of compiler implementation defined (but not undefined)
behaviours. Although the behaviours we require are essentially universal we test them specifically here to
Expand Down Expand Up @@ -55,7 +58,7 @@ struct secp256k1_assumption_checker {

/* To int64_t. */
((int64_t)(uint64_t)0xB123C456D789E012ULL == (int64_t)-(int64_t)0x4EDC3BA928761FEEULL) &&
#if defined(SECP256K1_WIDEMUL_INT128)
#if defined(SECP256K1_INT128_NATIVE)
((int64_t)(((uint128_t)0xA1234567B8901234ULL << 64) + 0xC5678901D2345678ULL) == (int64_t)-(int64_t)0x3A9876FE2DCBA988ULL) &&
(((int64_t)(int128_t)(((uint128_t)0xB1C2D3E4F5A6B7C8ULL << 64) + 0xD9E0F1A2B3C4D5E6ULL)) == (int64_t)(uint64_t)0xD9E0F1A2B3C4D5E6ULL) &&
(((int64_t)(int128_t)(((uint128_t)0xABCDEF0123456789ULL << 64) + 0x0123456789ABCDEFULL)) == (int64_t)(uint64_t)0x0123456789ABCDEFULL) &&
Expand All @@ -71,7 +74,7 @@ struct secp256k1_assumption_checker {
((((int16_t)0xE9AC) >> 4) == (int16_t)(uint16_t)0xFE9A) &&
((((int32_t)0x937C918A) >> 9) == (int32_t)(uint32_t)0xFFC9BE48) &&
((((int64_t)0xA8B72231DF9CF4B9ULL) >> 19) == (int64_t)(uint64_t)0xFFFFF516E4463BF3ULL) &&
#if defined(SECP256K1_WIDEMUL_INT128)
#if defined(SECP256K1_INT128_NATIVE)
((((int128_t)(((uint128_t)0xCD833A65684A0DBCULL << 64) + 0xB349312F71EA7637ULL)) >> 39) == (int128_t)(((uint128_t)0xFFFFFFFFFF9B0674ULL << 64) + 0xCAD0941B79669262ULL)) &&
#endif
1) * 2 - 1];
Expand Down
Loading

0 comments on commit ddf2b29

Please sign in to comment.