From df5c15870d886ca4eff6c3c9ee3919d862ec2174 Mon Sep 17 00:00:00 2001 From: Jason Gross Date: Mon, 23 Oct 2023 14:43:17 -0700 Subject: [PATCH] Use a simpler version of skipping the asm builds cf https://github.com/mit-plv/fiat-crypto/pull/1692#discussion_r1368943065 --- .github/workflows/c.yml | 4 +- .../2023-10-05-p256-adx.patch | 42 ------------- .../2023-10-05-p256-adx.patch | 62 ------------------- etc/ci/test-fiat-c-boringssl.sh | 15 +---- 4 files changed, 3 insertions(+), 120 deletions(-) delete mode 100644 etc/ci/boringssl-bedrock2-patches/2023-10-05-p256-adx.patch delete mode 100644 etc/ci/boringssl-patches/2023-10-05-p256-adx.patch diff --git a/.github/workflows/c.yml b/.github/workflows/c.yml index 9fafcbd03f..5636ebae1b 100644 --- a/.github/workflows/c.yml +++ b/.github/workflows/c.yml @@ -37,6 +37,6 @@ jobs: - name: make only-test-bedrock2-files CC=gcc run: make only-test-bedrock2-files CC=gcc EXTERNAL_DEPENDENCIES=1 - name: BoringSSL C test - run: EXTRA_CFLAGS="" PATCH_FOLDER="etc/ci/boringssl-patches/" etc/ci/test-fiat-c-boringssl.sh fiat-c/src + run: EXTRA_CFLAGS="" etc/ci/test-fiat-c-boringssl.sh fiat-c/src - name: BoringSSL bedrock2 test - run: EXTRA_CFLAGS="$(make bedrock2-extra-cflags SKIP_INCLUDE=1 2>/dev/null)" PATCH_FOLDER="etc/ci/boringssl-bedrock2-patches/" etc/ci/test-fiat-c-boringssl.sh fiat-bedrock2/src + run: EXTRA_CFLAGS="$(make bedrock2-extra-cflags SKIP_INCLUDE=1 2>/dev/null)" etc/ci/test-fiat-c-boringssl.sh fiat-bedrock2/src diff --git a/etc/ci/boringssl-bedrock2-patches/2023-10-05-p256-adx.patch b/etc/ci/boringssl-bedrock2-patches/2023-10-05-p256-adx.patch deleted file mode 100644 index f38a08c8a4..0000000000 --- a/etc/ci/boringssl-bedrock2-patches/2023-10-05-p256-adx.patch +++ /dev/null @@ -1,42 +0,0 @@ -diff --git a/third_party/fiat/p256_64.h b/third_party/fiat/p256_64.h -index 81a90013d..17bb23f0b 100644 ---- a/third_party/fiat/p256_64.h -+++ b/third_party/fiat/p256_64.h -@@ -1,3 +1,9 @@ -+#include "../../crypto/internal.h" -+#if !defined(OPENSSL_NO_ASM) && defined(__GNUC__) && defined(__x86_64__) -+void fiat_p256_adx_mul(uint64_t*, const uint64_t*, const uint64_t*); -+void fiat_p256_adx_sqr(uint64_t*, const uint64_t*); -+#endif -+ - /* Autogenerated: 'src/ExtractionOCaml/bedrock2_word_by_word_montgomery' --lang bedrock2 --static --no-wide-int --widen-carry --widen-bytes --split-multiret --no-select --no-field-element-typedefs p256 64 '2^256 - 2^224 + 2^192 + 2^96 - 1' mul square add sub opp from_montgomery to_montgomery nonzero selectznz to_bytes from_bytes one msat divstep divstep_precomp */ - /* curve description: p256 */ - /* machine_wordsize = 64 (from "64") */ -@@ -426,6 +432,13 @@ void internal_fiat_p256_mul(uintptr_t out0, uintptr_t in0, uintptr_t in1) { - - /* NOTE: The following wrapper function is not covered by Coq proofs */ - static void fiat_p256_mul(uint64_t out1[4], const uint64_t arg1[4], const uint64_t arg2[4]) { -+#if !defined(OPENSSL_NO_ASM) && defined(__GNUC__) && defined(__x86_64__) -+ if (CRYPTO_is_BMI1_capable() && CRYPTO_is_BMI2_capable() && -+ CRYPTO_is_ADX_capable()) { -+ fiat_p256_adx_mul(out1, arg1, arg2); -+ return; -+ } -+#endif - internal_fiat_p256_mul((uintptr_t)out1, (uintptr_t)arg1, (uintptr_t)arg2); - } - -@@ -769,6 +782,13 @@ void internal_fiat_p256_square(uintptr_t out0, uintptr_t in0) { - - /* NOTE: The following wrapper function is not covered by Coq proofs */ - static void fiat_p256_square(uint64_t out1[4], const uint64_t arg1[4]) { -+#if !defined(OPENSSL_NO_ASM) && defined(__GNUC__) && defined(__x86_64__) -+ if (CRYPTO_is_BMI1_capable() && CRYPTO_is_BMI2_capable() && -+ CRYPTO_is_ADX_capable()) { -+ fiat_p256_adx_sqr(out1, arg1); -+ return; -+ } -+#endif - internal_fiat_p256_square((uintptr_t)out1, (uintptr_t)arg1); - } - diff --git a/etc/ci/boringssl-patches/2023-10-05-p256-adx.patch b/etc/ci/boringssl-patches/2023-10-05-p256-adx.patch deleted file mode 100644 index 8d1ac4fa9f..0000000000 --- a/etc/ci/boringssl-patches/2023-10-05-p256-adx.patch +++ /dev/null @@ -1,62 +0,0 @@ -commit 20c9406971b39d214d4d6997f3a6e3ec772c440a -Author: Andres Erbsen -Date: Mon Sep 25 19:28:44 2023 +0000 - - Add table-independent x86+adx asm for P-256 - - With -march=haswell -DOPENSSL_SMALL=1 on cascadelake: - Did 9999 ECDH P-256 operations in 1062469us (9411.1 ops/sec) [+63.5%] - Did 25000 ECDSA P-256 signing operations in 1028302us (24311.9 ops/sec) [+48.9%] - Did 11004 ECDSA P-256 verify operations in 1072646us (10258.7 ops/sec) [+58.8%] - - Same configuration measured no performance difference on haswell. - - The added assembly code occupies 1352 bytes. - - Change-Id: I42635b7a9bf24d942817976a5d4ce269f642251c - Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63185 - Reviewed-by: David Benjamin - Commit-Queue: David Benjamin - -diff --git a/third_party/fiat/p256_64.h b/third_party/fiat/p256_64.h -index c77263843..a691407b6 100644 ---- a/third_party/fiat/p256_64.h -+++ b/third_party/fiat/p256_64.h -@@ -1,3 +1,9 @@ -+#include "../../crypto/internal.h" -+#if !defined(OPENSSL_NO_ASM) && defined(__GNUC__) && defined(__x86_64__) -+void fiat_p256_adx_mul(uint64_t*, const uint64_t*, const uint64_t*); -+void fiat_p256_adx_sqr(uint64_t*, const uint64_t*); -+#endif -+ - /* Autogenerated: 'src/ExtractionOCaml/word_by_word_montgomery' --inline --static --use-value-barrier p256 64 '2^256 - 2^224 + 2^192 + 2^96 - 1' mul square add sub opp from_montgomery to_montgomery nonzero selectznz to_bytes from_bytes one msat divstep divstep_precomp */ - /* curve description: p256 */ - /* machine_wordsize = 64 (from "64") */ -@@ -165,6 +171,13 @@ static FIAT_P256_FIAT_INLINE void fiat_p256_cmovznz_u64(uint64_t* out1, fiat_p25 - * - */ - static FIAT_P256_FIAT_INLINE void fiat_p256_mul(fiat_p256_montgomery_domain_field_element out1, const fiat_p256_montgomery_domain_field_element arg1, const fiat_p256_montgomery_domain_field_element arg2) { -+#if !defined(OPENSSL_NO_ASM) && defined(__GNUC__) && defined(__x86_64__) -+ if (CRYPTO_is_BMI1_capable() && CRYPTO_is_BMI2_capable() && -+ CRYPTO_is_ADX_capable()) { -+ fiat_p256_adx_mul(out1, arg1, arg2); -+ return; -+ } -+#endif - uint64_t x1; - uint64_t x2; - uint64_t x3; -@@ -472,6 +485,13 @@ static FIAT_P256_FIAT_INLINE void fiat_p256_mul(fiat_p256_montgomery_domain_fiel - * - */ - static FIAT_P256_FIAT_INLINE void fiat_p256_square(fiat_p256_montgomery_domain_field_element out1, const fiat_p256_montgomery_domain_field_element arg1) { -+#if !defined(OPENSSL_NO_ASM) && defined(__GNUC__) && defined(__x86_64__) -+ if (CRYPTO_is_BMI1_capable() && CRYPTO_is_BMI2_capable() && -+ CRYPTO_is_ADX_capable()) { -+ fiat_p256_adx_sqr(out1, arg1); -+ return; -+ } -+#endif - uint64_t x1; - uint64_t x2; - uint64_t x3; diff --git a/etc/ci/test-fiat-c-boringssl.sh b/etc/ci/test-fiat-c-boringssl.sh index f4eb654a44..33522f6c1e 100755 --- a/etc/ci/test-fiat-c-boringssl.sh +++ b/etc/ci/test-fiat-c-boringssl.sh @@ -1,8 +1,6 @@ #!/usr/bin/env bash # USAGE: $0 SUBCOMPONENT (e.g., fiat-c/src) -DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" - ################################################################################ # Tests for BoringSSL ################################################################################ @@ -24,10 +22,6 @@ if [ -z "$SUBCOMPONENT" ]; then fi SUBCOMPONENT_PATH="$(cd "$SUBCOMPONENT" && pwd)" -if [ ! -z "${PATCH_FOLDER}" ]; then - PATCH_FOLDER="$(realpath "${PATCH_FOLDER}")" -fi - pushd boringssl >/dev/null echo "::group::Patching BoringSSL" @@ -38,13 +32,6 @@ echo "::group::Patching BoringSSL" cp "${SUBCOMPONENT_PATH}/${i/.h/.c}" "$i" || exit $? done ) || exit $? ( cd third_party/fiat && git --no-pager diff ) - if [ ! -z "${PATCH_FOLDER}" ]; then - ( cd third_party/fiat && - for i in "${PATCH_FOLDER}"/*.patch; do - git apply "$i" - done ) || exit $? - fi - ( cd third_party/fiat && git --no-pager diff ) }) || exit $? echo "::endgroup::" @@ -53,7 +40,7 @@ echo "::group::Building BoringSSL" set -ex mkdir build cd build - cmake -GNinja .. -DCMAKE_CXX_FLAGS="-Wno-error=unused-function ${EXTRA_CFLAGS}" -DCMAKE_C_FLAGS="-Wno-error=unused-function ${EXTRA_CFLAGS}" || exit $? + cmake -GNinja .. -DOPENSSL_NO_ASM=1 -DCMAKE_CXX_FLAGS="-Wno-error=unused-function ${EXTRA_CFLAGS}" -DCMAKE_C_FLAGS="-Wno-error=unused-function ${EXTRA_CFLAGS}" || exit $? ninja || exit $? }) || exit $? echo "::endgroup::"