4vtomat added inline comments.

================
Comment at: clang/include/clang/Basic/riscv_vector.td:2405
+  defvar suffix = !if(IsVV, "vv", "vi");
+  defvar prototype = !if(IsVV, "UvUvUvUv", "UvUvUvKz");
+  defm "" : RVVBuiltinSet<NAME, type_range, [[suffix, "Uv", prototype]], [-1, 
2]>;
----------------
craig.topper wrote:
> Can we split this into two classes and get rid of IsVV?
Sure, good idea~


================
Comment at: clang/lib/Sema/SemaChecking.cpp:4693
+  }
+  case RISCVVector::BI__builtin_rvv_vaesdf_vv:
+  case RISCVVector::BI__builtin_rvv_vaesdf_vs:
----------------
craig.topper wrote:
> Are there `tu` versions of these builtins?
Yes, there are, thanks for reminding!


================
Comment at: 
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vwsll.c:12
+//
+vint16mf4_t test_vwsll_vv_i16mf4(vint8mf8_t op1, vint8mf8_t op2, size_t vl) {
+  return __riscv_vwsll_vv_i16mf4(op1, op2, vl);
----------------
craig.topper wrote:
> craig.topper wrote:
> > It doesn't make sense for op2 to be signed. It's a shift amount, its always 
> > a positive number
> The spec doesn't define signed versions of these instrinsics from what I can 
> see.
You are right, maybe it should only be signed version!


================
Comment at: clang/test/Sema/zvk-invalid.c:19
+  __riscv_vaesdf_vv_u32mf2(vd, vs2, vl); // expected-error {{RISC-V type 
'vuint32mf2_t' (aka '__rvv_uint32mf2_t') requires the 'zvl256b' extension}}
+}
----------------
craig.topper wrote:
> Test a _vs intrinsic since the vs operand has a different type than the 
> result?
Sure~


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138810

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to