Upstream PRs 1147, 1149, 1000, 1155, 1156 #240
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently CHECK is used only in test and bench mark files except for one usage in `ecmult_impl.h`. We would like to move the definition of CHECK out of `util.h` so that `util.h` no longer has a hard dependency on `stdio.h`. Done in preparation for moving the definition of `CHECK` as part of an effort to allow secp256k1 to be compiled to WASM as part of `rust-secp256k1`.
After this commit, int128.h and int128_impl.h are included as follows: - .c files which use int128 include int128_impl.h (after util.h) - .h files which use int128 include int128.h (after util.h) This list is exhaustive. util.h needs to included first because it sets up necessary #defines.
… the job outside of CI 4e54c03 ci: print env to allow reproducing the job outside of CI (Jonas Nick) Pull request description: Example output: ``` WERROR_CFLAGS="-Werror -pedantic-errors" MAKEFLAGS="-j4" BUILD="check" ECMULTWINDOW="auto" ECMULTGENPRECISION="auto" ASM="no" WIDEMUL="int64" WITH_VALGRIND="no" EXTRAFLAGS="" EXPERIMENTAL="no" ECDH="no" RECOVERY="yes" SCHNORRSIG="no" SECP256K1_TEST_ITERS="" BENCH="yes" SECP256K1_BENCH_ITERS="2" CTIMETEST="yes" EXAMPLES="yes" WRAPPER_CMD="" CC="gcc" AR="" NM="" HOST="" ./ci/cirrus.sh ``` ACKs for top commit: sipa: ACK 4e54c03 real-or-random: ACK bitcoin-core/secp256k1@4e54c03 Tree-SHA512: b74a8724e72b3de7884e4d93fe933dc5043aec37020672b7997a8faebda3b0cbbba1bca69c344109729261ab4a94e76f4eca0d8773dc101a443fdf9e0d7d54f5
…t file 6a965b6 Remove usage of CHECK from non-test file (Tobin C. Harding) Pull request description: Currently CHECK is used only in test and bench mark files except for one usage in `ecmult_impl.h`. We would like to move the definition of CHECK out of `util.h` so that `util.h` no longer has a hard dependency on `stdio.h`. Done as part of an effort to allow secp256k1 to be compiled to WASM as part of `rust-secp256k1`. ### Note to reviewers Please review carefully, I don't actually know if this patch is correct. Done while working on #1095. I'm happy to make any changes both in concept and execution - I'm super rusty at C programming. cc real-or-random ACKs for top commit: sipa: utACK 6a965b6 real-or-random: utACK 6a965b6 Tree-SHA512: 6bfb456bdb92a831acd3bc202607e80f6d0a194d6b2cf745c8eceb12ba675d03a319d6d105332b0cbca474e443969295e5a8e938635453e21e057d0ee597440b
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
Also add a corresponding CI job
99bd335 Make int128 overflow test use secp256k1_[ui]128_mul (Pieter Wuille) 3afce0a Avoid signed overflow in MSVC AMR64 secp256k1_mul128 (Pieter Wuille) 9b5f589 Heuristically decide whether to use int128_struct (Pieter Wuille) 63ff064 int128: Add test override for testing __(u)mulh on MSVC X64 (Tim Ruffing) f2b7e88 Add int128 randomized tests (Pieter Wuille) Pull request description: This is a follow-up to #1000: * Add randomized unit tests for int128 logic. * Add CI for the `_(u)mulh` code path (on non-ARM64 MSVC). * Add heuristic logic to enable int128_struct based arithmetic on 64-bit MSVC, or systems with pointers wider than 32 bits. * Fix signed overflow in ARM64 MSVC code. ACKs for top commit: roconnor-blockstream: utACK 99bd335 real-or-random: ACK 99bd335 tested this also on MSVC locally with the override, including all the benchmark binaries jonasnick: utACK 99bd335 Tree-SHA512: 5ea897362293b45a86650593e1fdc8c4004a1d9452eed2fa070d22dffc7ed7ca1ec50a4df61e3a33dbe35e08132ad9686286ac44af6742b32b82f11c9d3341c6
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[bitcoin-core/secp256k1#1147]: ci: print env to allow reproducing the job outside of CI
[bitcoin-core/secp256k1#1149]: Remove usage of CHECK from non-test file
[bitcoin-core/secp256k1#1000]: Synthetic int128 type.
[bitcoin-core/secp256k1#1155]: Add MSan CI jobs
[bitcoin-core/secp256k1#1156]: Followups to int128_struct arithmetic
This PR can be recreated with
./contrib/sync-upstream.sh -b sync-upstream range e40fd277b7a157e68576a457e2968f0adb2bbab1.