Implement simplified backend selection (#428)

tarcieri · web-flow · commit 081f632d911a · 2022-11-13T12:17:42.000-05:00
As proposed in #414, this commit changes the backend selection approach, introspecting `target_pointer_width` to select `u32_backend` vs `u64_backend` (or `fiat_u32_backend`/`fiat_u64_backend` if the `fiat_backend` feature is enabled). This helps eliminate the use of non-additive features, and also the rather confusing errors that happen if multiple backends are selected (i.e. thousands of lines of rustc errors). The selection logic checks if `target_pointer_width = "64"` and uses the 64-bit backend, or falls back to the 32-bit backend otherwise. This means the crate will always have a valid backend regardless of the pointer width, although there may be odd edge cases for exotic platforms which would optimally use the 64-bit backend but have a non-"64" target pointer width for whatever reason. We can handle those cases as they come up.
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
@@ -10,21 +10,24 @@ env:
   CARGO_TERM_COLOR: always
 
 jobs:
-  test-u32:
-    name: Test u32 backend
+  test:
     runs-on: ubuntu-latest
-    steps:
-    - uses: actions/checkout@v3
-    - uses: dtolnay/rust-toolchain@stable
-    - run: cargo test --no-default-features --features "std u32_backend"
+    strategy:
+      matrix:
+        include:
+          # 32-bit target
+          - target: i686-unknown-linux-gnu
+            deps: sudo apt update && sudo apt install gcc-multilib
 
-  test-u64:
-    name: Test u64 backend
-    runs-on: ubuntu-latest
+          # 64-bit target
+          - target: x86_64-unknown-linux-gnu
     steps:
-    - uses: actions/checkout@v3
-    - uses: dtolnay/rust-toolchain@stable
-    - run: cargo test --no-default-features --features "std u64_backend"
+      - uses: actions/checkout@v3
+      - uses: dtolnay/rust-toolchain@stable
+      - run: rustup target add ${{ matrix.target }}
+      - run: ${{ matrix.deps }}
+      - run: cargo test --target ${{ matrix.target }}
+      - run: cargo test --target ${{ matrix.target }} --features fiat_backend
 
   build-simd:
     name: Build simd backend (nightly)
@@ -54,7 +57,9 @@ jobs:
     steps:
     - uses: actions/checkout@v3
     - uses: dtolnay/rust-toolchain@stable
-    - run: cargo test --lib --no-default-features --features "alloc u32_backend"
+    - run: rustup target add i686-unknown-linux-gnu
+    - run: sudo apt update && sudo apt install gcc-multilib
+    - run: cargo test --lib --no-default-features --features alloc --target i686-unknown-linux-gnu
 
   nightly:
     name: Test nightly compiler
@@ -72,11 +77,11 @@ jobs:
     # First run `cargo +nightly -Z minimal-verisons check` in order to get a
     # Cargo.lock with the oldest possible deps
     - uses: dtolnay/rust-toolchain@nightly
-    - run: cargo -Z minimal-versions check --no-default-features --features "fiat_u64_backend serde"
+    - run: cargo -Z minimal-versions check --no-default-features --features fiat_backend,serde
     # Now check that `cargo build` works with respect to the oldest possible
     # deps and the stated MSRV
     - uses: dtolnay/rust-toolchain@1.56.1
-    - run: cargo build --no-default-features --features "fiat_u64_backend serde"
+    - run: cargo build --no-default-features --features fiat_backend,serde
 
   bench:
     name: Check that benchmarks compile
diff --git a/Cargo.toml b/Cargo.toml
@@ -43,6 +43,7 @@ name = "dalek_benchmarks"
 harness = false
 
 [dependencies]
+cfg-if = "1"
 rand_core = { version = "0.6", default-features = false }
 digest = { version = "0.10", default-features = false }
 subtle = { version = "^2.2.1", default-features = false }
@@ -55,20 +56,11 @@ fiat-crypto = { version = "0.1.6", optional = true}
 
 [features]
 nightly = ["subtle/nightly"]
-default = ["std", "u64_backend"]
+default = ["std"]
 std = ["alloc", "subtle/std", "rand_core/std"]
 alloc = ["zeroize/alloc"]
 
-# The u32 backend uses u32s with u64 products.
-u32_backend = []
-# The u64 backend uses u64s with u128 products.
-u64_backend = []
-# fiat-u64 backend (with formally-verified field arith) uses u64s with u128 products.
-fiat_u64_backend = ["fiat-crypto"]
-# fiat-u32 backend (with formally-verified field arith) uses u32s with u64 products.
-fiat_u32_backend = ["fiat-crypto"]
+# fiat-crypto backend with formally-verified field arithmetic
+fiat_backend = ["fiat-crypto"]
 # The SIMD backend uses parallel formulas, using either AVX2 or AVX512-IFMA.
-simd_backend = ["nightly", "u64_backend", "packed_simd"]
-# DEPRECATED: this is now an alias for `simd_backend` and may be removed
-# in some future release.
-avx2_backend = ["simd_backend"]
+simd_backend = ["nightly", "packed_simd"]
diff --git a/README.md b/README.md
@@ -82,9 +82,13 @@ version, and in terms of non-breaking changes it includes:
 ### 4.x (current alpha)
 
 The `4.x` series has an API largely unchanged from `3.x`, with a breaking change
-to update the `rand` dependency crates.  It also requires including a new trait,
-`use curve25519_dalek::traits::BasepointTable`, whenever using `EdwardsBasepointTable`
-or `RistrettoBasepointTable`.
+to update the `rand` dependency crates.
+
+It also requires including a new trait,
+`use curve25519_dalek::traits::BasepointTable`, whenever using
+`EdwardsBasepointTable` or `RistrettoBasepointTable`.
+
+Backend selection has also been updated to be more automatic. See below.
 
 # Backends and Features
 
@@ -98,24 +102,26 @@ Curve arithmetic is implemented using one of the following backends:
 * a `u64` backend using serial formulas and `u128` products;
 * an `avx2` backend using [parallel formulas][parallel_doc] and `avx2` instructions (sets speed records);
 * an `ifma` backend using [parallel formulas][parallel_doc] and `ifma` instructions (sets speed records);
-
-By default the `u64` backend is selected.  To select a specific backend, use:
-```sh
-cargo build --no-default-features --features "std u32_backend"
-cargo build --no-default-features --features "std u64_backend"
-# Requires nightly, RUSTFLAGS="-C target_feature=+avx2" to use avx2
-cargo build --no-default-features --features "std simd_backend"
-# Requires nightly, RUSTFLAGS="-C target_feature=+avx512ifma" to use ifma
-cargo build --no-default-features --features "std simd_backend"
-```
-Crates using `curve25519-dalek` can either select a backend on behalf of their
-users, or expose feature flags that control the `curve25519-dalek` backend.
+* a `fiat` backend using formally verified field arithmetic from [fiat-crypto];
 
 The `std` feature is enabled by default, but it can be disabled for no-`std`
 builds using `--no-default-features`.  Note that this requires explicitly
 selecting an arithmetic backend using one of the `_backend` features.
 If no backend is selected, compilation will fail.
 
+## Backend selection
+
+Backend selection is done automatically. E.g., if you're compiling on a
+64-bit machine, then the `u64` backend is automatically chosen. And
+if the `fiat_backend` feature is set, then the fiat `u64` backend is
+chosen.
+
+If you need a `u32` backend on a `u64` machine, then simple
+cross-compiling will work on an x86-64 Linux machine:
+
+* `sudo apt install gcc-multilib` (or whatever package manager you use)
+* `rustup target add i686-unknown-linux-gnu`
+* `cargo build --target i686-unknown-linux-gnu`
 
 # Minimum Supported Rust Version
 
@@ -166,11 +172,10 @@ compiled with appropriate `target_feature`s, so this cannot occur.
 Benchmarks are run using [`criterion.rs`][criterion]:
 
 ```sh
-cargo bench --no-default-features --features "std u32_backend"
-cargo bench --no-default-features --features "std u64_backend"
+cargo bench --no-default-features
 # Uses avx2 or ifma only if compiled for an appropriate target.
 export RUSTFLAGS="-C target_cpu=native"
-cargo bench --no-default-features --features "std simd_backend"
+cargo +nightly bench --no-default-features --features simd_backend
 ```
 
 Performance is a secondary goal behind correctness, safety, and
@@ -227,10 +232,9 @@ optimised batch inversion was contributed by Sean Bowe and Daira Hopwood.
 
 The `no_std` and `zeroize` support was contributed by Tony Arcieri.
 
-The formally verified backends, `fiat_u32_backend` and `fiat_u64_backend`, which
-integrate with the Rust generated by the
-[Fiat Crypto project](https://github.com/mit-plv/fiat-crypto) were contributed
-by François Garillot.
+The formally verified `fiat_backend` integrates Rust code generated by the
+[Fiat Crypto project](https://github.com/mit-plv/fiat-crypto) and was
+contributed by François Garillot.
 
 Thanks also to Ashley Hauck, Lucas Salibian, Manish Goregaokar, Jack Grigg,
 Pratyush Mishra, Michael Rosenberg, and countless others for their
@@ -244,3 +248,4 @@ contributions.
 [criterion]: https://github.com/japaric/criterion.rs
 [parallel_doc]: https://doc-internal.dalek.rs/curve25519_dalek/backend/vector/avx2/index.html
 [subtle_doc]: https://doc.dalek.rs/subtle/
+[fiat-crypto]: https://github.com/mit-plv/fiat-crypto
diff --git a/src/backend/mod.rs b/src/backend/mod.rs
@@ -34,18 +34,6 @@
 //! The [`vector`] backend is selected by the `simd_backend` cargo
 //! feature; it uses the [`serial`] backend for non-vectorized operations.
 
-#[cfg(not(any(
-    feature = "u32_backend",
-    feature = "u64_backend",
-    feature = "fiat_u32_backend",
-    feature = "fiat_u64_backend",
-    feature = "simd_backend",
-)))]
-compile_error!(
-    "no curve25519-dalek backend cargo feature enabled! \
-     please enable one of: u32_backend, u64_backend, fiat_u32_backend, fiat_u64_backend, simd_backend"
-);
-
 pub mod serial;
 
 #[cfg(any(
