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

Remove build-time check for stdsimd feature #183

Merged
merged 2 commits into from
Nov 10, 2023

Conversation

Amanieu
Copy link
Contributor

@Amanieu Amanieu commented Nov 1, 2023

This is blocking rust-lang/rust#117372, which replaces stdsimd with more fine-grained features. However the auto-detection in aHash breaks when bootstrapping Rust because it detects the stdsimd feature on the old toolchain which is not present on the newly build libcore.

This PR removes the build-time detection of the stdsimd feature and instead just uses the ARM AES intrinsics directly since they are now stable, but only on AArch64.

This is blocking rust-lang/rust#117372, which replaces `stdsimd` with
more fine-grained features. However the auto-detection in aHash breaks
when bootstrapping Rust because it detects the `stdsimd` feature on the
old toolchain which is not present on the newly build libcore.

This PR removes the build-time detection of the `stdsimd` feature and
instead just uses the ARM AES intrinsics directly since they are now
stable, but only on AArch64.
@tkaitchuck
Copy link
Owner

A couple of questions:
This removes support for AES on arm which is not AArch64. Can we leave that in? What will happen there?
When the linked PR is merged, will older versions of aHash fail to build?

@Amanieu
Copy link
Contributor Author

Amanieu commented Nov 1, 2023

I'm not a fan of probing for nightly features in build.rs since, even if the feature name stays the same, breaking changes could be introduced at any time. This means that aHash could randomly break on a future nightly compiler. It's better to let users opt-in to using nightly-only features with a nightly feature in Cargo.toml (or even a separate feature for each nightly feature).

I could add an arm-simd Cargo feature which uses the unstable arm AES intrinsics, but it would have to be explicitly enabled by users. It could be removed when the arm neon intrinsics are stabilized. Would that be ok?

When the linked PR is merged, will older versions of aHash fail to build?

No, I think this only happened due to a peculiarity of rustc's bootstraping. The build script was run against the old libcore which used the stdsimd feature, but the new libcore that ahash was actually built with didn't have that feature.

@Amanieu
Copy link
Contributor Author

Amanieu commented Nov 2, 2023

I've added back ARM support behind a Cargo feature.

@tkaitchuck tkaitchuck merged commit b583310 into tkaitchuck:master Nov 10, 2023
9 checks passed
@Amanieu
Copy link
Contributor Author

Amanieu commented Nov 14, 2023

Could you publish a new version of the crate?

@Amanieu
Copy link
Contributor Author

Amanieu commented Nov 21, 2023

@tkaitchuck Ping! Can you please publish a new release with this change? This is blocking updates of stdarch is the standard library.

@Amanieu
Copy link
Contributor Author

Amanieu commented Nov 27, 2023

@tkaitchuck Another friendly ping! Could you please publish a new release?

@daxpedda daxpedda mentioned this pull request Dec 15, 2023
waywardmonkeys added a commit to waywardmonkeys/wgpu that referenced this pull request Feb 26, 2024
Rust has removed the `stdsimd` feature: rust-lang/rust#117372
This broke `ahash` for versions prior to `0.8.7`: tkaitchuck/aHash#183
@zliucd
Copy link

zliucd commented Jul 25, 2024

aHash version 0.8.11 works here (rustc 1.78.0-nightly (878c8a2a6 2024-02-29))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants