[PATCH] D95016: [Clang][RISCV] Add custom TableGen backend for riscv-vector intrinsics.

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: clang/include/clang/Basic/riscv_vector.td:66
+//      element type which is bool
+//   0: void type, ignores "t"
+//   z: size_t, ignores "t"
----------------
craig.topper wrote:
> jrtc27 wrote:
> > khchen wrote:
> > > jrtc27 wrote:
> > > > Then why aren't these just base types? We don't have to follow the brain-dead nature of printf.
> > > Basically builtin interface is instantiated by the "base type + LMUL" with type transformers. But in some intrinsic function we need a specific type regardless "base type + LMUL"
> > > ex. `vuint32m2_t vssrl_vx_u32m2_vl (vuint32m2_t op1, uint8_t op2, size_t vl);`
> > Then fix the way you define these? This is just bad design IMO.
> For each signature there is effectively a single key type that is a vector. The type transformer is a list of rules for how to derive all of the other operands from that one key type. Conceptually similar to LLVMScalarOrSameVectorWidth or LLVMHalfElementsVectorType in Intrinsics.td. Some types are fixed and don't vary by the key type. Like the size_t vl operand or a store intrinsic returning void.  There is no separate place to put a base type.
Oh I see, I hadn't appreciated that TypeRange got split up and each character was a whole separate intrinsic, I had misinterpreted it as it being a list of the types of arguments, i.e. an N-character string for an (N-1)-argument (plus return type) intrinsic that you could then use as a base and apply transformations too (e.g. "if" for a float-to-int intrinsic, with "vv" etc giving you a vectorised versions) and so was confused as to why the void/size_t/ptrdiff_t/uint8_t/bool-ness couldn't just be pushed into the TypeRange and the corresponding transforms left as "e". But, how _would_ you define vector float-to-int (and back) conversions with this scheme then?

On a related note, I feel one way to make this less obfuscated is change v/w/q/o to be v1/v2/v4/v8 (maybe with the 1 being optional, don't really care), and is also more extensible in future rather than ending up with yet more alphabet soup, though it does change the parsing from being a list of characters to a list of strings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95016