[PATCH] D82345: [sve][acle] Implement some of the C intrinsics for brain float.

[Cullen Rhodes via Phabricator via llvm-commits]

c-rhodes added a comment.

@fpetrogalli thanks for updating! I have a few more comments, sorry I missed a few things yesterday



================
Comment at: clang/include/clang/Basic/arm_sve.td:725-727
+  def NAME   : SInst<n, p,   "csilUcUsUiUlhfd", mt,    i>;
+  let ArchGuard = "defined(__ARM_FEATURE_SVE_BF16)" in {
+    def _BF16: SInst<n, p,   "b", mt,    i>;
----------------
nit: could you fix the spacing? I don't think it's worth trying to keep the two defs inline, single spaces everywhere would do


================
Comment at: clang/include/clang/Basic/arm_sve.td:1198
 def SVDUPQ_LANE  : SInst<"svdupq_lane[_{d}]", "ddn",  "csilUcUsUiUlhfd", MergeNone, "aarch64_sve_dupq_lane">;
+let ArchGuard = "defined(__ARM_FEATURE_SVE_BF16) && defined(__ARM_FEATURE_BF16_SCALAR_ARITHMETIC) " in {
+  def SVDUPQ_LANE_BF16  : SInst<"svdupq_lane[_{d}]", "ddn",  "b", MergeNone, "aarch64_sve_dupq_lane">;
----------------
`__ARM_FEATURE_BF16_SCALAR_ARITHMETIC` can be removed


================
Comment at: llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td:426-427
+  let Predicates = [HasSVE, HasBF16] in {
+    def : Pat<(nxv8bf16 (AArch64dup (bf16 FPR16:$src))),
+              (DUP_ZZI_H (INSERT_SUBREG (IMPLICIT_DEF), FPR16:$src, hsub), 0)>;
+  }
----------------
I think we're missing a test for this pattern in `llvm/test/CodeGen/AArch64/sve-vector-splat.ll`? Same applies to dup 0 patterns below.


================
Comment at: llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td:431-436
+  def : Pat<(nxv8f16  (AArch64dup (f16 fpimm0))),  (DUP_ZI_H 0, 0)>;
+  def : Pat<(nxv4f16  (AArch64dup (f16 fpimm0))),  (DUP_ZI_H 0, 0)>;
+  def : Pat<(nxv2f16  (AArch64dup (f16 fpimm0))),  (DUP_ZI_H 0, 0)>;
+  def : Pat<(nxv4f32  (AArch64dup (f32 fpimm0))),  (DUP_ZI_S 0, 0)>;
+  def : Pat<(nxv2f32  (AArch64dup (f32 fpimm0))),  (DUP_ZI_S 0, 0)>;
+  def : Pat<(nxv2f64  (AArch64dup (f64 fpimm0))),  (DUP_ZI_D 0, 0)>;
----------------
formatting changes can be reverted


================
Comment at: llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td:1496-1498
+    def : Pat<(nxv2i64 (bitconvert (nxv8bf16 ZPR:$src))), (nxv2i64 ZPR:$src)>;
+    def : Pat<(nxv8bf16 (bitconvert (nxv2i64 ZPR:$src))), (nxv8bf16 ZPR:$src)>;
+    def : Pat<(nxv8bf16 (bitconvert (nxv8i16 ZPR:$src))), (nxv8bf16 ZPR:$src)>;
----------------
missing tests in `llvm/test/CodeGen/AArch64/sve-bitcast.ll`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82345