[PATCH] D145088: [RISCV] Add attribute(riscv_rvv_vector_bits(N)) based on AArch64 arm_sve_vector_bits.

Tue Apr 18 18:46:24 PDT 2023

craig.topper added inline comments.

================
Comment at: clang/include/clang/Basic/AttrDocs.td:2332
+
+  #if __RISCV_RVV_VLEN_BITS==512
+  typedef vint8m1_t fixed_vint8m1_t __attribute__((riscv_rvv_vector_bits(512)));
----------------
rjmccall wrote:
> craig.topper wrote:
> > rjmccall wrote:
> > > This probably needs a `defined(__RISCV_RVV_VLEN_BITS)` clause, right?  Because the compiler doesn't actually define this macro unless `-mrvv-vector-bits` is given.
> > I guess so. I copied the documentation from the SVE attribute and modified it to RISC-V.
> Ah, I see.  Yeah, it's probably wrong there, too.
As written the #if would evaluate to false if __RISCV_RVV_VLEN_BITS isn't defined or it's not defined to be 512. The code line it was guarding is using a hardcoded 512.

This isn't how I'd encourage this to be used so I'm changing to #if defined() and will use the preprocessor define in the next line.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145088