[PATCH] D128648: [Clang][AArch64][SME] Add vector read/write (mova) intrinsics

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added a comment.

Other than the two nits, the patch looks good. Thanks.



================
Comment at: clang/include/clang/Basic/arm_sme.td:82
+    def NAME # _H : SInst<"svread_hor_" # n_suffix # "[_{d}]", "ddPimi", t, MergeOp1,
+                          "aarch64_sme_read" # !cond(!eq(n_suffix, "za128") : "q", true: "") # "_horiz",
+                          [IsRead, IsStreaming, IsSharedZA, IsPreservesZA], ch>;
----------------
I'd prefer to just pass in the LLVM IR intrinsic as a parameter to ZARead than doing this, because that makes it easier to see from the definitions what intrinsic the builtin maps to.  (I have found myself grepping in these files quite regularly)


================
Comment at: clang/include/clang/Basic/arm_sve_sme_incl.td:219
 def IsPreservesZA             : FlagType<0x10000000000>;
+def IsRead                    : FlagType<0x20000000000>;
+def IsWrite                   : FlagType<0x40000000000>;
----------------
nit: IsReadZA ? (and IsWriteZA)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128648