eopXD added a comment.
Thank you for the patch. Few comments here.
================
Comment at: clang/include/clang/Basic/riscv_vector.td:2219
+ def vfwcvt_f_f_v : RVVConvBuiltin<"w", "wv", "f", "vfwcvt_f">;
+ let RequiredFeatures = ["ZvfhminOrZvfh"] in
+ def vfwcvt_f_f_v_fp16 : RVVConvBuiltin<"w", "wv", "x", "vfwcvt_f"> {
----------------
I think using `ZvfhminOrZvfh` is not accurate here. By the v-spec:
> When the Zvfhmin extension is implemented, the vfwcvt.f.f.v and vfncvt.f.f.w
> instructions become defined when SEW=16.
> The Zvfh extension depends on the Zve32f and Zfhmin extensions.
I think making it `let RequiredFeatures = ["Zvfhmin"]` would be clearer.
================
Comment at: clang/lib/Sema/SemaRISCVVectorLookup.cpp:199
+ bool HasZvfh = TI.hasFeature("experimental-zvfh");
+ bool HasZvfhminOrZvfh = TI.hasFeature("experimental-zvfhmin") || HasZvfh;
----------------
I think `HasZvfhmin` is a more accurate naming.
================
Comment at:
clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/zvfhmin-error.c:20
+
+// CHECK-ZVFHMIN-ERR: no matching function for call to '__riscv_vfadd'
+
----------------
If `zvfhmin` is not specified, should the compiler emit semantic error when
encountering `vfloat16*_t` types?
================
Comment at: clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/zvfhmin.c:15
+//
+vfloat16m1_t test_vfncvt_f_f_w_f16m1(vfloat32m2_t src, size_t vl) {
+ return __riscv_vfncvt_f(src, vl);
----------------
This test case is already covered.
https://github.com/llvm/llvm-project/blob/main/clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfncvt.c#L356
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150253/new/
https://reviews.llvm.org/D150253
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
