evandro marked 7 inline comments as done.
evandro added inline comments.
================
Comment at: clang/include/clang/Basic/RISCVVTypes.def:32
+// - ElBits is the size of one element in bits (SEW).
+//
+// - IsSigned is true for vectors of signed integer elements and
----------------
HsiangKai wrote:
> craig.topper wrote:
> > NF argument isn't documented. And is always 1.
> > NF argument isn't documented. And is always 1.
>
> We could remove it in this patch. The field will be needed when we are going
> to upstream Zvlsseg implementation.
Perhaps we can just note that `NF` is going to be used by Zvlsseg instead.
================
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)
----------------
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.
================
Comment at: clang/lib/AST/ASTContext.cpp:2178
+ Width = 0; \
+ Align = 128; \
+ break;
----------------
HsiangKai wrote:
> craig.topper wrote:
> > Does this alignment need to be this high? The VMV0 register class in the
> > backend has an alignment of 64 for spills. Just wondering why they aren't
> > consistent.
> > Does this alignment need to be this high? The VMV0 register class in the
> > backend has an alignment of 64 for spills. Just wondering why they aren't
> > consistent.
>
> Indeed, it should be 64.
> > Does this alignment need to be this high? The VMV0 register class in the
> > backend has an alignment of 64 for spills. Just wondering why they aren't
> > consistent.
>
> Indeed, it should be 64.
Good catch!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92715/new/
https://reviews.llvm.org/D92715
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
