[PATCH] D78995: [SveEmitter] NFCI: Describe splat operands like any other modifier

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen marked an inline comment as done.
sdesmalen added a comment.

Thanks for having a look @SjoerdMeijer



================
Comment at: clang/utils/TableGen/SveEmitter.cpp:68
   TypeSpec TS;
+  bool IsSplat;
   bool Float, Signed, Immediate, Void, Constant, Pointer;
----------------
SjoerdMeijer wrote:
> I was wondering if IsSplat belongs here or somewhere else. It doesn't sound like a type, like the other members do, but more a specific case of a vector? But saying this off the top of my head, haven't looked at the code and the context, but perhaps you can comment on this if this is where it should be or not.
This is for type modifiers like `a` which is defined as:
  a: scalar of element type (splat to vector type)

which is used in the definition of the `_n` forms of builtins like:
  svuint16_t svadd_n_u16_z(svbool_t pg, svuint16_t op1, uint16_t op2)

Here `op2` is `uint16_t` instead of `svuint16_t`. This operand is conceptually splat to a SVE vector for the operation (that is what the instruction will do). This is also how we implement it in CGBuiltin, because there is only a LLVM IR intrinsic for the (vector, vector) form. In CodeGen we may map the (vector, vector) form back onto a (vector, imm) instruction if the value fits the immediate.

Because it is part of the modifier, I thought it made sense to add that information to SVEType so that `applyModifier` can set that property, avoiding the very opaque `Proto.find_first_of("ajfrKLR")` in `hasSplat` to determine this. Maybe it isn't strictly part of the type (although you can probably also argue it kind of is), but in any case I thought might make the code a bit more readable because now all such information can live in one place (`applyModifier`).


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

https://reviews.llvm.org/D78995