Merge bitcoin-core/secp256k1#1047: ci: Various improvements

b4ac1a1 ci: Run valgrind/memcheck tasks with 2 CPUs (Tim Ruffing) e70acab ci: Use Cirrus "greedy" flag to use idle CPU time when available (Tim Ruffing) d07e301 ci: Update brew on macOS (Tim Ruffing) 22382f0 ci: Test different ecmult window sizes (Tim Ruffing) 26a022a ci: Remove STATICPRECOMPUTATION (Tim Ruffing) 10461d8 precompute_ecmult: Always compute all tables up to default WINDOW_G (Tim Ruffing) Pull request description: ACKs for top commit: elichai: utACK b4ac1a1 jonasnick: ACK b4ac1a1 Tree-SHA512: b283d7b1c72cf87484de1fe98318298698fe9982dc33389eaca62e92318ab0074c183b9799add274f46358032491fee875e5ffb2a76a47f3b07520b850f4c85e
BlockstreamResearch · Dec 22, 2021 · 09971a3 · 09971a3
2 parents 0b83b20 + b4ac1a1
commit 09971a3
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 74 deletions.
diff --git a/.cirrus.yml b/.cirrus.yml
@@ -4,10 +4,10 @@ env:
   # Specific warnings can be disabled with -Wno-error=foo.
   # -pedantic-errors is not equivalent to -Werror=pedantic and thus not implied by -Werror according to the GCC manual.
   WERROR_CFLAGS: -Werror -pedantic-errors
-  MAKEFLAGS: -j2
+  MAKEFLAGS: -j4
   BUILD: check
   ### secp256k1 config
-  STATICPRECOMPUTATION: yes
+  ECMULTWINDOW: auto
   ECMULTGENPRECISION: auto
   ASM: no
   WIDEMUL: auto
@@ -50,14 +50,19 @@ merge_base_script_snippet: &MERGE_BASE
     - git config --global user.name "ci"
     - git merge FETCH_HEAD  # Merge base to detect silent merge conflicts
 
-task:
-  name: "x86_64: Linux (Debian stable)"
+linux_container_snippet: &LINUX_CONTAINER
   container:
     dockerfile: ci/linux-debian.Dockerfile
     # Reduce number of CPUs to be able to do more builds in parallel.
     cpu: 1
+    # Gives us more CPUs for free if they're available.
+    greedy: true
     # More than enough for our scripts.
     memory: 1G
+
+task:
+  name: "x86_64: Linux (Debian stable)"
+  << : *LINUX_CONTAINER
   matrix: &ENV_MATRIX
     - env: {WIDEMUL:  int64,  RECOVERY: yes}
     - env: {WIDEMUL:  int64,                 ECDH: yes, EXPERIMENTAL: yes, SCHNORRSIG: yes}
@@ -66,12 +71,11 @@ task:
     - env: {WIDEMUL: int128,                 ECDH: yes, EXPERIMENTAL: yes, SCHNORRSIG: yes}
     - env: {WIDEMUL: int128,  ASM: x86_64}
     - env: {                  RECOVERY: yes,            EXPERIMENTAL: yes, SCHNORRSIG: yes}
-    - env: {                  STATICPRECOMPUTATION: no}
     - env: {BUILD: distcheck, WITH_VALGRIND: no, CTIMETEST: no, BENCH: no}
     - env: {CPPFLAGS: -DDETERMINISTIC}
     - env: {CFLAGS: -O0, CTIMETEST: no}
-    - env: { ECMULTGENPRECISION: 2 }
-    - env: { ECMULTGENPRECISION: 8 }
+    - env: { ECMULTGENPRECISION: 2, ECMULTWINDOW: 2 }
+    - env: { ECMULTGENPRECISION: 8, ECMULTWINDOW: 4 }
   matrix:
     - env:
         CC: gcc
@@ -84,10 +88,7 @@ task:
 
 task:
   name: "i686: Linux (Debian stable)"
-  container:
-    dockerfile: ci/linux-debian.Dockerfile
-    cpu: 1
-    memory: 1G
+  << : *LINUX_CONTAINER
   env:
     HOST: i686-linux-gnu
     ECDH: yes
