Upstream PRs 1056, 1104, 1105, 1084, 1114, 1115 #236
Closed
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
This removes a check for $ac_cv_prog_cc_c89 which is set by AC_PROG_CC if defined(__STDC__) in the preprocessor. (Standard compliant compilers are supposed to define __STDC__ to 1 but the value is actually not checked here.) Unfortunately, MSVC doesn't define it, so configure fails for MSVC. This check is not very useful in practice. Over 30 years after C89 has been released, there are no C compilers out there that are not sufficiently compliant with C89 for the project. The only practically relevant case was that the check rejected C++ compilers. A different method to reject C++ compilers will be introduced in a later commit.
- Updated _gej_add_var, _gej_add_ge_var, _gej_add_zinv_var - 2 fewer _fe_negate in each method - Updated operation counts and standardize layout - Added internal benchmark for _gej_add_zinv_var - Update sage files (fixed by Tim Ruffing)
…ddition 2f984ff Save negations in var-time group addition (Peter Dettman) Pull request description: - Updated _gej_add_var, _gej_add_ge_var, _gej_add_zinv_var - 2 fewer _fe_negate in each method - Updated operation counts and standardize layout - Added internal benchmark for _gej_add_zinv_var benchmark_internal shows about 2% speedup in each method as a result (64bit). ACKs for top commit: real-or-random: ACK 2f984ff jonasnick: ACK 2f984ff Tree-SHA512: 01366fa23c83a8dd37c9a0a24e0acc53ce38a201607fe4da6672ea5618d82c62d1299f0e0aa50317883821539af739ea52b6561faff230c148e6fdc5bc5af30b
…BIT_ASM_CHECK` 7efc983 Fix the false positive of `SECP_64BIT_ASM_CHECK` (Sprite) Pull request description: I'm trying to compile this project for RISC-V architecture, and I encountered errors: ``` src/field_5x52_asm_impl.h:28:1: error: unknown register name '%r15' in 'asm' 28 | __asm__ __volatile__( | ^ src/field_5x52_asm_impl.h:28:1: error: unknown register name '%r14' in 'asm' src/field_5x52_asm_impl.h:28:1: error: unknown register name '%r13' in 'asm' src/field_5x52_asm_impl.h:28:1: error: unknown register name '%r12' in 'asm' src/field_5x52_asm_impl.h:28:1: error: unknown register name '%r11' in 'asm' src/field_5x52_asm_impl.h:28:1: error: unknown register name '%r10' in 'asm' src/field_5x52_asm_impl.h:28:1: error: unknown register name '%r9' in 'asm' src/field_5x52_asm_impl.h:28:1: error: unknown register name '%r8' in 'asm' src/field_5x52_asm_impl.h:28:1: error: unknown register name '%rdx' in 'asm' src/field_5x52_asm_impl.h:28:1: error: unknown register name '%rcx' in 'asm' src/field_5x52_asm_impl.h:28:1: error: unknown register name '%rax' in 'asm' src/field_5x52_asm_impl.h:28:1: error: output number 0 not directly addressable src/field_5x52_asm_impl.h: In function 'secp256k1_fe_sqr': src/field_5x52_asm_impl.h:298:1: error: unknown register name '%r15' in 'asm' 298 | __asm__ __volatile__( | ^ src/field_5x52_asm_impl.h:298:1: error: unknown register name '%r14' in 'asm' src/field_5x52_asm_impl.h:298:1: error: unknown register name '%r13' in 'asm' src/field_5x52_asm_impl.h:298:1: error: unknown register name '%r12' in 'asm' src/field_5x52_asm_impl.h:298:1: error: unknown register name '%r11' in 'asm' src/field_5x52_asm_impl.h:298:1: error: unknown register name '%r10' in 'asm' src/field_5x52_asm_impl.h:298:1: error: unknown register name '%r9' in 'asm' src/field_5x52_asm_impl.h:298:1: error: unknown register name '%r8' in 'asm' src/field_5x52_asm_impl.h:298:1: error: unknown register name '%rdx' in 'asm' src/field_5x52_asm_impl.h:298:1: error: unknown register name '%rcx' in 'asm' src/field_5x52_asm_impl.h:298:1: error: unknown register name '%rbx' in 'asm' src/field_5x52_asm_impl.h:298:1: error: unknown register name '%rax' in 'asm' src/field_5x52_asm_impl.h:298:1: error: output number 0 not directly addressable ``` After further investigation I found that for RISC-V, macro `USE_ASM_X86_64` was defined unexpectedly, and `checking for x86_64 assembly availability... yes` appeared in the compilation log file, which means `SECP_64BIT_ASM_CHECK` was not working as expected. For unknown reasons, `AC_COMPILE_IFELSE` does not check if `__asm__` can be compiled, and an example can verify this point: ```m4 AC_DEFUN([SECP_64BIT_ASM_CHECK],[ AC_MSG_CHECKING(for x86_64 assembly availability) AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ #include <stdint.h>]],[[ __asm__ __volatile__("this is obviously wrong"); ]])],[has_64bit_asm=yes],[has_64bit_asm=no]) AC_MSG_RESULT([$has_64bit_asm]) ]) ``` It always gives results: `checking for x86_64 assembly availability... yes` After testing, replacing `AC_COMPILE_IFELSE` with `AC_LINK_IFELSE` can correctly check if `__asm__` can be compiled and make the project able to compile for RISC-V. ACKs for top commit: real-or-random: ACK 7efc983 Tree-SHA512: 7318dd42004b2930cfcd6541c5a9ce0aa186e2179a668b76089a908bea8d9f70fcfdb264512f971e395a3ce9dc7f9ca24c8f3d46175cad2972a2d713f518ed85
libtool takes care of building both object versions, we just need to pick the right one to export symbols.
…raries 6f6cab9 abi: Don't export symbols in static Windows libraries (Cory Fields) Pull request description: For context, Bitcoin Core has recently merged [libbitcoin-kernel](bitcoin/bitcoin#24322), a small library that intends to eventually minimally encompass Core's validation engine. This kernel lib includes a static libsecp256k1. Without this change, because libsecp256k1.a ends up with exported symbols, we end up with libsecp256k1 symbols exported by our libbitcoin-kernel library (which causes unrelated problems not worth getting into here). libtool takes care of building both object versions, and it automatically builds objects for shared libs with -DDLL_EXPORT. We just need to opt-in to its functionality. I can't imagine this having any negative impact on any current statically-linking applications, if anything they'll just be a tiny bit smaller because they can now strip unused symbols. ACKs for top commit: real-or-random: utACK 6f6cab9 theuni: > Not sure what other changes made compilation on CI fail but Concept ACK [6f6cab9](bitcoin-core/secp256k1@6f6cab9). This should be entirely harmless. sipa: utACK 6f6cab9 laanwj: utACK 6f6cab9 Tree-SHA512: 39f240046639738f7a8c01068e728b2f9ceac2754cc4b0a5fa46c28f6f57a8c4124653b56dfbf5c13106b07c11ac599cc41b508e16862d539ce1af6c3365a205
This adds MSVC builds built on Linux using wine. This requires some settings of tools and flags because the autotools support for MSVC is naturally somewhat limited. The advantage of this approach is that it is compatible with our existing CI scripts, so there's no need to write a Windows CI script (in PowerShell or similar). If we want to test building and running on Windows native (e.g., as supported by Cirrus CI) we could still do this in the future. Another advantage of this approach is that contributors can simply use the docker image if they need a MSVC installation in a non-Windows environment. This commit also improves the Dockerfile by grouping RUN commands according to Docker docs: https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#run
This commit also raises the TEST_ITERS for wine tasks to the default. The overhead of wine is negligible, so we can certainly afford the same number of iterations as for native Linux tests.
49e2acd configure: Improve rationale for WERROR_CFLAGS (Tim Ruffing) 8dc4b03 ci: Add a C++ job that compiles the public headers without -fpermissive (Tim Ruffing) 51f296a ci: Run persistent wineserver to speed up wine (Tim Ruffing) 3fb3269 ci: Add 32-bit MinGW64 build (Tim Ruffing) 9efc2e5 ci: Add MSVC builds (Tim Ruffing) 2be6ba0 configure: Convince autotools to work with MSVC's archiver lib.exe (Tim Ruffing) bd81f41 schnorrsig bench: Suppress a stupid warning in MSVC (Tim Ruffing) 09f3d71 configure: Add a few CFLAGS for MSVC (Tim Ruffing) 3b4f3d0 build: Reject C++ compilers in the preprocessor (Tim Ruffing) 1cc0941 configure: Don't abort if the compiler does not define __STDC__ (Tim Ruffing) cca8cbb configure: Output message when checking for valgrind (Tim Ruffing) 1a6be57 bench: Make benchmarks compile on MSVC (Tim Ruffing) Pull request description: ACKs for top commit: jonasnick: ACK 49e2acd Tree-SHA512: 986c498fb218231fff3519167d34a92e11dea6a4383788a9723be105c20578cd483c6b06ba5686c6669e3a02cfeebc29b8e5f1428552ebf4ec67fa7a86957548
…HECK` after invalid scrach space check 1827c9b scratch_destroy: move VERIFY_CHECK after invalid scrach space check (siv2r) Pull request description: ACKs for top commit: sipa: utACK 1827c9b jonasnick: ACK 1827c9b Tree-SHA512: 5c4f97ca16f46380b30d1d077a54e25eab21efb10e0ce3ca27e7f73a2318d130f0f0773e26b2fdfc8f9d98c1334880f9d80a51b0941be3ba0af2b656b7c0ea7e
.cirrus.yml
Outdated
Comment on lines
260
to
263
| ECDH: yes | ||
| RECOVERY: yes | ||
| EXPERIMENTAL: yes | ||
| SCHNORRSIG: yes |
There was a problem hiding this comment.
We should probably enable all our modules here.
jonasnick
force-pushed
the
temp-merge-1115
branch
from
275e03f
to
9a98106
Compare
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#1056]: Save negations in var-time group addition
[bitcoin-core/secp256k1#1104]: Fix the false positive of
SECP_64BIT_ASM_CHECK[bitcoin-core/secp256k1#1105]: Don'''t export symbols in static libraries
[bitcoin-core/secp256k1#1084]: ci: Add MSVC builds
[bitcoin-core/secp256k1#1114]:
_scratch_destroy: moveVERIFY_CHECKafter invalid scrach space check[bitcoin-core/secp256k1#1115]: Fix sepc256k1 -> secp256k1 typo in group.h
This PR can be recreated with
./contrib/sync-upstream.sh range 43756da819a235d813e7ecd53eae6df073b53247.