Skip to content

[backport] Various secp256k1 ASM-related (optional backport)

Calin Culianu requested to merge cculianu/bitcoin-cash-node:backport_D5480_D5542_D5521_D5590 into master

Summary

This is a backport of: D5480 D5542 D5521 D5590 -- all seckp256k1 build-related.

I'm not entirely sure what the point of these changes is. I have decided to add them to keep the SECP256K1 cmake files in synch with ABC so we don't have to worry too much in the future. However, if you guys feel this set of commits is useless, feel free to say so and we can close this.

I'll be honest: I don't understand what these do. I guess on some architectures it used to attempt to force ASM to ON even if not supported, and would require -DSECP256K1_USE_ASM=OFF to compile -- whereas with this it "just works"?

I don't even understand the test plan.

Anyway below are the original messages in ascending commit order.

Original messages

[SECP256K1] CMake: Build the ARM ASM field implementation

Summary: This diff allow cmake to use the ARM optimized assembly (equivalent to autotools --with-asm=arm).

It attempts to guess the build target, and default with an error if assembly is enabled but unsupported by the target (or the target is not known). This makes the build to succeed by default on x86_64, and on ARM when the toolchain file is used, so no user intervention is needed for the "normal" path.

Depends on D5501.

Test Plan: Should ask to use -DSECP256K1_USE_ASM=OFF:

cmake -GNinja .. -DCMAKE_TOOLCHAIN_FILE=../cmake/platforms/Linux32.cmake

Should succeed:

cmake -GNinja .. -DCMAKE_TOOLCHAIN_FILE=../cmake/platforms/LinuxARM.cmake
ninja

Run the test binaries on an ARM target and check there is no error (tested on a RPi).

Run the build on OSX.

Run the Gitian builds.

Run the Travis build (see https://travis-ci.org/github/Fabcien/secp256k1/builds/663863958).

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Subscribers: deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D5480

[SECP256K1] Disable ASM for native executables

Summary: If the platforms is detected x86_64 and the ASM check fails, the native executable will fail as well even if -DSECP256K1_USE_ASM=OFF is passed to cmake. This diff disables the ASM for the native executables and get rids of the issue.

Got the issue on FreeBSD 12.0.

Test Plan:

ninja check

Build on FreeBSD 12.0.

Reviewers: #bitcoin_abc, jasonbcox

Reviewed By: #bitcoin_abc, jasonbcox

Differential Revision: https://reviews.bitcoinabc.org/D5542

[cmake] Fix typo in error message

Summary: cmake variable are case sensitive.

Test Plan: build on linux32 and check it actually build with the flag provided by the error message

Reviewers: #bitcoin_abc, jasonbcox

Reviewed By: #bitcoin_abc, jasonbcox

Differential Revision: https://reviews.bitcoinabc.org/D5521

[SECP256K1] Turn off ASM by default on target with no ASM support

Summary: This will drop the need to set -DSECP256K1_USE_ASM=OFF for targets with no support for ASM in secp256k1.

Test Plan: Run cmake on various platforms and check the ASM is enabled/disabled as needed.

Run the Gitian builds.

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Subscribers: deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D5590

Edited by Calin Culianu

Merge request reports