[PATCH] D96381: [AArch64] Adding SHA3 Intrinsics support

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

The approach for xar looks fine to me, matches how we handled vcvt_n_* (https://developer.arm.com/architectures/instruction-sets/simd-isas/neon/intrinsics?search=vcvt_n_).



================
Comment at: clang/include/clang/Basic/arm_neon.td:1139
+def BCAX : SInst<"vbcax", "....", "QUcQUsQUiQUlQsQcQiQl">;
+def EOR3 : SInst<"veor3", "....", "QUcQUsQUiQUlQsQcQiQl">;
+def RAX1 : SInst<"vrax1", "...", "QUl">;
----------------
Put the second set in c-s-i-l order like the first.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:928
+
+class SHA512H_pattern<Instruction INST, Intrinsic OpNode>
+  : Pat<(v2i64 (OpNode (v2i64 V128:$Vd), (v2i64 V128:$Vn), (v2i64 V128:$Vm))),
----------------
This is unused.


================
Comment at: llvm/test/CodeGen/AArch64/neon-sha3.ll:105
+declare <4 x i32> @llvm.aarch64.crypto.eor3s.v4i32(<4 x i32>, <4 x i32>, <4 x i32>)
+declare <2 x i64> @llvm.aarch64.crypto.eor3s.v2i64(<2 x i64>, <2 x i64>, <2 x i64>)
+
----------------
I'm not sure you need to declare variants you aren't using but in any case you should test the missing ones e.g. crypto.eor3u.v8i16.
Maybe there's an argument that it isn't very useful to test them all like this but the other files follow this pattern so might as well.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96381/new/

https://reviews.llvm.org/D96381