search

dalek-cryptography / curve25519-dalek

A pure-Rust implementation of group operations on Ristretto and Curve25519
Other
867 stars 439 forks source link

CPU auto-detection feature / cfg gate #529

Closed pinkforest closed 1 year ago

pinkforest commented 1 year ago

This should not block landing #523 and can be done as follow-up

Follow-up from

Cc/ @jrose-signal @koute

pinkforest commented 1 year ago

Some q's

1) Which use-cases would need to disable the CPU detecion ? 1.1) Or is this just in case something goes wrong a.k.a "break glass" ?

2) Is there much use for disabling CPU detection in granular basis 2.1) Or should there be just plain override vs disallowing detection one-by-one ?

3) Should we just re-use cfg(curve25519_dalek_backend) "override" 3.1) which could be simd | simd_avx512 | simd_avx2

4) We already use some hazardous / risky features that should be enabled with care and 4.1) not sure how many (would) use --all-features

5) If consensus is to do feature-set here then should the existing cfg(*_dalek_*) also migrated as features

Having a plain override with cfg() would align to everything else - it would be great to know what use-cases there are to make it easier if it's widely expected to be used ?

jrose-signal commented 1 year ago

One use case for us (Signal) is code size, though not on Intel platforms. If there's ever autodetection for mobile platforms, we might want to turn it off if it's not enough of a speedup to be worth the extra code.

tarcieri commented 1 year ago

I would think that specifying curve25519_dalek_backend would override autodetection

tarcieri commented 1 year ago

If there's ever autodetection for mobile platforms, we might want to turn it off if it's not enough of a speedup to be worth the extra code.

I would think mobile platforms are ARM64, in which case the neon target feature is enabled by default (#457)

pinkforest commented 1 year ago

Yeah consensus here seems to be that override seems to be the best.

There is this discussion re-opened so let's redirect there to simplify things further:

tarcieri commented 1 year ago

Oh sorry, I forgot you opened this issue, but thanks for directing the conversation back to #414

pinkforest commented 1 year ago

Yeah I think we got consensus on this issue so your get-together words it well what needs ot happen overall :)

I mainly tried to gather use-cases to surface where additional complexity would be required but I don't think we have any.