Add SVE AES APIs

[a74nh]

Fixes #94423

Also, fix up the exisiting AES tests to use the new helper

[azure-pipelines]

Azure Pipelines:
Successfully started running 6 pipeline(s).
9 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-runtime-intrinsics
See info in area-owners.md if you want to be subscribed.

[a74nh]

@dhartglassMSFT : The final set for .NET11

[jeffhandley]

Note

This review was generated by the holistic code review workflow being iterated on as part of #130339. Please treat its findings as assistive input for human review.

Holistic Review

Motivation: The PR fills a real gap for FEAT_SVE_AES by adding managed SVE AES intrinsics and JIT/codegen support, with a linked approved API proposal (#94423). The area and recent history show this follows the ongoing SVE crypto intrinsic work.

Approach: The JIT/table wiring and generated test strategy generally follow the adjacent SVE SHA3/SM4 and SVE2 patterns. However, the new public API surface in ref/System.Runtime.Intrinsics.cs does not match the API shape approved on the linked issue.

Summary: ❌ Reject. API-approval check failed: the implementation adds extra overloads, omits the approved polynomial-multiply overloads, and changes approved parameter names. Because this PR adds new public API surface, the ref surface needs to match an api-approved shape or the approved proposal needs to be updated before this can merge.

---

Detailed Findings

❌ API Approval — implemented public surface does not match approved shape

The PR adds new public API in src/libraries/System.Runtime.Intrinsics/ref/System.Runtime.Intrinsics.cs (SveAes at lines 7328-7344), so I ran the blocking API-approval check. The linked issue is #94423 and it has api-approved; the label was applied by bartonjs at 2026-01-06T19:31:22Z, immediately after the approval comment #94423 (comment).

The approved shape contains six methods:

Vector<byte> InverseMixColumns(Vector<byte> value)
Vector<byte> MixColumns(Vector<byte> value)
Vector<byte> Decrypt(Vector<byte> left, Vector<byte> right)
Vector<byte> Encrypt(Vector<byte> left, Vector<byte> right)
Vector<ulong> PolynomialMultiplyWideningEven(Vector<ulong> left, Vector<ulong> right)
Vector<ulong> PolynomialMultiplyWideningOdd(Vector<ulong> left, Vector<ulong> right)

The ref implementation instead exposes eight methods at lines 7332-7339:

Vector<byte> Decrypt(Vector<byte> value, Vector<byte> roundKey)
Vector<byte> Encrypt(Vector<byte> value, Vector<byte> roundKey)
Vector<ushort> PolynomialMultiplyWideningEven(Vector<byte> left, Vector<byte> right)
Vector<ulong> PolynomialMultiplyWideningEven(Vector<uint> left, Vector<uint> right)
Vector<ushort> PolynomialMultiplyWideningOdd(Vector<byte> left, Vector<byte> right)
Vector<ulong> PolynomialMultiplyWideningOdd(Vector<uint> left, Vector<uint> right)

Concretely:

• Decrypt/Encrypt parameter names changed from approved left/right to value/roundKey. Public parameter names are API-significant for named arguments.
• The approved Vector<ulong> PolynomialMultiplyWideningEven/Odd(Vector<ulong>, Vector<ulong>) overloads are missing.
• Four unapproved polynomial multiply overloads were added (byte -> ushort and uint -> ulong forms).

There is a later issue comment suggesting the AES parameter names should match the existing AdvSimd AES APIs, but the API-approval procedure treats the comment by the user who applied api-approved as the source of truth. Please either update the implementation to match the approved shape or get the approved API comment/label updated to cover this exact surface.

✅ JIT wiring — follows the expected intrinsic-table pattern

The new SveAes ISA range is wired in src/coreclr/jit/hwintrinsic.cpp, and the intrinsic table entries in src/coreclr/jit/hwintrinsiclistarm64sve.h lines 542-549 follow the neighboring SVE SHA3/SM4/SVE2 table style. The tied-operand SVE AES instructions are handled in emitarm64sve.cpp lines 2325-2333 and 3030-3031, consistent with the existing RMW/movprfx handling used for SVE instructions with tied operands.

✅ Tests — coverage is directionally consistent, but blocked by API shape

The generated-test inputs in src/tests/Common/GenerateHWIntrinsicTests/Arm/SveTests.cs lines 1759-1768 add hardware tests for AESD/AESE/AESIMC/AESMC and PMULLB/PMULLT, and GenerateHWIntrinsicTests_Arm.cs now processes the new SveAesInputs. The helper-based AES expected-value computation also improves the existing AdvSimd AES tests by avoiding fixed single-vector expected data. I did not see a separate test-quality blocker beyond the fact that the tests currently validate the unapproved API shape.

[a74nh]

Concretely:

• Decrypt/Encrypt parameter names changed from approved left/right to value/roundKey. Public parameter names are API-significant for named arguments.
• The approved Vector<ulong> PolynomialMultiplyWideningEven/Odd(Vector<ulong>, Vector<ulong>) overloads are missing.
• Four unapproved polynomial multiply overloads were added (byte -> ushort and uint -> ulong forms).

I believe the implementation is correct, and the API as approved is incorrect.
I've raised two comments on the API review: #94423 (comment) and #94423 (comment)

Could someone approve the changes please. (@jeffhandley ? @tannergooding ?)

[github-actions]

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "0a139c556250d1ce8d1d8e1cf2b980e1adaa5fa1",
  "last_dispatched_base_ref": "main",
  "last_dispatched_base_sha": "6e0eb8307f56dc6fea81aa4cce2cf40c1013fa4d",
  "last_reviewed_commit": "0a139c556250d1ce8d1d8e1cf2b980e1adaa5fa1",
  "last_reviewed_base_ref": "main",
  "last_reviewed_base_sha": "6e0eb8307f56dc6fea81aa4cce2cf40c1013fa4d",
  "last_recorded_worker_run_id": "29686401184",
  "review_attempt_commit": "",
  "review_attempt_base_ref": "",
  "review_attempt_count": 0,
  "max_review_attempts": 5,
  "review_history_format": "holistic-review-disclosure-v1",
  "review_history": [
    {
      "commit": "0a139c556250d1ce8d1d8e1cf2b980e1adaa5fa1",
      "review_id": 4730737503
    }
  ]
}

