Merge #1535: build: Replace hardcoded "auto" value with default one

4d9645b cmake: Remove "AUTO" value of `SECP256K1_ECMULT_GEN_KB` option (Hennadii Stepanov) a06805e cmake: Remove "AUTO" value of `SECP256K1_ECMULT_WINDOW_SIZE` option (Hennadii Stepanov) 26b94ee autotools: Remove "auto" value of `--with-ecmult-gen-kb` option (Hennadii Stepanov) 122dbae autotools: Remove "auto" value of `--with-ecmult-window` option (Hennadii Stepanov) Pull request description: "auto" implies that a value is being chosen based on build system introspection or host system capabilities. However, for the `--with-ecmult-window` and `--with-ecmult-gen-kb` options, the values "auto" are hardcoded, which might lead to confusion. This PR replaces "auto" with more appropriate default values. If Concept ACKed, I'll add equivalent commits for CMake. ACKs for top commit: sipa: utACK 4d9645b real-or-random: utACK 4d9645b good from my side, but let's see if we can get more (Concept) ACKs Tree-SHA512: 9e68f73682c5310c68d2337594f13b99a52bfc365564e39df2e412b576635c90cccd2298406a4281f014916c4a1710e19c7390f05a4b0acbd09869bfb56f36ac
bitcoin-core · Jun 25, 2024 · 4af241b · 4af241b
2 parents f473c95 + 4d9645b
commit 4af241b
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 36 deletions.
diff --git a/.cirrus.yml b/.cirrus.yml
@@ -10,8 +10,8 @@ env:
   MAKEFLAGS: -j4
   BUILD: check
   ### secp256k1 config
-  ECMULTWINDOW: auto
-  ECMULTGENKB: auto
+  ECMULTWINDOW: 15
+  ECMULTGENKB: 22
   ASM: no
   WIDEMUL: auto
   WITH_VALGRIND: yes

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -21,8 +21,8 @@ env:
   MAKEFLAGS: '-j4'
   BUILD: 'check'
   ### secp256k1 config
-  ECMULTWINDOW: 'auto'
-  ECMULTGENKB: 'auto'
+  ECMULTWINDOW: 15
+  ECMULTGENKB: 22
   ASM: 'no'
   WIDEMUL: 'auto'
   WITH_VALGRIND: 'yes'

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -91,21 +91,15 @@ if(SECP256K1_USE_EXTERNAL_DEFAULT_CALLBACKS)
   add_compile_definitions(USE_EXTERNAL_DEFAULT_CALLBACKS=1)
 endif()
 
-set(SECP256K1_ECMULT_WINDOW_SIZE "AUTO" CACHE STRING "Window size for ecmult precomputation for verification, specified as integer in range [2..24]. \"AUTO\" is a reasonable setting for desktop machines (currently 15). [default=AUTO]")
-set_property(CACHE SECP256K1_ECMULT_WINDOW_SIZE PROPERTY STRINGS "AUTO" 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24)
+set(SECP256K1_ECMULT_WINDOW_SIZE 15 CACHE STRING "Window size for ecmult precomputation for verification, specified as integer in range [2..24]. The default value is a reasonable setting for desktop machines (currently 15). [default=15]")
+set_property(CACHE SECP256K1_ECMULT_WINDOW_SIZE PROPERTY STRINGS 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24)
 include(CheckStringOptionValue)
 check_string_option_value(SECP256K1_ECMULT_WINDOW_SIZE)
-if(SECP256K1_ECMULT_WINDOW_SIZE STREQUAL "AUTO")
-  set(SECP256K1_ECMULT_WINDOW_SIZE 15)
-endif()
 add_compile_definitions(ECMULT_WINDOW_SIZE=${SECP256K1_ECMULT_WINDOW_SIZE})
 
-set(SECP256K1_ECMULT_GEN_KB "AUTO" CACHE STRING "The size of the precomputed table for signing in multiples of 1024 bytes (on typical platforms). Larger values result in possibly better signing or key generation performance at the cost of a larger table. Valid choices are 2, 22, 86. \"AUTO\" is a reasonable setting for desktop machines (currently 22). [default=AUTO]")
-set_property(CACHE SECP256K1_ECMULT_GEN_KB PROPERTY STRINGS "AUTO" 2 22 86)
+set(SECP256K1_ECMULT_GEN_KB 22 CACHE STRING "The size of the precomputed table for signing in multiples of 1024 bytes (on typical platforms). Larger values result in possibly better signing or key generation performance at the cost of a larger table. Valid choices are 2, 22, 86. The default value is a reasonable setting for desktop machines (currently 22). [default=22]")
+set_property(CACHE SECP256K1_ECMULT_GEN_KB PROPERTY STRINGS 2 22 86)
 check_string_option_value(SECP256K1_ECMULT_GEN_KB)
