[PATCH] D134677: [Clang][AArch64][SME] Add ZA zeroing intrinsics

Sander de Smalen via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 6 01:44:34 PDT 2023


sdesmalen added inline comments.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:9921-9922
     return EmitSMEReadWrite(TypeFlags, Ops, Builtin->LLVMIntrinsic);
+  else if (TypeFlags.isZero())
+    return EmitSMEZero(TypeFlags, Ops, Builtin->LLVMIntrinsic);
 
----------------
Given that the type flags are a little precious (we've already used 42 out of the 64 bits) and there only being a single intrinsic for svzero, can you do something similar to what's done in `EmitAArch64SVEBuiltinExpr` and create a switch on `BuiltinID` instead?


================
Comment at: clang/test/CodeGen/aarch64-sme-intrinsics/acle_sme_zero.c:13
+// CHECK-NEXT:    ret void
+//
+void test_svzero_mask_za() {
----------------
These tests are missing some kind of `ARM_SHARED_ZA_ATTR` macro (similar to what you've done for `ARM_STREAMING_ATTR` in D127910 and D128648), because `svzero` requires ZA state to be available.


================
Comment at: clang/test/Sema/aarch64-sme-intrinsics/acle_sme_imm.cpp:206
 
+ARM_STREAMING_ATTR
+void test_range_0_255(svbool_t pg, void *ptr) {
----------------
svzero is streaming-compatible, but does require shared_za. So you should probably change this to ARM_SHARED_ZA_ATTR (and add the #define for it)


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

https://reviews.llvm.org/D134677



More information about the cfe-commits mailing list