diff --git a/src/backend/serial/mod.rs b/src/backend/serial/mod.rs
@@ -19,32 +19,24 @@
 //!
 //! When the vector backend is enabled, the field and scalar
 //! implementations are still used for non-vectorized operations.
-//!
-//! Note: at this time the `u32` and `u64` backends cannot be built
-//! together.
-
-#[cfg(not(any(
-    feature = "u32_backend",
-    feature = "u64_backend",
-    feature = "fiat_u32_backend",
-    feature = "fiat_u64_backend"
-)))]
-compile_error!(
-    "no curve25519-dalek backend cargo feature enabled! \
-     please enable one of: u32_backend, u64_backend, fiat_u32_backend, fiat_u64_backend"
-);
 
-#[cfg(feature = "u32_backend")]
-pub mod u32;
+use cfg_if::cfg_if;
 
-#[cfg(feature = "u64_backend")]
-pub mod u64;
+cfg_if! {
+    if #[cfg(feature = "fiat_backend")] {
+        #[cfg(not(target_pointer_width = "64"))]
+        pub mod fiat_u32;
 
-#[cfg(feature = "fiat_u32_backend")]
-pub mod fiat_u32;
+        #[cfg(target_pointer_width = "64")]
+        pub mod fiat_u64;
+    } else {
+        #[cfg(not(target_pointer_width = "64"))]
+        pub mod u32;
 
-#[cfg(feature = "fiat_u64_backend")]
-pub mod fiat_u64;
+        #[cfg(target_pointer_width = "64")]
+        pub mod u64;
+    }
+}
 
 pub mod curve_models;
 