-if(SECP256K1_ECMULT_GEN_KB STREQUAL "AUTO")
-  set(SECP256K1_ECMULT_GEN_KB 22)
-endif()
 if(SECP256K1_ECMULT_GEN_KB EQUAL 2)
   add_compile_definitions(COMB_BLOCKS=2)
   add_compile_definitions(COMB_TEETH=5)

diff --git a/configure.ac b/configure.ac
@@ -203,22 +203,22 @@ AC_ARG_WITH([test-override-wide-multiply], [] ,[set_widemul=$withval], [set_wide
 AC_ARG_WITH([asm], [AS_HELP_STRING([--with-asm=x86_64|arm32|no|auto],
 [assembly to use (experimental: arm32) [default=auto]])],[req_asm=$withval], [req_asm=auto])
 
-AC_ARG_WITH([ecmult-window], [AS_HELP_STRING([--with-ecmult-window=SIZE|auto],
+AC_ARG_WITH([ecmult-window], [AS_HELP_STRING([--with-ecmult-window=SIZE],
 [window size for ecmult precomputation for verification, specified as integer in range [2..24].]
 [Larger values result in possibly better performance at the cost of an exponentially larger precomputed table.]
 [The table will store 2^(SIZE-1) * 64 bytes of data but can be larger in memory due to platform-specific padding and alignment.]
 [A window size larger than 15 will require you delete the prebuilt precomputed_ecmult.c file so that it can be rebuilt.]
 [For very large window sizes, use "make -j 1" to reduce memory use during compilation.]
-["auto" is a reasonable setting for desktop machines (currently 15). [default=auto]]
+[The default value is a reasonable setting for desktop machines (currently 15). [default=15]]
 )],
-[req_ecmult_window=$withval], [req_ecmult_window=auto])
+[set_ecmult_window=$withval], [set_ecmult_window=15])
 
-AC_ARG_WITH([ecmult-gen-kb], [AS_HELP_STRING([--with-ecmult-gen-kb=2|22|86|auto],
+AC_ARG_WITH([ecmult-gen-kb], [AS_HELP_STRING([--with-ecmult-gen-kb=2|22|86],
 [The size of the precomputed table for signing in multiples of 1024 bytes (on typical platforms).]
 [Larger values result in possibly better signing/keygeneration performance at the cost of a larger table.]
-["auto" is a reasonable setting for desktop machines (currently 22). [default=auto]]
+[The default value is a reasonable setting for desktop machines (currently 22). [default=22]]
 )],
-[req_ecmult_gen_kb=$withval], [req_ecmult_gen_kb=auto])
+[set_ecmult_gen_kb=$withval], [set_ecmult_gen_kb=22])
 
 AC_ARG_WITH([valgrind], [AS_HELP_STRING([--with-valgrind=yes|no|auto],
 [Build with extra checks for running inside Valgrind [default=auto]]
@@ -349,14 +349,7 @@ auto)
   ;;
 esac
 
-# Set ecmult window size
-if test x"$req_ecmult_window" = x"auto"; then
-  set_ecmult_window=15
-else
-  set_ecmult_window=$req_ecmult_window
-fi
-
-error_window_size=['window size for ecmult precomputation not an integer in range [2..24] or "auto"']
+error_window_size=['window size for ecmult precomputation not an integer in range [2..24]']
 case $set_ecmult_window in
 ''|*[[!0-9]]*)
   # no valid integer
@@ -371,13 +364,6 @@ case $set_ecmult_window in
   ;;
 esac
 
-# Set ecmult gen kb
-if test x"$req_ecmult_gen_kb" = x"auto"; then
-  set_ecmult_gen_kb=22
-else
-  set_ecmult_gen_kb=$req_ecmult_gen_kb
-fi
-
 case $set_ecmult_gen_kb in
 2)
   SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DCOMB_BLOCKS=2 -DCOMB_TEETH=5"
@@ -389,7 +375,7 @@ case $set_ecmult_gen_kb in
   SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DCOMB_BLOCKS=43 -DCOMB_TEETH=6"
   ;;
 *)
-  AC_MSG_ERROR(['ecmult gen table size not 2, 22, 86 or "auto"'])
+  AC_MSG_ERROR(['ecmult gen table size not 2, 22 or 86'])
   ;;
 esac