[PATCH] D95016: [Clang][RISCV] Add custom TableGen backend for riscv-vector intrinsics.

[Zakk Chen via Phabricator via llvm-commits]

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