diff --git a/src/constants.rs b/src/constants.rs
@@ -28,20 +28,27 @@
 
 #![allow(non_snake_case)]
 
+use cfg_if::cfg_if;
+
 use crate::edwards::CompressedEdwardsY;
 use crate::montgomery::MontgomeryPoint;
 use crate::ristretto::CompressedRistretto;
 use crate::ristretto::RistrettoPoint;
 use crate::scalar::Scalar;
 
-#[cfg(feature = "fiat_u32_backend")]
-pub use crate::backend::serial::fiat_u32::constants::*;
-#[cfg(feature = "fiat_u64_backend")]
-pub use crate::backend::serial::fiat_u64::constants::*;
-#[cfg(feature = "u32_backend")]
-pub use crate::backend::serial::u32::constants::*;
-#[cfg(feature = "u64_backend")]
-pub use crate::backend::serial::u64::constants::*;
+cfg_if! {
+    if #[cfg(feature = "fiat_backend")] {
+        #[cfg(not(target_pointer_width = "64"))]
+        pub use crate::backend::serial::fiat_u32::constants::*;
+        #[cfg(target_pointer_width = "64")]
+        pub use crate::backend::serial::fiat_u64::constants::*;
+    } else {
+        #[cfg(not(target_pointer_width = "64"))]
+        pub use crate::backend::serial::u32::constants::*;
+        #[cfg(target_pointer_width = "64")]
+        pub use crate::backend::serial::u64::constants::*;
+    }
+}
 
 /// The Ed25519 basepoint, in `CompressedEdwardsY` format.
 ///
