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

chore(concrete_cpu): fix nightly feature which was not forwarded to TFHE-rs #654

Merged
merged 1 commit into from
Jan 4, 2024

Conversation

IceTDrinker
Copy link
Member

  • this will allow to have AVX512 on x86 machines which have the hardware feature available if concrete-cpu is built with a rust nightly toolchain and the nightly feature enabled

as discussed and as proposed by @bcm-at-zama I pushed this small fix to make the AVX512 feature from TFHE-rs available in concrete-cpu when the nightly feature is enbaled.

- this will allow to have AVX512 on x86 machines which have the hardware
feature available if concrete-cpu is built with a rust nightly toolchain
and the nightly feature enabled
@IceTDrinker IceTDrinker force-pushed the am/fix/concrete-cpu-nightly branch from 59a92cd to 5833b23 Compare January 3, 2024 14:31
@bcm-at-zama
Copy link
Contributor

@BourgerieQuentin , what about we organize a peer review by TFHE-rs people in Q1, about our integration? To be sure we've the best setting

@bcm-at-zama
Copy link
Contributor

And if it's better for speed, let's make sure we take it for Q4 release!

@BourgerieQuentin
Copy link
Member

@slab-ci compiler-cpu-bench

@slab-ci
Copy link

slab-ci bot commented Jan 3, 2024

Failed to run command: command 'compiler-cpu-bench' not found in config file

@BourgerieQuentin
Copy link
Member

@slab-ci compiler-cpu-benchmark

@BourgerieQuentin
Copy link
Member

where the avx512 instruction are leverage? only in the fft? from benchmark result is difficult to see improvement (is on the line)

@IceTDrinker
Copy link
Member Author

where the avx512 instruction are leverage? only in the fft? from benchmark result is difficult to see improvement (is on the line)

it is code around the fft and data conversion mostly, it's a mismatch compared to what it was in concrete-cpu. I added the nightly feature to be sure that the same code was sure to be used in TFHE-rs (e.g. in concrete-cpu the convert/x86.rs file was making use of the nightly feature https://github.com/zama-ai/concrete/pull/605/files#diff-f2a36b075a72ef8dfb6ec4f65d38151bdadb460c0117eaf78079a1bff204ba60) in TFHE-rs this would be in tfhe/src/core_crypto/fft_impl/fft64/math/fft/x86.rs with nightly-avx512 use pulp::x86::V4; coming from Sarah's pulp SIMD crate

If there is no benefit then I guess it could be left off but be aware that TFHE-rs won't be using AVX512 if available on the machine without that feature

@BourgerieQuentin BourgerieQuentin merged commit 2d3dc73 into main Jan 4, 2024
25 checks passed
@BourgerieQuentin BourgerieQuentin deleted the am/fix/concrete-cpu-nightly branch January 4, 2024 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants