Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the u64 backend by default on x86_64 #126

Closed
3 tasks
hdevalence opened this issue Apr 6, 2018 · 3 comments
Closed
3 tasks

Use the u64 backend by default on x86_64 #126

hdevalence opened this issue Apr 6, 2018 · 3 comments

Comments

@hdevalence
Copy link
Contributor

(Branching off of this comment since I think this may be off-topic there)

curve25519-dalek has two field arithmetic implementations, a u32 implementation (using u64s for products) and a u64 implementation (using u128s for products). The u64 backend is selected by the radix_51 feature, which is selected by the nightly feature. Now that u128s are stabilizing (tracking issue), u128s aren't nightly-only. We should:

  • remove the #![feature(i128_type)] lines;
  • unlink radix_51 from nightly;
  • make radix_51 the default feature on x86_64.

The last point seems tricky / impossible, since target-specific or target-default features are currently an open issue in Cargo.

Features are selected before any code is built, so it's not possible to use architecture-dependent #[cfg]s to choose a feature.

One hack (mentioned in the Cargo issue and in the u128 tracking issue) is to use a #[cfg]s in a build script to emit feature selections for the main crate. But this doesn't work for us, because we use the build script to generate constants, so by the time we're in build.rs we need to already know which backend to use.

Another approach is to select the backend not by a feature but directly by architecture, or to always use u128s, as in zkcrypto/pairing#80 .

However, the problem is that just because u128s are supported by a target's backend (1) or even lowered to instructions that do a 64 x 64 -> 128 bit multiplication (2) doesn't mean it's better to use them. For (1), if a 64-bit multiplication is going to be emulated by 32-bit multiplications anyways, it would be better just to use them directly, and as an example of (2), aarch64 has instructions for computing high and low parts of a 128-bit product, but they're not necessarily fast. (This is considering only speed and not safety, like whether emulated multiplications are constant-time).

@est31
Copy link

est31 commented Apr 14, 2018

One hack (mentioned in the Cargo issue and in the u128 tracking issue) is to use a #[cfg]s in a build script to emit feature selections for the main crate.

build.rs gets always compiled for the host platform but has to create programs for the target platform. If the two platforms are different (in a cross compilation scenario), the platform build.rs gets built for is different from the platform it has to build stuff for. So don't use #[cfg] or cfg! in build.rs for that purpose. Instead, build.rs has the TARGET env var.

@hdevalence
Copy link
Contributor Author

hdevalence commented May 14, 2018

It seems like the "optimal" thing to do would be something like the following:

  • Allow explicit backend selection with u32, u64, avx2 features;
  • If no backend feature is specified, build.rs looks at the TARGET env var and makes a best-guess about which backend to use, then emits that feature for the main compilation.

However, because we load the entire crate into the build.rs already in order to generate constants using crate-private code, we need to have already selected the target backend before compiling the build script. So the approach above won't work. But this seems like the only way to access crate-private code without exposing it to the public.

hdevalence added a commit that referenced this issue May 15, 2018
Each backend can now be selected by an individual feature:

- `u32_backend` for `backend::u32`;
- `u64_backend` for `backend::u64`;
- `avx2_backend` for `backend::avx2`;

The `u64_backend` is selected by default, since most people use X64 and we have
no way to select based on target (see discussion in #126).  However, these
changes mean that it is possible to select the backend explicitly, and if we
had the ability to select target-default features, we could do so easily.
hdevalence added a commit that referenced this issue May 15, 2018
Each backend can now be selected by an individual feature:

- `u32_backend` for `backend::u32`;
- `u64_backend` for `backend::u64`;
- `avx2_backend` for `backend::avx2`;

The `u64_backend` is selected by default, since most people use X64 and we have
no way to select based on target (see discussion in #126).  However, these
changes mean that it is possible to select the backend explicitly, and if we
had the ability to select target-default features, we could do so easily.
hdevalence added a commit that referenced this issue May 15, 2018
Each backend can now be selected by an individual feature:

- `u32_backend` for `backend::u32`;
- `u64_backend` for `backend::u64`;
- `avx2_backend` for `backend::avx2`;

The `u64_backend` is selected by default, since most people use X64 and we have
no way to select based on target (see discussion in #126).  However, these
changes mean that it is possible to select the backend explicitly, and if we
had the ability to select target-default features, we could do so easily.
@hdevalence
Copy link
Contributor Author

The current status is that the u64 backend is used by default, and that all backends are selectable with features.

Ideally we would auto-select the backend, but for the reasons described above we can't currently do this.

@hdevalence hdevalence added this to the 1.0.0 milestone Jun 21, 2018
pinkforest pushed a commit to pinkforest/curve25519-dalek that referenced this issue Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants