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:
> 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.
================
Comment at: clang/include/clang/Basic/AttrDocs.td:2345
+is enabled under the ``-mrvv-vector-bits`` flag. ``__RISCV_RVV_VLEN_BITS`` can
+only be a power of 2 between 64 and 65536.
+
----------------
rjmccall wrote:
> This doesn't describe the actual behavior of the compiler, which is that it's
> *ill-formed* to use this attribute except when providing the same value to
> `-mrvv-vector-bits`.
>
> Also, this feels like an awkward attempt to also document the
> `__RISCV_RVV_VLEN_BITS` macro, which probably ought to be primarily
> documented in the command line argument reference for `-mrvv-vector-bits`.
> This doesn't describe the actual behavior of the compiler, which is that it's
> *ill-formed* to use this attribute except when providing the same value to
> `-mrvv-vector-bits`.
I think that means the SVE doc is also incorrect?
>
> Also, this feels like an awkward attempt to also document the
> `__RISCV_RVV_VLEN_BITS` macro, which probably ought to be primarily
> documented in the command line argument reference for `-mrvv-vector-bits`.
Ok I'll move it there.
================
Comment at: clang/lib/Basic/Targets/RISCV.cpp:207
+ Builder.defineMacro("__RISCV_RVV_VLEN_BITS",
+ Twine(VScale->first * llvm::RISCV::RVVBitsPerBlock));
}
----------------
rjmccall wrote:
> Is this macro name coming from somewhere specifically? Because it doesn't
> match the normal scheme for RISC-V target macros, which are all lowercase,
> and it doesn't match the name of the command line argument it reflects.
>
> Also, why is the computation of this thing so complicated when the
> command-line argument is basically a single number?
> Is this macro name coming from somewhere specifically? Because it doesn't
> match the normal scheme for RISC-V target macros, which are all lowercase,
> and it doesn't match the name of the command line argument it reflects.
I made it up. I'll reconsider it.
>
> Also, why is the computation of this thing so complicated when the
> command-line argument is basically a single number?
The command line is converted to `-mvscale-min=` and `-mvscale-max=` options
just like SVE. We divide by `llvm::RISCV::RVVBitsPerBlock` where SVE divides by
128.
RISC-V does have a concept of minimum vector length through -march already
which is checked by getVScaleRange to deal with any disagreement. There's a
special value `-mriscv-rvv-vector-bits=zvl` to use the minimum value from
-march without needing to repeat the value.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145088/new/
https://reviews.llvm.org/D145088
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
