This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Adding Neon Sm3 & Sm4 Intrinsics
ClosedPublic

Authored by rsanthir.quic on Jan 28 2021, 6:27 PM.

Details

Summary

This adds SM3 and SM4 Intrinsics support for AArch64, specifically:

vsm3ss1q_u32
vsm3tt1aq_u32
vsm3tt1bq_u32
vsm3tt2aq_u32
vsm3tt2bq_u32
vsm3partw1q_u32
vsm3partw2q_u32
vsm4eq_u32
vsm4ekeyq_u32

Diff Detail

Event Timeline

rsanthir.quic created this revision.Jan 28 2021, 6:27 PM
rsanthir.quic requested review of this revision.Jan 28 2021, 6:27 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJan 28 2021, 6:27 PM

This is the first in a series of patches that will address the following:
https://bugs.llvm.org/show_bug.cgi?id=47828

labrinea added inline comments.
llvm/test/CodeGen/AArch64/neon-sm4-sm3.ll
25

The forth argument (<4 x i32> %d) is redundant.

78

Shouldn't the registers be the other way around: sm4e v0.4s, v1.4s ? I believe the reason this happens is because of how CryptoRRTied is defined in llvm/lib/Target/AArch64/AArch64InstrFormats.td:

class CryptoRRTied<bits<1>op0, bits<2>op1, string asm, string asmops>
  : BaseCryptoV82<(outs V128:$Vd), (ins V128:$Vn, V128:$Vm), asm, asmops,
                  "$Vm = $Vd", []> {

Vd be should be the first source register (as well as destination register) and Vn should be the second source register.

lebedev.ri retitled this revision from Adding Neon Sm3 & Sm4 Intrinsics to [AArch64] Adding Neon Sm3 & Sm4 Intrinsics.Jan 29 2021, 5:23 AM
rsanthir.quic marked 2 inline comments as done.Jan 29 2021, 1:14 PM

Thank you for taking a look at this @labrinea !

llvm/test/CodeGen/AArch64/neon-sm4-sm3.ll
78

I see what you mean, this has the added effect of correcting SHA512SU0 as well

rsanthir.quic marked an inline comment as done.

Corrected register ordering for sm4e and removed redundant argument in sm3ss1 test

labrinea accepted this revision.Jan 30 2021, 7:26 AM

LGTM, thanks @rsanthir.quic !

This revision is now accepted and ready to land.Jan 30 2021, 7:26 AM
apazos added a subscriber: apazos.Feb 1 2021, 9:48 AM

@t.p.northover Could you take a look before I ask for this to be merged?

apazos added inline comments.Feb 9 2021, 11:13 AM
clang/test/CodeGen/aarch64-neon-sm4-sm3-invalid.c
8 ↗(On Diff #320199)

Consider renaming the test with range-check suffix so it is clear it is checking for invalid range for the immediate
In addition, consider rewriting it with //expected-error checks

clang/test/CodeGen/aarch64-neon-sm4-sm3.c
6

The test is written for aarch64 triple, in addition @t.p.northover might want to add arm64 triple checks. Tim what do you think?

t.p.northover added inline comments.Feb 9 2021, 12:07 PM
clang/test/CodeGen/aarch64-neon-sm4-sm3.c
6

Thanks for thinking of us, but generally I don't think it's worth duplicating tests along those lines, it's really difficult to write something that cares about the difference between arm64 and aarch64.

linux-gnu vs apple-ios (or others, these days) is more likely to be an issue, but even there I tend to think it's fine to just check one unless there's a specific reason to think it'll affect the targets differently.

Updated test name and checks performed

@labrinea could you commit this when you have some time please?

This revision was automatically updated to reflect the committed changes.