From 8fa201639aeeb1a3b90f38c561441097a36105e4 Mon Sep 17 00:00:00 2001 From: Michael Rosenberg Date: Fri, 28 Oct 2022 13:10:44 -0400 Subject: [PATCH] Fix AVX2 and AVX-512 builds (#419) Build was broken by 5758b8c. This adds a regression test to CI so it doesn't happen again --- .github/workflows/rust.yml | 12 ++++++++--- src/backend/vector/avx2/constants.rs | 6 +++--- src/backend/vector/avx2/edwards.rs | 20 +++++++++---------- src/backend/vector/avx2/field.rs | 6 ++++-- src/backend/vector/ifma/constants.rs | 2 +- src/backend/vector/ifma/edwards.rs | 14 ++++++------- src/backend/vector/ifma/field.rs | 2 +- src/backend/vector/scalar_mul/pippenger.rs | 18 ++++++++--------- .../vector/scalar_mul/precomputed_straus.rs | 14 ++++++------- src/backend/vector/scalar_mul/straus.rs | 12 +++++------ .../vector/scalar_mul/variable_base.rs | 10 +++++----- .../vector/scalar_mul/vartime_double_base.rs | 12 +++++------ src/edwards.rs | 2 +- src/ristretto.rs | 2 +- 14 files changed, 69 insertions(+), 63 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 7e97cdc..6e0ecef 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -26,13 +26,19 @@ jobs: - uses: dtolnay/rust-toolchain@stable - run: cargo test --no-default-features --features "std u64_backend" - test-simd: - name: Test simd backend (nightly) + build-simd: + name: Build simd backend (nightly) runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 - uses: dtolnay/rust-toolchain@nightly - - run: cargo test --no-default-features --features "std simd_backend" + # Build with AVX2 features, then with AVX512 features + - env: + RUSTFLAGS: "-C target_feature=+avx2" + run: cargo build --no-default-features --features "std simd_backend" + - env: + RUSTFLAGS: "-C target_feature=+avx512ifma" + run: cargo build --no-default-features --features "std simd_backend" test-defaults-serde: name: Test default feature selection and serde diff --git a/src/backend/vector/avx2/constants.rs b/src/backend/vector/avx2/constants.rs index 122068e..048f6aa 100644 --- a/src/backend/vector/avx2/constants.rs +++ b/src/backend/vector/avx2/constants.rs @@ -13,9 +13,9 @@ use packed_simd::u32x8; -use backend::vector::avx2::edwards::{CachedPoint, ExtendedPoint}; -use backend::vector::avx2::field::FieldElement2625x4; -use window::NafLookupTable8; +use crate::backend::vector::avx2::edwards::{CachedPoint, ExtendedPoint}; +use crate::backend::vector::avx2::field::FieldElement2625x4; +use crate::window::NafLookupTable8; /// The identity element as an `ExtendedPoint`. pub(crate) static EXTENDEDPOINT_IDENTITY: ExtendedPoint = ExtendedPoint(FieldElement2625x4([ diff --git a/src/backend/vector/avx2/edwards.rs b/src/backend/vector/avx2/edwards.rs index 821d516..8fab79d 100644 --- a/src/backend/vector/avx2/edwards.rs +++ b/src/backend/vector/avx2/edwards.rs @@ -41,10 +41,10 @@ use core::ops::{Add, Neg, Sub}; use subtle::Choice; use subtle::ConditionallySelectable; -use edwards; -use window::{LookupTable, NafLookupTable5, NafLookupTable8}; +use crate::edwards; +use crate::window::{LookupTable, NafLookupTable5, NafLookupTable8}; -use traits::Identity; +use crate::traits::Identity; use super::constants; use super::field::{FieldElement2625x4, Lanes, Shuffle}; @@ -330,7 +330,7 @@ mod test { use super::*; fn serial_add(P: edwards::EdwardsPoint, Q: edwards::EdwardsPoint) -> edwards::EdwardsPoint { - use backend::serial::u64::field::FieldElement51; + use crate::backend::serial::u64::field::FieldElement51; let (X1, Y1, Z1, T1) = (P.X, P.Y, P.Z, P.T); let (X2, Y2, Z2, T2) = (Q.X, Q.Y, Q.Z, Q.T); @@ -420,8 +420,8 @@ mod test { #[test] fn vector_addition_vs_serial_addition_vs_edwards_extendedpoint() { - use constants; - use scalar::Scalar; + use crate::constants; + use crate::scalar::Scalar; println!("Testing id +- id"); let P = edwards::EdwardsPoint::identity(); @@ -507,8 +507,8 @@ mod test { #[test] fn vector_doubling_vs_serial_doubling_vs_edwards_extendedpoint() { - use constants; - use scalar::Scalar; + use crate::constants; + use crate::scalar::Scalar; println!("Testing [2]id"); let P = edwards::EdwardsPoint::identity(); @@ -525,8 +525,8 @@ mod test { #[test] fn basepoint_odd_lookup_table_verify() { - use constants; - use backend::vector::avx2::constants::{BASEPOINT_ODD_LOOKUP_TABLE}; + use crate::constants; + use crate::backend::vector::avx2::constants::{BASEPOINT_ODD_LOOKUP_TABLE}; let basepoint_odd_table = NafLookupTable8::::from(&constants::ED25519_BASEPOINT_POINT); println!("basepoint_odd_lookup_table = {:?}", basepoint_odd_table); diff --git a/src/backend/vector/avx2/field.rs b/src/backend/vector/avx2/field.rs index 94a06ee..969ec09 100644 --- a/src/backend/vector/avx2/field.rs +++ b/src/backend/vector/avx2/field.rs @@ -43,8 +43,10 @@ const D_LANES64: u8 = 0b11_00_00_00; use core::ops::{Add, Mul, Neg}; use packed_simd::{i32x8, u32x8, u64x4, IntoBits}; -use backend::vector::avx2::constants::{P_TIMES_16_HI, P_TIMES_16_LO, P_TIMES_2_HI, P_TIMES_2_LO}; -use backend::serial::u64::field::FieldElement51; +use crate::backend::vector::avx2::constants::{ + P_TIMES_16_HI, P_TIMES_16_LO, P_TIMES_2_HI, P_TIMES_2_LO, +}; +use crate::backend::serial::u64::field::FieldElement51; /// Unpack 32-bit lanes into 64-bit lanes: /// ```ascii,no_run diff --git a/src/backend/vector/ifma/constants.rs b/src/backend/vector/ifma/constants.rs index fd89058..e9dc24f 100644 --- a/src/backend/vector/ifma/constants.rs +++ b/src/backend/vector/ifma/constants.rs @@ -11,7 +11,7 @@ use packed_simd::u64x4; -use window::NafLookupTable8; +use crate::window::NafLookupTable8; use super::edwards::{CachedPoint, ExtendedPoint}; use super::field::{F51x4Reduced, F51x4Unreduced}; diff --git a/src/backend/vector/ifma/edwards.rs b/src/backend/vector/ifma/edwards.rs index 5c8d819..194bc64 100644 --- a/src/backend/vector/ifma/edwards.rs +++ b/src/backend/vector/ifma/edwards.rs @@ -9,15 +9,15 @@ #![allow(non_snake_case)] -use traits::Identity; +use crate::traits::Identity; use std::ops::{Add, Neg, Sub}; use subtle::Choice; use subtle::ConditionallySelectable; -use edwards; -use window::{LookupTable, NafLookupTable5, NafLookupTable8}; +use crate::edwards; +use crate::window::{LookupTable, NafLookupTable5, NafLookupTable8}; use super::constants; use super::field::{F51x4Reduced, F51x4Unreduced, Lanes, Shuffle}; @@ -258,8 +258,8 @@ mod test { #[test] fn vector_addition_vs_serial_addition_vs_edwards_extendedpoint() { - use constants; - use scalar::Scalar; + use crate::constants; + use crate::scalar::Scalar; println!("Testing id +- id"); let P = edwards::EdwardsPoint::identity(); @@ -297,8 +297,8 @@ mod test { #[test] fn vector_doubling_vs_serial_doubling_vs_edwards_extendedpoint() { - use constants; - use scalar::Scalar; + use crate::constants; + use crate::scalar::Scalar; println!("Testing [2]id"); let P = edwards::EdwardsPoint::identity(); diff --git a/src/backend/vector/ifma/field.rs b/src/backend/vector/ifma/field.rs index a393b22..94d9e59 100644 --- a/src/backend/vector/ifma/field.rs +++ b/src/backend/vector/ifma/field.rs @@ -14,7 +14,7 @@ use core::ops::{Add, Mul, Neg}; use packed_simd::{u64x4, IntoBits}; -use backend::serial::u64::field::FieldElement51; +use crate::backend::serial::u64::field::FieldElement51; /// A wrapper around `vpmadd52luq` that works on `u64x4`. #[inline(always)] diff --git a/src/backend/vector/scalar_mul/pippenger.rs b/src/backend/vector/scalar_mul/pippenger.rs index 7f9e241..3ed5e91 100644 --- a/src/backend/vector/scalar_mul/pippenger.rs +++ b/src/backend/vector/scalar_mul/pippenger.rs @@ -11,13 +11,13 @@ use core::borrow::Borrow; -use backend::vector::{CachedPoint, ExtendedPoint}; -use edwards::EdwardsPoint; -use scalar::Scalar; -use traits::{Identity, VartimeMultiscalarMul}; +use crate::backend::vector::{CachedPoint, ExtendedPoint}; +use crate::edwards::EdwardsPoint; +use crate::scalar::Scalar; +use crate::traits::{Identity, VartimeMultiscalarMul}; #[allow(unused_imports)] -use prelude::*; +use crate::prelude::*; /// Implements a version of Pippenger's algorithm. /// @@ -50,9 +50,7 @@ impl VartimeMultiscalarMul for Pippenger { // Collect optimized scalars and points in a buffer for repeated access // (scanning the whole collection per each digit position). - let scalars = scalars - .into_iter() - .map(|s| s.borrow().to_radix_2w(w)); + let scalars = scalars.into_iter().map(|s| s.borrow().to_radix_2w(w)); let points = points .into_iter() @@ -127,8 +125,8 @@ impl VartimeMultiscalarMul for Pippenger { #[cfg(test)] mod test { use super::*; - use constants; - use scalar::Scalar; + use crate::constants; + use crate::scalar::Scalar; #[test] fn test_vartime_pippenger() { diff --git a/src/backend/vector/scalar_mul/precomputed_straus.rs b/src/backend/vector/scalar_mul/precomputed_straus.rs index 2c6fdf5..cac8a75 100644 --- a/src/backend/vector/scalar_mul/precomputed_straus.rs +++ b/src/backend/vector/scalar_mul/precomputed_straus.rs @@ -13,15 +13,15 @@ use core::borrow::Borrow; -use backend::vector::{CachedPoint, ExtendedPoint}; -use edwards::EdwardsPoint; -use scalar::Scalar; -use traits::Identity; -use traits::VartimePrecomputedMultiscalarMul; -use window::{NafLookupTable5, NafLookupTable8}; +use crate::backend::vector::{CachedPoint, ExtendedPoint}; +use crate::edwards::EdwardsPoint; +use crate::scalar::Scalar; +use crate::traits::Identity; +use crate::traits::VartimePrecomputedMultiscalarMul; +use crate::window::{NafLookupTable5, NafLookupTable8}; #[allow(unused_imports)] -use prelude::*; +use crate::prelude::*; pub struct VartimePrecomputedStraus { diff --git a/src/backend/vector/scalar_mul/straus.rs b/src/backend/vector/scalar_mul/straus.rs index b6c02f9..4a8c92e 100644 --- a/src/backend/vector/scalar_mul/straus.rs +++ b/src/backend/vector/scalar_mul/straus.rs @@ -15,14 +15,14 @@ use core::borrow::Borrow; use zeroize::Zeroizing; -use backend::vector::{CachedPoint, ExtendedPoint}; -use edwards::EdwardsPoint; -use scalar::Scalar; -use window::{LookupTable, NafLookupTable5}; -use traits::{Identity, MultiscalarMul, VartimeMultiscalarMul}; +use crate::backend::vector::{CachedPoint, ExtendedPoint}; +use crate::edwards::EdwardsPoint; +use crate::scalar::Scalar; +use crate::window::{LookupTable, NafLookupTable5}; +use crate::traits::{Identity, MultiscalarMul, VartimeMultiscalarMul}; #[allow(unused_imports)] -use prelude::*; +use crate::prelude::*; /// Multiscalar multiplication using interleaved window / Straus' /// method. See the `Straus` struct in the serial backend for more diff --git a/src/backend/vector/scalar_mul/variable_base.rs b/src/backend/vector/scalar_mul/variable_base.rs index f53c4a0..5bdb912 100644 --- a/src/backend/vector/scalar_mul/variable_base.rs +++ b/src/backend/vector/scalar_mul/variable_base.rs @@ -1,10 +1,10 @@ #![allow(non_snake_case)] -use backend::vector::{CachedPoint, ExtendedPoint}; -use edwards::EdwardsPoint; -use scalar::Scalar; -use traits::Identity; -use window::LookupTable; +use crate::backend::vector::{CachedPoint, ExtendedPoint}; +use crate::edwards::EdwardsPoint; +use crate::scalar::Scalar; +use crate::traits::Identity; +use crate::window::LookupTable; /// Perform constant-time, variable-base scalar multiplication. pub fn mul(point: &EdwardsPoint, scalar: &Scalar) -> EdwardsPoint { diff --git a/src/backend/vector/scalar_mul/vartime_double_base.rs b/src/backend/vector/scalar_mul/vartime_double_base.rs index 3f7cc3e..757a9ce 100644 --- a/src/backend/vector/scalar_mul/vartime_double_base.rs +++ b/src/backend/vector/scalar_mul/vartime_double_base.rs @@ -11,12 +11,12 @@ #![allow(non_snake_case)] -use backend::vector::BASEPOINT_ODD_LOOKUP_TABLE; -use backend::vector::{CachedPoint, ExtendedPoint}; -use edwards::EdwardsPoint; -use scalar::Scalar; -use traits::Identity; -use window::NafLookupTable5; +use crate::backend::vector::BASEPOINT_ODD_LOOKUP_TABLE; +use crate::backend::vector::{CachedPoint, ExtendedPoint}; +use crate::edwards::EdwardsPoint; +use crate::scalar::Scalar; +use crate::traits::Identity; +use crate::window::NafLookupTable5; /// Compute \\(aA + bB\\) in variable time, where \\(B\\) is the Ed25519 basepoint. pub fn mul(a: &Scalar, A: &EdwardsPoint, b: &Scalar) -> EdwardsPoint { diff --git a/src/edwards.rs b/src/edwards.rs index 5dd18e3..6b89882 100644 --- a/src/edwards.rs +++ b/src/edwards.rs @@ -148,7 +148,7 @@ use crate::backend::serial::scalar_mul; feature = "simd_backend", any(target_feature = "avx2", target_feature = "avx512ifma") ))] -use backend::vector::scalar_mul; +use crate::backend::vector::scalar_mul; // ------------------------------------------------------------------------ // Compressed points diff --git a/src/ristretto.rs b/src/ristretto.rs index 94c537c..2c663b1 100644 --- a/src/ristretto.rs +++ b/src/ristretto.rs @@ -202,7 +202,7 @@ use crate::backend::serial::scalar_mul; feature = "simd_backend", any(target_feature = "avx2", target_feature = "avx512ifma") ))] -use backend::vector::scalar_mul; +use crate::backend::vector::scalar_mul; // ------------------------------------------------------------------------ // Compressed points