build: Rename arm to arm32 and check if it's really supported #1304

hebasto commented at 11:05 AM on May 11, 2023: member

Closes #1034.

Solves one item in #1235.

build: Rename `arm` to `arm32` e5cf4bf3ff

real-or-random added the label build on May 11, 2023

hebasto cross-referenced this on May 11, 2023 from issue build: Meta-issue for follow-ups to initial CMake merge (#1113) by hebasto

in build-aux/m4/bitcoin_secp.m4:16 in f3c5a634b7 outdated

   8 | @@ -9,6 +9,25 @@ AC_LINK_IFELSE([AC_LANG_PROGRAM([[
   9 |  AC_MSG_RESULT([$has_64bit_asm])
  10 |  ])
  11 |  
  12 | +AC_DEFUN([SECP_ARM32_ASM_CHECK], [
  13 | +  AC_MSG_CHECKING(for ARM32 assembly availability)
  14 | +  SECP_ARM32_ASM_CHECK_CFLAGS_saved_CFLAGS="$CFLAGS"
  15 | +  CFLAGS="-x assembler"
  16 | +  AC_COMPILE_IFELSE([[

real-or-random commented at 4:44 PM on May 11, 2023:

I think we want to need to run the linker here, because checking the compiler may not be enough when LTO is enabled, see #1104.

hebasto commented at 6:23 PM on May 11, 2023:

Updated.

real-or-random added the label needs-changelog on May 11, 2023

hebasto force-pushed on May 11, 2023

real-or-random approved

real-or-random commented at 9:20 AM on May 12, 2023: contributor

utACK d1831a23c0c3b40f784c0fbbf31297564ece569a

Ideally, someone would test this on ARM32. But I think it's not a strict requirement because it seems to work on CI.

Perhaps that PR is a good opportunity to rename check_64bit_assembly into check_x86_64_assembly (and same for related variables and functions)

cmake: Add `check_arm32_assembly` function ed4ba238e2

autotools: Add `SECP_ARM32_ASM_CHECK` macro 03246457a8

build: Rename `64bit` to `x86_64` c6bb29b303

hebasto force-pushed on May 12, 2023

hebasto commented at 9:54 AM on May 12, 2023: member

Perhaps that PR is a good opportunity to rename check_64bit_assembly into check_x86_64_assembly (and same for related variables and functions)

Done.

hebasto commented at 10:04 AM on May 12, 2023: member

Ideally, someone would test this on ARM32.

FWIW, here are my logs on a machine with

$ uname -m
armv7l

Autotools:

<details> <summary>Details</summary>

$ ./configure --with-asm=arm32 --enable-experimental
checking build system type... armv7l-unknown-linux-gnueabihf
checking host system type... armv7l-unknown-linux-gnueabihf
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a race-free mkdir -p... /usr/bin/mkdir -p
checking for gawk... no
checking for mawk... mawk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes
checking whether make supports nested variables... (cached) yes
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables... 
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether the compiler supports GNU C... yes
checking whether gcc accepts -g... yes
checking for gcc option to enable C11 features... none needed
checking whether gcc understands -c and -o together... yes
checking whether make supports the include directive... yes (GNU style)
checking dependency style of gcc... gcc3
checking dependency style of gcc... gcc3
checking for ar... ar
checking the archiver (ar) interface... ar
checking how to print strings... printf
checking for a sed that does not truncate output... /usr/bin/sed
checking for grep that handles long lines and -e... /usr/bin/grep
checking for egrep... /usr/bin/grep -E
checking for fgrep... /usr/bin/grep -F
checking for ld used by gcc... /usr/bin/ld
checking if the linker (/usr/bin/ld) is GNU ld... yes
checking for BSD- or MS-compatible name lister (nm)... /usr/bin/nm -B
checking the name lister (/usr/bin/nm -B) interface... BSD nm
checking whether ln -s works... yes
checking the maximum length of command line arguments... 1572864
checking how to convert armv7l-unknown-linux-gnueabihf file names to armv7l-unknown-linux-gnueabihf format... func_convert_file_noop
checking how to convert armv7l-unknown-linux-gnueabihf file names to toolchain format... func_convert_file_noop
checking for /usr/bin/ld option to reload object files... -r
checking for objdump... objdump
checking how to recognize dependent libraries... pass_all
checking for dlltool... no
checking how to associate runtime and link libraries... printf %s\n
checking for archiver [@FILE](/bitcoin-core-secp256k1/contributor/file/) support... @
checking for strip... strip
checking for ranlib... ranlib
checking command to parse /usr/bin/nm -B output from gcc object... ok
checking for sysroot... no
checking for a working dd... /usr/bin/dd
checking how to truncate binary pipes... /usr/bin/dd bs=4096 count=1
checking for mt... mt
checking if mt is a manifest tool... no
checking for stdio.h... yes
checking for stdlib.h... yes
checking for string.h... yes
checking for inttypes.h... yes
checking for stdint.h... yes
checking for strings.h... yes
checking for sys/stat.h... yes
checking for sys/types.h... yes
checking for unistd.h... yes
checking for dlfcn.h... yes
checking for objdir... .libs
checking if gcc supports -fno-rtti -fno-exceptions... no
checking for gcc option to produce PIC... -fPIC -DPIC
checking if gcc PIC flag -fPIC -DPIC works... yes
checking if gcc static flag -static works... yes
checking if gcc supports -c -o file.o... yes
checking if gcc supports -c -o file.o... (cached) yes
checking whether the gcc linker (/usr/bin/ld) supports shared libraries... yes
checking whether -lc should be explicitly linked in... no
checking dynamic linker characteristics... GNU/Linux ld.so
checking how to hardcode library paths into programs... immediate
checking whether stripping libraries is possible... yes
checking if libtool supports shared libraries... yes
checking whether to build shared libraries... yes
checking whether to build static libraries... yes
checking if gcc supports -Werror... yes
checking if gcc supports -std=c89 -pedantic -Wno-long-long -Wnested-externs -Wshadow -Wstrict-prototypes -Wundef... yes
checking if gcc supports -Wno-overlength-strings... yes
checking if gcc supports -Wall... yes
checking if gcc supports -Wno-unused-function... yes
checking if gcc supports -Wextra... yes
checking if gcc supports -Wcast-align... yes
checking if gcc supports -Wcast-align=strict... yes
checking if gcc supports -Wconditional-uninitialized... no
checking if gcc supports -Wreserved-identifier... no
checking if gcc supports -fvisibility=hidden... yes
checking for valgrind support... 
checking for ARM32 assembly availability... yes
configure: ******
configure: WARNING: experimental build
configure: Experimental features do not have stable APIs or properties, and may not be safe for production use.
configure: ******
checking that generated files are newer than configure... done
configure: creating ./config.status
config.status: creating Makefile
config.status: creating libsecp256k1.pc
config.status: executing depfiles commands
config.status: executing libtool commands

Build Options:
  with external callbacks = no
  with benchmarks         = yes
  with tests              = yes
  with ctime tests        = no
  with coverage           = no
  with examples           = no
  module ecdh             = yes
  module recovery         = no
  module extrakeys        = yes
  module schnorrsig       = yes

  asm                     = arm32
  ecmult window size      = 15
  ecmult gen prec. bits   = 4

  valgrind                = no
  CC                      = gcc
  CPPFLAGS                = 
  SECP_CFLAGS             = -O2  -std=c89 -pedantic -Wno-long-long -Wnested-externs -Wshadow -Wstrict-prototypes -Wundef -Wno-overlength-strings -Wall -Wno-unused-function -Wextra -Wcast-align -Wcast-align=strict -fvisibility=hidden 
  CFLAGS                  = -g -O2
  LDFLAGS                 =

</details>

CMake:

<details> <summary>Details</summary>

$ cmake -S . -B build -DSECP256K1_ASM=arm32 -DSECP256K1_EXPERIMENTAL=ON
-- The C compiler identification is GNU 11.3.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- The ASM compiler identification is GNU
-- Found assembler: /usr/bin/cc
-- Could NOT find Valgrind (missing: Valgrind_INCLUDE_DIR Valgrind_WORKS) 
-- Setting build type to "RelWithDebInfo" as none was specified
-- Performing Test C_SUPPORTS__PEDANTIC
-- Performing Test C_SUPPORTS__PEDANTIC - Success
-- Performing Test C_SUPPORTS__WALL
-- Performing Test C_SUPPORTS__WALL - Success
-- Performing Test C_SUPPORTS__WCAST_ALIGN
-- Performing Test C_SUPPORTS__WCAST_ALIGN - Success
-- Performing Test C_SUPPORTS__WCAST_ALIGN_STRICT
-- Performing Test C_SUPPORTS__WCAST_ALIGN_STRICT - Success
-- Performing Test C_SUPPORTS__WCONDITIONAL_UNINITIALIZED
-- Performing Test C_SUPPORTS__WCONDITIONAL_UNINITIALIZED - Failed
-- Performing Test C_SUPPORTS__WEXTRA
-- Performing Test C_SUPPORTS__WEXTRA - Success
-- Performing Test C_SUPPORTS__WNESTED_EXTERNS
-- Performing Test C_SUPPORTS__WNESTED_EXTERNS - Success
-- Performing Test C_SUPPORTS__WNO_LONG_LONG
-- Performing Test C_SUPPORTS__WNO_LONG_LONG - Success
-- Performing Test C_SUPPORTS__WNO_OVERLENGTH_STRINGS
-- Performing Test C_SUPPORTS__WNO_OVERLENGTH_STRINGS - Success
-- Performing Test C_SUPPORTS__WNO_UNUSED_FUNCTION
-- Performing Test C_SUPPORTS__WNO_UNUSED_FUNCTION - Success
-- Performing Test C_SUPPORTS__WRESERVED_IDENTIFIER
-- Performing Test C_SUPPORTS__WRESERVED_IDENTIFIER - Failed
-- Performing Test C_SUPPORTS__WSHADOW
-- Performing Test C_SUPPORTS__WSHADOW - Success
-- Performing Test C_SUPPORTS__WSTRICT_PROTOTYPES
-- Performing Test C_SUPPORTS__WSTRICT_PROTOTYPES - Success
-- Performing Test C_SUPPORTS__WUNDEF
-- Performing Test C_SUPPORTS__WUNDEF - Success


secp256k1 configure summary
===========================
Build artifacts:
  library type ........................ Shared
Optional modules:
  ECDH ................................ ON
  ECDSA pubkey recovery ............... OFF
  extrakeys ........................... ON
  schnorrsig .......................... ON
Parameters:
  ecmult window size .................. 15
  ecmult gen precision bits ........... 4
Optional features:
  assembly optimization ............... arm32
  external callbacks .................. OFF
Optional binaries:
  benchmark ........................... ON
  noverify_tests ...................... ON
  tests ............................... ON
  exhaustive tests .................... ON
  ctime_tests ......................... OFF
  examples ............................ OFF

Cross compiling ....................... FALSE
Valgrind .............................. OFF
Preprocessor defined macros ........... ENABLE_MODULE_ECDH=1 ENABLE_MODULE_SCHNORRSIG=1 ENABLE_MODULE_EXTRAKEYS=1 ECMULT_WINDOW_SIZE=15 ECMULT_GEN_PREC_BITS=4 USE_EXTERNAL_ASM=1
C compiler ............................ /usr/bin/cc
CFLAGS ................................ 
Compile options ....................... -pedantic -Wall -Wcast-align -Wcast-align=strict -Wextra -Wnested-externs -Wno-long-long -Wno-overlength-strings -Wno-unused-function -Wshadow -Wstrict-prototypes -Wundef
Build type:
 - CMAKE_BUILD_TYPE ................... RelWithDebInfo
 - CFLAGS ............................. -O2 -g 
 - LDFLAGS for executables ............ 
 - LDFLAGS for shared libraries ....... 


  ******
  WARNING: experimental build
  Experimental features do not have stable APIs or properties, and may not be safe for production use.
  ******

-- Configuring done
-- Generating done
-- Build files have been written to: /home/hebasto/secp256k1/build

</details>

real-or-random approved

real-or-random commented at 10:10 AM on May 12, 2023: contributor

ACK c6bb29b3037c6b5264f2d2916c5a2d38de25df19 tested on x86_64 but not on ARM

hebasto commented at 10:16 AM on May 12, 2023: member

FWIW, this PR can also be tested using cross compiling:

$ ./configure --host=arm-linux-gnueabihf --with-asm=arm32 --enable-experimental

or

$ cmake -S . -B build -DCMAKE_TOOLCHAIN_FILE=cmake/arm-linux-gnueabihf.toolchain.cmake -DSECP256K1_ASM=arm32 -DSECP256K1_EXPERIMENTAL=ON

UPD. It must fail for --host=aarch64-linux-gnu.

sipa commented at 10:54 AM on May 12, 2023: contributor

Concept ACK

real-or-random merged this on May 12, 2023

real-or-random closed this on May 12, 2023

hebasto deleted the branch on May 12, 2023

hebasto referenced this in commit 49c52ea2b1 on May 13, 2023

real-or-random removed the label needs-changelog on May 13, 2023

vmta referenced this in commit e1120c94a1 on Jun 4, 2023

sipa referenced this in commit 901336eee7 on Jun 21, 2023

vmta referenced this in commit 8f03457eed on Jul 1, 2023

jonasnick cross-referenced this on Jul 24, 2023 from issue Upstream PRs 1268, 1276, 1267, 1265, 1230, 1279, 1273, 1263, 1231, 1285, 1283, 1205, 1286, 1275, 1234, 1239, 1240, 1284, 1277, 1289, 1270, 1296, 1301, 1299, 1066, 1300, 1292, 1305, 1303, 1133, 1306, 1207, 1304, 1307, 1311, 1309, 1312 by jonasnick