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

Fix AArch64 build when the nightly feature is enabled #36

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AlexTMjugador
Copy link
Contributor

rust-lang/stdarch#1573 stabilized the stdarch_arm_crc32 feature, meaning that it's no longer necessary to enable it to get ARM CRC intrinsics, and doing so in the latest nightlies causes compile errors to happen:

The described build error

Let's get rid of that removed unstable feature and update the documentation accordingly.

rust-lang/stdarch#1573 stabilized the
`stdarch_arm_crc32` feature, meaning that it's no longer necessary to
enable it to get ARM CRC intrinsics, and doing so in the latest
nightlies causes compile errors to happen.

Let's get rid of that removed unstable feature and update the
documentation accordingly.
@srijs
Copy link
Owner

srijs commented May 20, 2024

Hi, thanks for the PR! It's great to see that the feature is finally stabilized.

However, the main issue that I see is that removing the nightly guard completely will mean that older rust version (our MSRV is currently 1.46) will then fail to build on aarch64.

I've just removed our version detection in fb5bf16, which I'm open to bring back to guard this feature for rust >= 1.80, but I'm also open to other suggestions!

@srijs
Copy link
Owner

srijs commented May 20, 2024

I've published a short term fix in 1.4.2 to make the nightly feature compile again, but what's still missing is moving the aarch64 CRC32 feature from behind the nightly flag to general availability for Rust version >= 1.80.

@AlexTMjugador
Copy link
Contributor Author

Hi, thanks for the review!

I made the PR in a bit of a hurry and didn't think about older stable Rust version support, sorry.

That being said, if we don't want to bump MSRV at the moment, I think the best we can do is bringing back some form of rustc version detection, to only enable AArch64 SIMD intrinsics on versions that have this feature stabilized. Would you mind if I repurpose this PR to do exactly that?

@srijs
Copy link
Owner

srijs commented May 20, 2024

No worries - yep that sounds fine, let's use this PR then for the proper fix 👍

Feel free to copy most of the old code that got removed in fb5bf16 and adapt it, in general I'd prefer not adding another dependency just for this.

@paolobarbolini
Copy link

Is there any chance of this getting merged? Rust 1.84 will stabilize the cargo MSRV aware resolver, so it shouldn't be a problem to bump MSRV since users of older Rust versions will easily be able to use a previous version of this crate.

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