@@ -142,7 +149,7 @@ mod test {
 
     /// Test that d = -121665/121666
     #[test]
-    #[cfg(feature = "u32_backend")]
+    #[cfg(all(not(target_pointer_width = "64"), not(feature = "fiat_backend")))]
     fn test_d_vs_ratio() {
         use crate::backend::serial::u32::field::FieldElement2625;
         let a = -&FieldElement2625([121665, 0, 0, 0, 0, 0, 0, 0, 0, 0]);
@@ -155,7 +162,7 @@ mod test {
 
     /// Test that d = -121665/121666
     #[test]
-    #[cfg(feature = "u64_backend")]
+    #[cfg(all(target_pointer_width = "64", not(feature = "fiat_backend")))]
     fn test_d_vs_ratio() {
         use crate::backend::serial::u64::field::FieldElement51;
         let a = -&FieldElement51([121665, 0, 0, 0, 0]);
diff --git a/src/field.rs b/src/field.rs
@@ -25,6 +25,8 @@
 
 use core::cmp::{Eq, PartialEq};
 
+use cfg_if::cfg_if;
+
 use subtle::Choice;
 use subtle::ConditionallyNegatable;
 use subtle::ConditionallySelectable;
@@ -33,40 +35,52 @@ use subtle::ConstantTimeEq;
 use crate::backend;
 use crate::constants;
 
-#[cfg(feature = "fiat_u32_backend")]
-pub use backend::serial::fiat_u32::field::*;
-#[cfg(feature = "fiat_u64_backend")]
-pub use backend::serial::fiat_u64::field::*;
-/// A `FieldElement` represents an element of the field
-/// \\( \mathbb Z / (2\^{255} - 19)\\).
-///
-/// The `FieldElement` type is an alias for one of the platform-specific
-/// implementations.
-/// Using formally-verified field arithmetic from fiat-crypto
-#[cfg(feature = "fiat_u32_backend")]
-pub type FieldElement = backend::serial::fiat_u32::field::FieldElement2625;
-#[cfg(feature = "fiat_u64_backend")]
-pub type FieldElement = backend::serial::fiat_u64::field::FieldElement51;
-
-#[cfg(feature = "u64_backend")]
-pub use crate::backend::serial::u64::field::*;
-/// A `FieldElement` represents an element of the field
-/// \\( \mathbb Z / (2\^{255} - 19)\\).
-///
-/// The `FieldElement` type is an alias for one of the platform-specific
-/// implementations.
-#[cfg(feature = "u64_backend")]
-pub type FieldElement = backend::serial::u64::field::FieldElement51;
-
-#[cfg(feature = "u32_backend")]
-pub use backend::serial::u32::field::*;
-/// A `FieldElement` represents an element of the field
-/// \\( \mathbb Z / (2\^{255} - 19)\\).
-///
-/// The `FieldElement` type is an alias for one of the platform-specific
-/// implementations.
-#[cfg(feature = "u32_backend")]
-pub type FieldElement = backend::serial::u32::field::FieldElement2625;
+cfg_if! {
+    if #[cfg(feature = "fiat_backend")] {
+        #[cfg(not(target_pointer_width = "64"))]
+        pub use backend::serial::fiat_u32::field::*;
+        #[cfg(target_pointer_width = "64")]
+        pub use backend::serial::fiat_u64::field::*;
+
+        /// A `FieldElement` represents an element of the field
+        /// \\( \mathbb Z / (2\^{255} - 19)\\).
+        ///
+        /// The `FieldElement` type is an alias for one of the platform-specific
+        /// implementations.
+        ///
+        /// Using formally-verified field arithmetic from fiat-crypto.
+        #[cfg(not(target_pointer_width = "64"))]
+        pub type FieldElement = backend::serial::fiat_u32::field::FieldElement2625;
+
+        /// A `FieldElement` represents an element of the field
+        /// \\( \mathbb Z / (2\^{255} - 19)\\).
+        ///
+        /// The `FieldElement` type is an alias for one of the platform-specific
+        /// implementations.
+        ///
+        /// Using formally-verified field arithmetic from fiat-crypto.
+        #[cfg(target_pointer_width = "64")]
+        pub type FieldElement = backend::serial::fiat_u64::field::FieldElement51;
+    } else if #[cfg(target_pointer_width = "64")] {
+        pub use crate::backend::serial::u64::field::*;
+
+        /// A `FieldElement` represents an element of the field
+        /// \\( \mathbb Z / (2\^{255} - 19)\\).
+        ///
+        /// The `FieldElement` type is an alias for one of the platform-specific
+        /// implementations.
+        pub type FieldElement = backend::serial::u64::field::FieldElement51;
+    } else {
+        pub use backend::serial::u32::field::*;
+
+        /// A `FieldElement` represents an element of the field
+        /// \\( \mathbb Z / (2\^{255} - 19)\\).
+        ///
+        /// The `FieldElement` type is an alias for one of the platform-specific
+        /// implementations.
+        pub type FieldElement = backend::serial::u32::field::FieldElement2625;
+    }
+}
 
 impl Eq for FieldElement {}
 
diff --git a/src/scalar.rs b/src/scalar.rs
