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.

Sign up for GitHub

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

Jump to bottom

Well known defaults for `curve25519_dalek_bits` #456

Open

pinkforest opened this issue Dec 8, 2022 · 5 comments

Contributor

pinkforest commented Dec 8, 2022 •

edited

Origin issue:

u32_backend is slower than u64_backend on armv7 Android #449

Our MVP became the "override" between [fiat_]u32 and [fiat_]u64 regardless of cfg(target_pointer_width)

u32_backend is slower than u64_backend on armv7 Android #449 (comment)

We implemented the MVP here:

Add target u32/u64 backend overrides #454

Where the default is based on cfg(target_pointer_width):

[fiat_]u32 on 32 bit targets
[fiat_]u64 on 64 bit targets
[fiat_]u32 on all other

@tarcieri raised to provide well known "target defaults" e.g. arm, wasm etc. : #449 (comment), #454 (comment)

This would be (https://doc.rust-lang.org/reference/conditional-compilation.html):

cfg(target_family = "wasm") -> [fiat_]u64
cfg(target_arch = "armv7") -> [fiat_]u64
... ?

Could we perhaps discuss / gather a list proper for MVP for this and then can perhaps throw PR at this ?

The text was updated successfully, but these errors were encountered:

This was referenced Dec 8, 2022

Add target u32/u64 backend overrides #454

Merged

u32_backend is slower than u64_backend on armv7 Android #449

Closed

Fixes curve25519_dalek_bits defaults for cross and wasm #465

Merged

pinkforest added a commit to pinkforest/curve25519-dalek that referenced this issue


          Set well known defaults for wasm and arm

f9f4cc2

As discussed in dalek-cryptography#456 this sets well known defaults for cfg(target_family = "wasm")
and cfg(target_arch = "arm") for 64 bit arithmetric via cfg(curve25519_dalek_bits = "64")

pinkforest added a commit to pinkforest/curve25519-dalek that referenced this issue


          Set well known defaults for wasm and arm

826a532

As discussed in dalek-cryptography#456 this sets well known defaults for cfg(target_family = "wasm")
and cfg(target_arch = "arm") for 64 bit arithmetric via cfg(curve25519_dalek_bits = "64")

Contributor Author

pinkforest commented Dec 9, 2022

PR's up:

Fixes curve25519_dalek_bits defaults for cross and wasm #465

pinkforest added a commit to pinkforest/curve25519-dalek that referenced this issue


          Set well known defaults for wasm and arm

6383f2d

As discussed in dalek-cryptography#456 this sets well known defaults for cfg(target_family = "wasm")
and cfg(target_arch = "arm") for 64 bit arithmetric via cfg(curve25519_dalek_bits = "64")

pinkforest added a commit to pinkforest/curve25519-dalek that referenced this issue


          Set well known defaults for wasm and arm

97c84d7

As discussed in dalek-cryptography#456 this sets well known defaults for cfg(target_family = "wasm")
and cfg(target_arch = "arm") for 64 bit arithmetric via cfg(curve25519_dalek_bits = "64")

Contributor

rozbb commented Dec 9, 2022

Related: we also need a README update to reflect the new backend selection stuff

pinkforest added a commit to pinkforest/curve25519-dalek that referenced this issue


          Set well known defaults for wasm and arm

c0d5ff6

As discussed in dalek-cryptography#456 this sets well known defaults for cfg(target_family = "wasm")
and cfg(target_arch = "arm") for 64 bit arithmetric via cfg(curve25519_dalek_bits = "64")

Contributor Author

pinkforest commented Dec 9, 2022

Yeah doing that now 👍

pinkforest added a commit to pinkforest/curve25519-dalek that referenced this issue


          Set well known defaults for wasm and arm

7f8cfd8

As discussed in dalek-cryptography#456 this sets well known defaults for cfg(target_family = "wasm")
and cfg(target_arch = "arm") for 64 bit arithmetric via cfg(curve25519_dalek_bits = "64")

Contributor Author

pinkforest commented Dec 9, 2022 •

edited

PR's up:

Document curve25519_dalek_ backend and bits selection #467

pinkforest mentioned this issue

Document curve25519_dalek_ backend and bits selection #467

Merged

pinkforest added a commit to pinkforest/curve25519-dalek that referenced this issue


          Set well known defaults for wasm and arm

c873f72

As discussed in dalek-cryptography#456 this sets well known defaults for cfg(target_family = "wasm")
and cfg(target_arch = "arm") for 64 bit arithmetric via cfg(curve25519_dalek_bits = "64")

pinkforest added a commit to pinkforest/curve25519-dalek that referenced this issue


          Set well known defaults for wasm and arm

97bd7df

As discussed in dalek-cryptography#456 this sets well known defaults for cfg(target_family = "wasm")
and cfg(target_arch = "arm") for 64 bit arithmetric via cfg(curve25519_dalek_bits = "64")

pinkforest added a commit to pinkforest/curve25519-dalek that referenced this issue


          Fixes curve25519_dalek_bits defaults for cross and wasm

31564a9

build.rs was using cfg(target) but it has to evaluate this from env TARGET
as build.rs cfg(target) in build context is the builder host and not the target.

This change fixes curve25519_dalek_bits lottery to determine the correct
automatic curve25119_dalek_bits with the help of platforms crate.

As discussed in dalek-cryptography#456 this also prepares for well known defaults for wasm and
arm serial backend via cfg(curve25519_dalek_bits = "64")

If the wasm32 or armv7 are going to be u64 serial by default these will be
followed up on later.

pinkforest added a commit to pinkforest/curve25519-dalek that referenced this issue


          Fixes curve25519_dalek_bits defaults for cross and wasm

d914fd6

build.rs was using cfg(target) but it has to evaluate this from env TARGET
as build.rs cfg(target) in build context is the builder host and not the target.

This change fixes curve25519_dalek_bits lottery to determine the correct
automatic curve25119_dalek_bits with the help of platforms crate.

As discussed in dalek-cryptography#456 this also prepares for well known defaults for wasm and
arm serial backend via cfg(curve25519_dalek_bits = "64")

If the wasm32 or armv7 are going to be u64 serial by default these will be
followed up on later.

pinkforest added a commit to pinkforest/curve25519-dalek that referenced this issue


          Fixes curve25519_dalek_bits defaults for cross and wasm

build.rs was using cfg(target) but it has to evaluate this from env TARGET
as build.rs cfg(target) in build context is the builder host and not the target.

This change fixes curve25519_dalek_bits lottery to determine the correct
automatic curve25119_dalek_bits with the help of platforms crate.

As discussed in dalek-cryptography#456 this also prepares for well known defaults for wasm and
arm serial backend via cfg(curve25519_dalek_bits = "64")

If the wasm32 or armv7 are going to be u64 serial by default these will be
followed up on later.

pinkforest added a commit to pinkforest/curve25519-dalek that referenced this issue


          Fixes curve25519_dalek_bits defaults for cross and wasm

b63bf33

build.rs was using cfg(target) but it has to evaluate this from env TARGET
as build.rs cfg(target) in build context is the builder host and not the target.

This change fixes curve25519_dalek_bits lottery to determine the correct
automatic curve25119_dalek_bits with the help of platforms crate.

As discussed in dalek-cryptography#456 this also prepares for well known defaults for wasm and
arm serial backend via cfg(curve25519_dalek_bits = "64")

If the wasm32 or armv7 are going to be u64 serial by default these will be
followed up on later.

pinkforest added a commit to pinkforest/curve25519-dalek that referenced this issue


          Fixes curve25519_dalek_bits defaults for cross and wasm

c32b031

build.rs was using cfg(target) but it has to evaluate this from env TARGET
as build.rs cfg(target) in build context is the builder host and not the target.

This change fixes curve25519_dalek_bits lottery to determine the correct
automatic curve25119_dalek_bits with the help of platforms crate.

As discussed in dalek-cryptography#456 this also prepares for well known defaults for wasm and
arm serial backend via cfg(curve25519_dalek_bits = "64")

If the wasm32 or armv7 are going to be u64 serial by default these will be
followed up on later.

pinkforest added a commit to pinkforest/curve25519-dalek that referenced this issue


          Fixes curve25519_dalek_bits defaults for cross and wasm

2bba4de

build.rs was using cfg(target) but it has to evaluate this from env TARGET
as build.rs cfg(target) in build context is the builder host and not the target.

This change fixes curve25519_dalek_bits lottery to determine the correct
automatic curve25119_dalek_bits with the help of platforms crate.

As discussed in dalek-cryptography#456 this also prepares for well known defaults for wasm and
arm serial backend via cfg(curve25519_dalek_bits = "64")

If the wasm32 or armv7 are going to be u64 serial by default these will be
followed up on later.

pinkforest added a commit to pinkforest/curve25519-dalek that referenced this issue


          Fixes curve25519_dalek_bits defaults for cross and wasm

3d2d3d9

build.rs was using cfg(target) but it has to evaluate this from env TARGET
as build.rs cfg(target) in build context is the builder host and not the target.

This change fixes curve25519_dalek_bits lottery to determine the correct
automatic curve25119_dalek_bits with the help of platforms crate.

As discussed in dalek-cryptography#456 this also prepares for well known defaults for wasm and
arm serial backend via cfg(curve25519_dalek_bits = "64")

If the wasm32 or armv7 are going to be u64 serial by default these will be
followed up on later.

rozbb pushed a commit that referenced this issue


          Fixes curve25519_dalek_bits defaults for cross and wasm (#465)

cb42e87

build.rs was using cfg(target) but it has to evaluate this from env TARGET
as build.rs cfg(target) in build context is the builder host and not the target.

This change fixes curve25519_dalek_bits lottery to determine the correct
automatic curve25119_dalek_bits with the help of platforms crate.

As discussed in #456 this also prepares for well known defaults for wasm and
arm serial backend via cfg(curve25519_dalek_bits = "64")

If the wasm32 or armv7 are going to be u64 serial by default these will be
followed up on later.

pinkforest mentioned this issue

Simplifying backend selection #414

Closed

Contributor

jrose-signal commented Aug 5, 2023

For the record we have re-tested 32-bit Android execution speed with the final 4.0 release and found the following results:

Moto X (2013) is faster with the u64 backend
Moto G4 (2016) is faster with the u64 backend
Pixel 6a (2022) is faster with the u64 backend
Zenfone 9 (2022) is faster with the u32 backend

So in conclusion, there's not going to be a perfect choice for all ARMv7 Android devices, but we've seen more success with the u64 backend than the u32 one, and we'll probably continue using it (it was the default in 3.x as well).

pinkforest mentioned this issue

curve25519: Remove build-dep:platforms in favor of CARGO_CFG_TARGET_POINTER_WIDTH #636

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment