[PATCH] D111617: [RISCV] Lazily add RVV C intrinsics.

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng added inline comments.


================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:480
+    // They are handled by riscv_vector.h
+    if (Name == "vsetvli" || Name == "vsetvlimax")
+      continue;
----------------
khchen wrote:
> I feel little tricky to checking the name here. what do you mean they are handled by riscv_vector.h?
> do you mean they have `vsetvl_macro:RVVHeader`?
Yeah, they are defined in riscv_vector.h like this:
```
#define vsetvl_e8mf8(avl) __builtin_rvv_vsetvli((size_t)(avl), 0, 5)
#define vsetvl_e8mf4(avl) __builtin_rvv_vsetvli((size_t)(avl), 0, 6)
#define vsetvl_e8mf2(avl) __builtin_rvv_vsetvli((size_t)(avl), 0, 7)
#define vsetvl_e8m1(avl) __builtin_rvv_vsetvli((size_t)(avl), 0, 0)
#define vsetvl_e8m2(avl) __builtin_rvv_vsetvli((size_t)(avl), 0, 1)
...
```


================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:611
+  for (const auto &SR : SemaRecords) {
+    // Output *MUST* sync with RVVIntrinsicRecord in SemaRVVLookup.cpp.
+    OS << "{"
----------------
khchen wrote:
> I'm thinking is it possible to have an unittest or test to make sure we won't screw up in the future implementation?
> Is it possible to have unittest to test implement really have `sync` correctly?
> Is it easy to debug the mismatch problem during implementation without any new test added?
> We will add a new implementation (really cool speed up and meaningful improvement), but unfortunately we don't have any tests, that make me a little hesitating...
> 
> What do you think?
Change to another way to preventing sync those file manually, thanks for point out that!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111617