kmclaughlin added a comment.

Hi @bryanpkc, thank you for updating this patch & applying the previous review 
comments here too.
I just have a couple of minor suggestions:



================
Comment at: clang/include/clang/Basic/arm_sme.td:103
+    def NAME # _H : SInst<"svwrite_hor_" # n_suffix # "[_{d}]", "vimiPd", t, 
MergeOp1,
+                          "aarch64_sme_write" # !cond(!eq(n_suffix, "za128") : 
"q", true: "") # "_horiz",
+                          [IsWrite, IsStreaming, IsSharedZA], ch>;
----------------
This is only a suggestion, but would it make the multiclasses simpler to just 
pass in either `"q"` or `""` depending on the instruction, and append this to 
`aarch64_sme_read/write`?


================
Comment at: clang/test/CodeGen/aarch64-sme-intrinsics/acle_sme_read.c:2
+// REQUIRES: aarch64-registered-target
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sme 
-target-feature +sve -S -O1 -Werror -emit-llvm -o - %s | FileCheck %s 
-check-prefixes=CHECK,CHECK-C
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sme 
-target-feature +sve -S -O1 -Werror -emit-llvm -o - -x c++ %s | FileCheck %s 
-check-prefixes=CHECK,CHECK-CXX
----------------
I think `-target-feature +sve` can be removed from this test and 
`acle_sme_write.c`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128648

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to