[PATCH] D136427: [AArch64][SVE2] Add the SVE2.1 fdot instructions

Fri Oct 21 08:46:31 PDT 2022

david-arm marked 2 inline comments as done.
david-arm added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td:3578-3579
 defm FCLAMP_ZZZ : sve2p1_fclamp<"fclamp">;
+def FDOT_ZZZ    : sve_float_dot<0b0, "fdot">;
+def FDOT_ZZZI   : sve_float_dot_indexed<0b0, "fdot">;
 } // End HasSVE2p1_or_HasSME2
----------------
paulwalker-arm wrote:
> david-arm wrote:
> > paulwalker-arm wrote:
> > > Sorry, last drive by comment I promise, but should these be `FDOT_ZZZ_S` and `FDOT_ZZZI_S` to match the existing naming style? As before, I think the existing bfloat classes are not to be followed because they're wrong.  
> > Is that to denote the fact the output size is different to the input, i.e. this is a special case? I just want to make sure we're consistent with naming schemes that's all because the FCLAMP_ZZZ above it doesn't have a _H suffix. It's useful to know for future patches if we should be putting a suffix for the destination element size on everything.
> `FCLAMP_ZZZ` does have this but it's buried within the `sve2p1_fclamp` multiclass.  In general all the SVE instructions end with an element type letter that is usually but not always the result register's element type.  See how SDOT/UDOT is defined. The exception here is that currently only a single variant exists but that might not always be the case.
Doh! Of course the fclamp are in a multiclass. :face_palm

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

https://reviews.llvm.org/D136427