[PATCH] D92715: [Clang][RISCV] Define RISC-V V builtin types

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen added inline comments.


================
Comment at: clang/include/clang/Basic/RISCVVTypes.def:67
+RVV_VECTOR_TYPE_INT("__rvv_int8m2_t",  RvvInt8m2,  RvvInt8m2Ty,  16,  8, 1, true)
+RVV_VECTOR_TYPE_INT("__rvv_int8m4_t",  RvvInt8m4,  RvvInt8m4Ty,  32,  8, 1, true)
+RVV_VECTOR_TYPE_INT("__rvv_int8m8_t",  RvvInt8m8,  RvvInt8m8Ty,  64,  8, 1, true)
----------------
evandro wrote:
> craig.topper wrote:
> > craig.topper wrote:
> > > jrtc27 wrote:
> > > > liaolucy wrote:
> > > > > RISC-V V has too many types, more than 200. All types use builtin types? Is it possible to reduce the number of builtin types?
> > > > Indeed this is madness, what's wrong with just using `__attribute__((vector_size(n)))` on the right type? We should not be encouraging people to write code with architecture-specific types... but if we _really_ need these because RISC-V GCC decided this is how RISC-V V is going to look them can we not just shove them all in a header as typedef's for the architecture-independent attributed types and push that complexity out of the compiler itself?
> > > We are using <vscale x 1 x i64> to specify types in IR. The size of the fixed part is being used to control the LMUL parameter. There is currently no way to spell a scalable vector type in C in a generic way.
> > > 
> > > Alternatively I guess we could make LMUL a parameter to the intrinsic and create the scalable IR types in the frontend based on it?
> > I do wonder why we bothered to have signed and unsigned types. The signedness of the operation should be carried in the intrinsic name.
> Some integer operations distinguish between signed and unsigned.  
> I do wonder why we bothered to have signed and unsigned types. The signedness of the operation should be carried in the intrinsic name.

I think the only good thing for supporting both signed and unsigned type is that are more readable and compiler can does type checking for programmer. 

maybe the alternative way is changing intrinsic naming rule like 

```
vint32m1_t a, b, c;
a = vadd_i32m1(b, c);
vuint32m1_t a, b, c;
a = vadd_u32m1(b, c);
vfloat32m1_t a, b, c;
a = vadd_f32m1(b, c);
```


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

https://reviews.llvm.org/D92715