@@ -134,8 +135,9 @@ task:
   ##   - rm /tmp/.com.apple.dt.CommandLineTools.installondemand.in-progress
   ##
   brew_valgrind_pre_script:
+    - brew update
     - brew config
-    - brew tap --shallow LouisBrunner/valgrind
+    - brew tap LouisBrunner/valgrind
     # Fetch valgrind source but don't build it yet.
     - brew fetch --HEAD LouisBrunner/valgrind/valgrind
   brew_valgrind_cache:
@@ -165,10 +167,7 @@ task:
 
 task:
   name: "s390x (big-endian): Linux (Debian stable, QEMU)"
-  container:
-    dockerfile: ci/linux-debian.Dockerfile
-    cpu: 1
-    memory: 1G
+  << : *LINUX_CONTAINER
   env:
     WRAPPER_CMD: qemu-s390x
     SECP256K1_TEST_ITERS: 16
@@ -188,10 +187,7 @@ task:
 
 task:
   name: "ARM32: Linux (Debian stable, QEMU)"
-  container:
-    dockerfile: ci/linux-debian.Dockerfile
-    cpu: 1
-    memory: 1G
+  << : *LINUX_CONTAINER
   env:
     WRAPPER_CMD: qemu-arm
     SECP256K1_TEST_ITERS: 16
@@ -212,10 +208,7 @@ task:
 
 task:
   name: "ARM64: Linux (Debian stable, QEMU)"
-  container:
-    dockerfile: ci/linux-debian.Dockerfile
-    cpu: 1
-    memory: 1G
+  << : *LINUX_CONTAINER
   env:
     WRAPPER_CMD: qemu-aarch64
     SECP256K1_TEST_ITERS: 16
@@ -233,10 +226,7 @@ task:
 
 task:
   name: "ppc64le: Linux (Debian stable, QEMU)"
-  container:
-    dockerfile: ci/linux-debian.Dockerfile
-    cpu: 1
-    memory: 1G
+  << : *LINUX_CONTAINER
   env:
     WRAPPER_CMD: qemu-ppc64le
     SECP256K1_TEST_ITERS: 16
@@ -254,10 +244,7 @@ task:
 
 task:
   name: "x86_64 (mingw32-w64): Windows (Debian stable, Wine)"
-  container:
-    dockerfile: ci/linux-debian.Dockerfile
-    cpu: 1
-    memory: 1G
+  << : *LINUX_CONTAINER
   env:
     WRAPPER_CMD: wine64-stable
     SECP256K1_TEST_ITERS: 16
@@ -275,10 +262,7 @@ task:
 
 # Sanitizers
 task:
-  container:
-    dockerfile: ci/linux-debian.Dockerfile
-    cpu: 1
-    memory: 2G
+  << : *LINUX_CONTAINER
   env:
     ECDH: yes
     RECOVERY: yes
@@ -287,11 +271,15 @@ task:
     CTIMETEST: no
   matrix:
     - name: "Valgrind (memcheck)"
+      container:
+        cpu: 2
       env:
         # The `--error-exitcode` is required to make the test fail if valgrind found errors, otherwise it'll return 0 (https://www.valgrind.org/docs/manual/manual-core.html)
         WRAPPER_CMD: "valgrind --error-exitcode=42"
         SECP256K1_TEST_ITERS: 2
     - name: "UBSan, ASan, LSan"
+      container:
+        memory: 2G
       env:
         CFLAGS: "-fsanitize=undefined,address -g"
         UBSAN_OPTIONS: "print_stacktrace=1:halt_on_error=1"
@@ -302,11 +290,10 @@ task:
   matrix:
     - env:
         ASM: auto
-        STATICPRECOMPUTATION: yes
     - env:
         ASM: no
-        STATICPRECOMPUTATION: no
         ECMULTGENPRECISION: 2
+        ECMULTWINDOW: 2
   matrix:
     - env:
         CC: clang
@@ -320,15 +307,12 @@ task:
 
 task:
   name: "C++ -fpermissive"
