michaelmaitland added inline comments.

================
Comment at: clang/include/clang/Basic/riscv_vector.td:1856
   def vfwcvt_f_x_v : RVVConvBuiltin<"Fw", "Fwv", "csi", "vfwcvt_f">;
-  def vfwcvt_f_f_v : RVVConvBuiltin<"w", "wv", "xf", "vfwcvt_f">;
+  let RequiredFeatures = ["ZvfhminOrZvfh"] in
+    def vfwcvt_f_f_v : RVVConvBuiltin<"w", "wv", "xf", "vfwcvt_f">;
----------------
michaelmaitland wrote:
> In general, I believe that `vfwcvt_f_f_v` and `vfncvt_f_f_w` do not require 
> Zvfhmin or Zvfh. The only time that these intrinsics require Zvfhmin or Zvfh 
> is when the operands to these intrinsics have EEW=16.
The semantics for `RequiredFeatures` is `Features required to enable for this 
builtin.` Since not all types in the range require the ZvfhminOrZvfh feature, 
it may make sense to do some refactoring:

I think two possible solutions are:
  1. to split def of `vfwcvt_f_f_v` and `vfncvt_f_f_w ` by type_range and the 
type range `x` uses the RequiredFeatures
  2. Use different required features for different type ranges (i.e. 
RequiredFeatures is a list of lists where the outer list is for each type in 
the range, and the inner list is the RequiredFeature for that type.)


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

Reply via email to