[PATCH] D82429: [sve][acle] Add some C intrinsics for brain float types.

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.


================
Comment at: clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_tbl2-bfloat.c:4-5
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -D__ARM_FEATURE_SVE2 -D__ARM_FEATURE_SVE_BF16 -D__ARM_FEATURE_BF16_SCALAR_ARITHMETIC -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve2 -target-feature +bf16 -fallow-half-arguments-and-returns -S -O1 -Werror -Wall -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -D__ARM_FEATURE_SVE_BF16 -D__ARM_FEATURE_BF16_SCALAR_ARITHMETIC -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fallow-half-arguments-and-returns -fsyntax-only -verify -verify-ignore-unexpected=error %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -D__ARM_FEATURE_SVE_BF16 -D__ARM_FEATURE_BF16_SCALAR_ARITHMETIC -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +bf16 -target-feature +sve -fallow-half-arguments-and-returns -fsyntax-only -verify=overload -verify-ignore-unexpected=error %s
+
----------------
Missing a test that checks for warning/error if `__ARM_FEATURE_SVE_BF16` isn't defined, these two run lines are checking that for sve2 where we get an implicit declaration warning. See https://reviews.llvm.org/D82399#change-gND08cruJN2Z for an example


================
Comment at: clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_tbx-bfloat.c:4-5
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -D__ARM_FEATURE_SVE2 -D__ARM_FEATURE_SVE_BF16 -D__ARM_FEATURE_BF16_SCALAR_ARITHMETIC -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve2 -target-feature +bf16 -fallow-half-arguments-and-returns -S -O1 -Werror -Wall -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -D__ARM_FEATURE_SVE_BF16 -D__ARM_FEATURE_BF16_SCALAR_ARITHMETIC -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +bf16 -fallow-half-arguments-and-returns -fsyntax-only -verify -verify-ignore-unexpected=error %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -D__ARM_FEATURE_SVE_BF16 -D__ARM_FEATURE_BF16_SCALAR_ARITHMETIC -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +bf16 -target-feature +sve -fallow-half-arguments-and-returns -fsyntax-only -verify=overload -verify-ignore-unexpected=error %s
+
----------------
same here


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:1024
+
+  def : SVE_2_Op_Pat<nxv8bf16, op, nxv8bf16, nxv8i16, !cast<Instruction>(NAME # _H)>;
 }
----------------
pattern needs to be predicated on `+bf16`. I suspect you might hit the same issue I did here: https://reviews.llvm.org/D82182#inline-758348

I found setting the predicate in the multiclass like that doesn't work as it's nested and there's an outer predicate (where the instruction is defined in `AArch64SVEInstrInfo.td`). For splice I found the following works:
```  let Predicates = [HasBF16, HasSVE] in {
    def : SVE_3_Op_Pat<nxv8bf16, int_aarch64_sve_splice, nxv8i1, nxv8bf16, nxv8bf16, SPLICE_ZPZ_H>;
  }```

by defining the pattern guarded on `+bf16` in `AArch64SVEInstrInfo.td` .


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:1058
 
+  def : Pat<(nxv8bf16 (op nxv8bf16:$Op1, nxv8bf16:$Op2, nxv8i16:$Op3)),
+            (nxv8bf16 (!cast<Instruction>(NAME # _H) (REG_SEQUENCE ZPR2, nxv8bf16:$Op1, zsub0,
----------------
as above, needs to be predicated on `+bf16`


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:1060
+            (nxv8bf16 (!cast<Instruction>(NAME # _H) (REG_SEQUENCE ZPR2, nxv8bf16:$Op1, zsub0,
+                                                                        nxv8bf16:$Op2, zsub1),
+                                                     nxv8i16:$Op3))>;
----------------
nit: space


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:1108
+
+  def : SVE_3_Op_Pat<nxv8bf16, op, nxv8bf16, nxv8bf16, nxv8i16, !cast<Instruction>(NAME # _H)>;
 }
----------------
needs to be predicated on `+bf16`


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:3698
+  def : SVE_3_Op_Pat<nxv8i16, op, nxv8i16, nxv8i1, nxv8f16,  !cast<Instruction>(NAME # _H)>;
+  def : SVE_3_Op_Pat<nxv8i16, op, nxv8i16, nxv8i1, nxv8bf16, !cast<Instruction>(NAME # _H)>;
+  def : SVE_3_Op_Pat<nxv4i32, op, nxv4i32, nxv4i1, nxv4f32,  !cast<Instruction>(NAME # _S)>;
----------------
needs to be predicated on `+bf16`


================
Comment at: llvm/test/CodeGen/AArch64/sve-intrinsics-counting-bits.ll:1
 ; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s 2>%t | FileCheck %s
 ; RUN: FileCheck --check-prefix=WARN --allow-empty %s <%t
----------------
need to add `+bf16` to flags


================
Comment at: llvm/test/CodeGen/AArch64/sve-intrinsics-counting-bits.ll:153-154
+  %out = call <vscale x 8 x i16> @llvm.aarch64.sve.cnt.nxv8bf16(<vscale x 8 x i16> %a,
+                                                               <vscale x 8 x i1> %pg,
+                                                               <vscale x 8 x bfloat> %b)
+  ret <vscale x 8 x i16> %out
----------------
nit: add space for alignment


================
Comment at: llvm/test/CodeGen/AArch64/sve2-intrinsics-perm-tb.ll:1
 ; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve2 < %s | FileCheck %s
 
----------------
need to add +bf16 to flags


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82429