-  container:
-    dockerfile: ci/linux-debian.Dockerfile
-    cpu: 1
-    memory: 1G
+  << : *LINUX_CONTAINER
   env:
     # ./configure correctly errors out when given CC=g++.
     # We hack around this by passing CC=g++ only to make.
     CC: gcc
-    MAKEFLAGS: -j2 CC=g++ CFLAGS=-fpermissive\ -g
+    MAKEFLAGS: -j4 CC=g++ CFLAGS=-fpermissive\ -g
     WERROR_CFLAGS:
     EXPERIMENTAL: yes
     ECDH: yes

diff --git a/ci/cirrus.sh b/ci/cirrus.sh
@@ -15,7 +15,8 @@ valgrind --version || true
 ./configure \
     --enable-experimental="$EXPERIMENTAL" \
     --with-test-override-wide-multiply="$WIDEMUL" --with-asm="$ASM" \
-    --enable-ecmult-static-precomputation="$STATICPRECOMPUTATION" --with-ecmult-gen-precision="$ECMULTGENPRECISION" \
+    --with-ecmult-window="$ECMULTWINDOW" \
+    --with-ecmult-gen-precision="$ECMULTGENPRECISION" \
     --enable-module-ecdh="$ECDH" --enable-module-recovery="$RECOVERY" \
     --enable-module-schnorrsig="$SCHNORRSIG" \
     --with-valgrind="$WITH_VALGRIND" \

diff --git a/src/precompute_ecmult.c b/src/precompute_ecmult.c
@@ -32,7 +32,7 @@ static void print_table(FILE *fp, const char *name, int window_g, const secp256k
 
     j = 1;
     for(i = 3; i <= window_g; ++i) {
-        fprintf(fp, "#if ECMULT_TABLE_SIZE(WINDOW_G) > %ld\n", ECMULT_TABLE_SIZE(i-1));
+        fprintf(fp, "#if WINDOW_G > %d\n", i-1);
         for(;j < ECMULT_TABLE_SIZE(i); ++j) {
             fprintf(fp, ",S(%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32
                           ",%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32")\n",
@@ -57,6 +57,8 @@ static void print_two_tables(FILE *fp, int window_g) {
 }
 
 int main(void) {
+    /* Always compute all tables for window sizes up to 15. */
+    int window_g = (ECMULT_WINDOW_SIZE < 15) ? 15 : ECMULT_WINDOW_SIZE;
     FILE* fp;
 
     fp = fopen("src/precomputed_ecmult.c","w");
@@ -77,15 +79,15 @@ int main(void) {
     fprintf(fp, "#include \"ecmult.h\"\n");
     fprintf(fp, "#include \"precomputed_ecmult.h\"\n");
     fprintf(fp, "#define S(a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p) SECP256K1_GE_STORAGE_CONST(0x##a##u,0x##b##u,0x##c##u,0x##d##u,0x##e##u,0x##f##u,0x##g##u,0x##h##u,0x##i##u,0x##j##u,0x##k##u,0x##l##u,0x##m##u,0x##n##u,0x##o##u,0x##p##u)\n");
-    fprintf(fp, "#if ECMULT_TABLE_SIZE(ECMULT_WINDOW_SIZE) > %ld\n", ECMULT_TABLE_SIZE(ECMULT_WINDOW_SIZE));
+    fprintf(fp, "#if ECMULT_WINDOW_SIZE > %d\n", window_g);
     fprintf(fp, "   #error configuration mismatch, invalid ECMULT_WINDOW_SIZE. Try deleting precomputed_ecmult.c before the build.\n");
     fprintf(fp, "#endif\n");
     fprintf(fp, "#ifdef EXHAUSTIVE_TEST_ORDER\n");
     fprintf(fp, "#    error Cannot compile precomputed_ecmult.c in exhaustive test mode\n");
     fprintf(fp, "#endif /* EXHAUSTIVE_TEST_ORDER */\n");
     fprintf(fp, "#define WINDOW_G ECMULT_WINDOW_SIZE\n");
 
-    print_two_tables(fp, ECMULT_WINDOW_SIZE);
+    print_two_tables(fp, window_g);
 
     fprintf(fp, "#undef S\n");
     fclose(fp);