Skip to content

Remove the aes feature from many arm intrinsics#2110

Open
adamgemmell wants to merge 1 commit into
rust-lang:mainfrom
adamgemmell:dev/adagem01/aes-feature
Open

Remove the aes feature from many arm intrinsics#2110
adamgemmell wants to merge 1 commit into
rust-lang:mainfrom
adamgemmell:dev/adagem01/aes-feature

Conversation

@adamgemmell
Copy link
Copy Markdown
Contributor

@adamgemmell adamgemmell commented May 8, 2026

Fixes #2109

The aes rust feature represents FEAT_AES and FEAT_PMULL - from the architecture reference manual, these just add 4 new aes* instructions, and the p64 variants of PMULL/PMULL2.

Clang's header doesn't seem to gate any poly types on the aes feature, and many of the intrinsics with the aes feature are nops or just load/stores, for which the aes feature shouldn't matter. Accordingly, I've removed the aes feature from all intrinsics except the vaes* intrinsics, and ones using the p64 variants of PMULL[2] instructions

The exceptions to this are the vaes* intrinsics, and ones using the P64
variants of PMULL[2] instructions
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 8, 2026

r? @folkertdev

rustbot has assigned @folkertdev.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: @Amanieu, @folkertdev, @sayantn
  • @Amanieu, @folkertdev, @sayantn expanded to Amanieu, folkertdev, sayantn
  • Random selection from Amanieu, folkertdev, sayantn

@folkertdev
Copy link
Copy Markdown
Contributor

This changes public, stable signatures. I don't think it can be observed (it's just removing a constraint), but we should be careful.

Is there any way to validate that the new versions in fact work? I suspect that our CI just has the aes feature available anyway, so if any of these functions did still generate an aes instruction we'd not notice. Maybe qemu can be configured to just have neon, and not aes locally?

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 10, 2026

☔ The latest upstream changes (possibly #2111) made this pull request unmergeable. Please resolve the merge conflicts.

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.

Should the p64 functions be gated under aes?

3 participants