Skip to content

[AArch64][SVE] Mark AES instructions commutable. #142919

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jun 9, 2025
Merged

[AArch64][SVE] Mark AES instructions commutable. #142919

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0395584
Precommit tests.
rj-jesus Jun 4, 2025
52274fe
[AArch64][SVE] Mark AES instructions commutable.
rj-jesus Jun 4, 2025
211cb2e
Move pattern to main class.
rj-jesus Jun 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4064,8 +4064,8 @@ let Predicates = [HasSVE2_or_SME] in {

let Predicates = [HasSVEAES, HasNonStreamingSVE2_or_SSVE_AES] in {
// SVE2 crypto destructive binary operations
defm AESE_ZZZ_B : sve2_crypto_des_bin_op<0b00, "aese", ZPR8, int_aarch64_sve_aese, nxv16i8>;
defm AESD_ZZZ_B : sve2_crypto_des_bin_op<0b01, "aesd", ZPR8, int_aarch64_sve_aesd, nxv16i8>;
defm AESE_ZZZ_B : sve2_crypto_des_bin_op<0b00, "aese", ZPR8, int_aarch64_sve_aese, nxv16i8, /*commutable=*/1>;
defm AESD_ZZZ_B : sve2_crypto_des_bin_op<0b01, "aesd", ZPR8, int_aarch64_sve_aesd, nxv16i8, /*commutable=*/1>;

// SVE2 crypto unary operations
defm AESMC_ZZ_B : sve2_crypto_unary_op<0b0, "aesmc", int_aarch64_sve_aesmc>;
Expand Down
4 changes: 3 additions & 1 deletion llvm/lib/Target/AArch64/SVEInstrFormats.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9254,7 +9254,9 @@ class sve2_crypto_des_bin_op<bits<2> opc, string asm, ZPRRegOp zprty>
}

multiclass sve2_crypto_des_bin_op<bits<2> opc, string asm, ZPRRegOp zprty,
SDPatternOperator op, ValueType vt> {
SDPatternOperator op, ValueType vt,
bit commutable = 0> {
let isCommutable = commutable in
Copy link
Contributor Author

@rj-jesus rj-jesus Jun 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a cleaner/preferable way of doing this?

Copy link
Collaborator

@paulwalker-arm paulwalker-arm Jun 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you just wrap the two instruction definitions in let isCommutable = 1 in {}? or is the pattern matching def preventing that?

Copy link
Contributor Author

@rj-jesus rj-jesus Jun 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that first, but the pattern matching gets in the way.
We can't set isCommutable unconditionally in sve2_crypto_des_bin_op either (just for the instruction def) because SM4E uses the same multiclass.
Is it worth creating a dedicated multiclass just for AES instructions as Neon seems to have?

Copy link
Collaborator

@paulwalker-arm paulwalker-arm Jun 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, worst come to worst I'll be happy with the current approach.

Just before that though... The multiclass only has a single pattern and we're unlikely to add more so I think you can remove the multiclass and change the main class's DAG from [] to [(set (vt zprty:$Zdn), (op (vt zprty:$_Zdn), (vt zprty:$Zm)))] and then you'll be able to wrap the relevant definitions with isCommutable.

Copy link
Contributor Author

@rj-jesus rj-jesus Jun 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thanks very much, that looks much better. :)
(Please let me know if I missed or misunderstood anything.)

def NAME : sve2_crypto_des_bin_op<opc, asm, zprty>;
def : SVE_2_Op_Pat<vt, op, vt, vt, !cast<Instruction>(NAME)>;
}
Expand Down
22 changes: 22 additions & 0 deletions llvm/test/CodeGen/AArch64/sve2-intrinsics-crypto.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,17 @@ define <vscale x 16 x i8> @aesd_i8(<vscale x 16 x i8> %a, <vscale x 16 x i8> %b)
ret <vscale x 16 x i8> %out
}

define <vscale x 16 x i8> @aesd_i8_commuted(<vscale x 16 x i8> %a,
; CHECK-LABEL: aesd_i8_commuted:
; CHECK: // %bb.0:
; CHECK-NEXT: aesd z0.b, z0.b, z1.b
; CHECK-NEXT: ret
<vscale x 16 x i8> %b) {
%out = call <vscale x 16 x i8> @llvm.aarch64.sve.aesd(<vscale x 16 x i8> %b,
<vscale x 16 x i8> %a)
ret <vscale x 16 x i8> %out
}

;
; AESIMC
;
Expand Down Expand Up @@ -43,6 +54,17 @@ define <vscale x 16 x i8> @aese_i8(<vscale x 16 x i8> %a, <vscale x 16 x i8> %b)
ret <vscale x 16 x i8> %out
}

define <vscale x 16 x i8> @aese_i8_commuted(<vscale x 16 x i8> %a,
; CHECK-LABEL: aese_i8_commuted:
; CHECK: // %bb.0:
; CHECK-NEXT: aese z0.b, z0.b, z1.b
; CHECK-NEXT: ret
<vscale x 16 x i8> %b) {
%out = call <vscale x 16 x i8> @llvm.aarch64.sve.aese(<vscale x 16 x i8> %b,
<vscale x 16 x i8> %a)
ret <vscale x 16 x i8> %out
}

;
; AESMC
;
Expand Down
Loading