[PATCH] D137321: [AArch64][SVE2] Add the SVE2.1 BF16 instructions

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td:3707-3708
+def BFSUB_ZZZ : sve_fp_3op_u_zd<0b00, 0b001, "bfsub", ZPR16>;
+def BFMUL_ZZZ : sve_fp_3op_u_zd<0b00, 0b010, "bfmul", ZPR16>;
+def BFADD_ZPZmZ : sve_fp_2op_p_zds<0b00, 0b0000, "bfadd", ZPR16>;
+def BFSUB_ZPZmZ : sve_fp_2op_p_zds<0b00, 0b0001, "bfsub", ZPR16>;
----------------
Do you mind grouping these by class and adding a space between those groups?


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:2175
 
-class sve_fp_fma_by_indexed_elem<bits<2> sz, bit opc, string asm,
+class sve_fp_fma_by_indexed_elem<bits<2> sz, bit o2, bit opc, string asm,
                                  ZPRRegOp zprty1,
----------------
Can this just be `bits<2> opc`? given it's really a 2-bit opcode and then push this on to the uses of `sve_fp_fma_by_indexed_elem` rather than hiding the fact within `sve_fp_fma_by_indexed_elem`.


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:2196
 
+multiclass sve_fp_bfma_by_indexed_elem<string asm, bit opc> {
+  def NAME : sve_fp_fma_by_indexed_elem<{0, ?}, 0b1, opc, asm, ZPR16, ZPR3b16,
----------------
c-rhodes wrote:
> c-rhodes wrote:
> > single def so this should be a class
> `sve2p1` like the class below?
Whilst true, ultimately we're going to require code gen patterns so we'll need this to be a multiclass later on anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137321