[github-actions]

Holistic Review

Motivation: Justified. This implements the FEAT_SVE_AES intrinsics (AESD/AESE/AESIMC/AESMC/PMULLB/PMULLT) tracked by the api-approved issue #94423, filling a real gap in the SVE surface and following the same pattern as the existing SveSha3/SveSm4 classes.

Approach: Sound and consistent with the codebase. The managed SveAes/SveAes.PlatformNotSupported classes, ref assembly entries, ILLink substitutions, projitems, JIT intrinsic table (hwintrinsiclistarm64sve.h), ISA range wiring, and the emitarm64sve.cpp encodings all mirror established SVE crypto patterns. The PR also usefully refactors the existing AdvSimd AES tests off the bespoke AesBinOpTest/AesUnOpTest templates onto the shared generated templates plus a new self-contained Helpers.Aes reference implementation.

Summary: ⚠️ Needs Human Review. I found no code correctness bugs — the intrinsic tables, encodings, ref shape, and AES helper reference implementation all look correct and internally consistent. The one item that warrants human confirmation is that the shipped public API surface differs from the shape originally posted with the api-approved label (see Detailed Findings); the author documented the corrections in the issue, but an API reviewer should confirm the final surface is signed off.

---

Detailed Findings

⚠️ API Approval — Shipped surface deviates from the originally approved shape

The api-approved comment on #94423 lists 6 signatures, including exactly two widening overloads:

public static Vector<ulong> PolynomialMultiplyWideningEven(Vector<ulong> left, Vector<ulong> right); // PMULLB
public static Vector<ulong> PolynomialMultiplyWideningOdd(Vector<ulong> left, Vector<ulong> right); // PMULLT

The implementation instead ships four widening overloads with different input/return types (Vector<byte>→Vector<ushort> and Vector<uint>→Vector<ulong>), and also renames the Decrypt/Encrypt parameters to value/roundKey. Both changes are reasonable — the author posted follow-up comments on the issue explaining that the original widening signatures were wrong (the PMULLB/PMULLT forms are .H<-.B and .D<-.S) and that the Decrypt/Encrypt parameter names should match the existing AdvSimd AES APIs. However, these corrections change the approved surface (extra overloads + parameter renames) and do not appear to have been re-blessed by a fresh api-approved comment. A human API reviewer should confirm the final 8-method surface is approved before merge. This is a process/approval concern, not a code defect.

✅ JIT encodings and intrinsic tables — Correct and consistent

hwintrinsiclistarm64sve.h correctly gates the byte lane for the AES ops (INS_sve_aesd/aese/aesimc/aesmc) and the ushort/ulong lanes for pmullb/pmullt, with HW_Flag_HasRMWSemantics on the RMW ops. hwintrinsic.cpp wires the SveAes ISA range in place of the previous NI_Illegal placeholder. In emitarm64sve.cpp, aesd/aese are added to the existing no-movprfx RMW mov-based path alongside sm4e, and aesimc/aesmc get a dedicated unary path that emits a tied mov before the op — both consistent with the instruction constraints (destructive, no predication, .B arrangement).

✅ AES reference helper — Correct

The new Helpers.Aes class is a textbook AES reference: standard forward/inverse S-boxes, GF(2^8) Multiply with the 0x1b reduction polynomial, and MixColumns/InvMixColumns matrices (02/03 and 0e/0b/0d/09). Crucially the operation ordering matches the Arm semantics being validated — AESE = AddRoundKey→SubBytes→ShiftRows and AESD = AddRoundKey→InvSubBytes→InvShiftRows — which differ from the textbook full-round ordering, so this is the right model for these single-instruction intrinsics. Block selection via index & ~0xF correctly processes each 128-bit segment of the SVE vector independently.

✅ Test wiring — Complete

The generated SveAesInputs group covers all 8 methods across the appropriate lane types, is registered in GenerateHWIntrinsicTests_Arm.cs, and new SveAes_r/SveAes_ro runner projects plus Program.SveAes.cs follow the sibling SveSm4 layout. The _SveUnaryOpTestTemplate left/leftOp variable rename is a self-consistent cleanup, and the AdvSimd AES tests are migrated onto the shared templates with matching validation against Helpers.Aes. I did not build or run these (this worker has no baseline); a maintainer should rely on CI to confirm the generated tests pass on an SVE-capable leg.

💡 Observation — Widening test uses a static per-run mask

The PolynomialMultiplyWidening* test cases use NextValueMask = Helpers.getMaskUInt16()/getMaskUInt64(). This mirrors existing widening tests, so it is consistent with the codebase; noting only that these masks constrain the random inputs, which is fine for correctness validation.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 186.9 AIC · ⌖ 11.2 AIC · ⊞ 10K