Skip to content

Commit 4af241b

Browse files
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
2 parents f473c95 + 4d9645b commit 4af241b

File tree

4 files changed

+16
-36
lines changed

4 files changed

+16
-36
lines changed

.cirrus.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ env:
1010
MAKEFLAGS: -j4
1111
BUILD: check
1212
### secp256k1 config
13-
ECMULTWINDOW: auto
14-
ECMULTGENKB: auto
13+
ECMULTWINDOW: 15
14+
ECMULTGENKB: 22
1515
ASM: no
1616
WIDEMUL: auto
1717
WITH_VALGRIND: yes

.github/workflows/ci.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ env:
2121
MAKEFLAGS: '-j4'
2222
BUILD: 'check'
2323
### secp256k1 config
24-
ECMULTWINDOW: 'auto'
25-
ECMULTGENKB: 'auto'
24+
ECMULTWINDOW: 15
25+
ECMULTGENKB: 22
2626
ASM: 'no'
2727
WIDEMUL: 'auto'
2828
WITH_VALGRIND: 'yes'

CMakeLists.txt

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -91,21 +91,15 @@ if(SECP256K1_USE_EXTERNAL_DEFAULT_CALLBACKS)
9191
add_compile_definitions(USE_EXTERNAL_DEFAULT_CALLBACKS=1)
9292
endif()
9393

94-
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]")
95-
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)
94+
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]")
95+
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)
9696
include(CheckStringOptionValue)
9797
check_string_option_value(SECP256K1_ECMULT_WINDOW_SIZE)
98-
if(SECP256K1_ECMULT_WINDOW_SIZE STREQUAL "AUTO")
99-
set(SECP256K1_ECMULT_WINDOW_SIZE 15)
100-
endif()
10198
add_compile_definitions(ECMULT_WINDOW_SIZE=${SECP256K1_ECMULT_WINDOW_SIZE})
10299

103-
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]")
104-
set_property(CACHE SECP256K1_ECMULT_GEN_KB PROPERTY STRINGS "AUTO" 2 22 86)
100+
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]")
101+
set_property(CACHE SECP256K1_ECMULT_GEN_KB PROPERTY STRINGS 2 22 86)
105102
check_string_option_value(SECP256K1_ECMULT_GEN_KB)
106-
if(SECP256K1_ECMULT_GEN_KB STREQUAL "AUTO")
107-
set(SECP256K1_ECMULT_GEN_KB 22)
108-
endif()
109103
if(SECP256K1_ECMULT_GEN_KB EQUAL 2)
110104
add_compile_definitions(COMB_BLOCKS=2)
111105
add_compile_definitions(COMB_TEETH=5)

configure.ac

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -203,22 +203,22 @@ AC_ARG_WITH([test-override-wide-multiply], [] ,[set_widemul=$withval], [set_wide
203203
AC_ARG_WITH([asm], [AS_HELP_STRING([--with-asm=x86_64|arm32|no|auto],
204204
[assembly to use (experimental: arm32) [default=auto]])],[req_asm=$withval], [req_asm=auto])
205205

206-
AC_ARG_WITH([ecmult-window], [AS_HELP_STRING([--with-ecmult-window=SIZE|auto],
206+
AC_ARG_WITH([ecmult-window], [AS_HELP_STRING([--with-ecmult-window=SIZE],
207207
[window size for ecmult precomputation for verification, specified as integer in range [2..24].]
208208
[Larger values result in possibly better performance at the cost of an exponentially larger precomputed table.]
209209
[The table will store 2^(SIZE-1) * 64 bytes of data but can be larger in memory due to platform-specific padding and alignment.]
210210
[A window size larger than 15 will require you delete the prebuilt precomputed_ecmult.c file so that it can be rebuilt.]
211211
[For very large window sizes, use "make -j 1" to reduce memory use during compilation.]
212-
["auto" is a reasonable setting for desktop machines (currently 15). [default=auto]]
212+
[The default value is a reasonable setting for desktop machines (currently 15). [default=15]]
213213
)],
214-
[req_ecmult_window=$withval], [req_ecmult_window=auto])
214+
[set_ecmult_window=$withval], [set_ecmult_window=15])
215215

216-
AC_ARG_WITH([ecmult-gen-kb], [AS_HELP_STRING([--with-ecmult-gen-kb=2|22|86|auto],
216+
AC_ARG_WITH([ecmult-gen-kb], [AS_HELP_STRING([--with-ecmult-gen-kb=2|22|86],
217217
[The size of the precomputed table for signing in multiples of 1024 bytes (on typical platforms).]
218218
[Larger values result in possibly better signing/keygeneration performance at the cost of a larger table.]
219-
["auto" is a reasonable setting for desktop machines (currently 22). [default=auto]]
219+
[The default value is a reasonable setting for desktop machines (currently 22). [default=22]]
220220
)],
221-
[req_ecmult_gen_kb=$withval], [req_ecmult_gen_kb=auto])
221+
[set_ecmult_gen_kb=$withval], [set_ecmult_gen_kb=22])
222222

223223
AC_ARG_WITH([valgrind], [AS_HELP_STRING([--with-valgrind=yes|no|auto],
224224
[Build with extra checks for running inside Valgrind [default=auto]]
@@ -349,14 +349,7 @@ auto)
349349
;;
350350
esac
351351

352-
# Set ecmult window size
353-
if test x"$req_ecmult_window" = x"auto"; then
354-
set_ecmult_window=15
355-
else
356-
set_ecmult_window=$req_ecmult_window
357-
fi
358-
359-
error_window_size=['window size for ecmult precomputation not an integer in range [2..24] or "auto"']
352+
error_window_size=['window size for ecmult precomputation not an integer in range [2..24]']
360353
case $set_ecmult_window in
361354
''|*[[!0-9]]*)
362355
# no valid integer
@@ -371,13 +364,6 @@ case $set_ecmult_window in
371364
;;
372365
esac
373366

374-
# Set ecmult gen kb
375-
if test x"$req_ecmult_gen_kb" = x"auto"; then
376-
set_ecmult_gen_kb=22
377-
else
378-
set_ecmult_gen_kb=$req_ecmult_gen_kb
379-
fi
380-
381367
case $set_ecmult_gen_kb in
382368
2)
383369
SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DCOMB_BLOCKS=2 -DCOMB_TEETH=5"
@@ -389,7 +375,7 @@ case $set_ecmult_gen_kb in
389375
SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DCOMB_BLOCKS=43 -DCOMB_TEETH=6"
390376
;;
391377
*)
392-
AC_MSG_ERROR(['ecmult gen table size not 2, 22, 86 or "auto"'])
378+
AC_MSG_ERROR(['ecmult gen table size not 2, 22 or 86'])
393379
;;
394380
esac
395381

0 commit comments

Comments
 (0)