khchen added inline comments.
================ Comment at: clang/test/CodeGen/RISCV/riscv-rvv-intrinsics-generic/vadd.c:10 + +// ASM-NOT: warning +#include <riscv_vector_generic.h> ---------------- jrtc27 wrote: > Asm checks are discouraged in Clang. If you want to check for Clang warnings, > use -verify, and in this case you want `// expected-no-diagnostics`. RVV is the scalable vector type similar to SVE, so I added this check. please see https://reviews.llvm.org/D82943. ================ Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:48-55 + bool Float, Bool, Signed; + // Constant indices are "int", but have the constant expression. + bool Immediate; + bool Void; + // const qualifier. + bool Constant; + bool Pointer; ---------------- jrtc27 wrote: > These are poor names; many of them don't sound like bools, and are some of > them not mutually exclusive? If so, an enum would be better. Those variables are used to descript the property of RVVType, I think maybe rename as IsXXX could become more clear. ps. I implement RVVType similar to SveType [[ https://github.com/llvm/llvm-project/blob/main/clang/utils/TableGen/SveEmitter.cpp#L68-L70 | did ]]. Do you mean only mutually exclusive property should be represented in an enum? ================ Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:308 + return false; + if (Float && ElementBitwidth == 8) + return false; ---------------- jrtc27 wrote: > or 1? Clearer to move this into the switch below IMO. This checks illegal type float8_t . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95016/new/ https://reviews.llvm.org/D95016 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
