craig.topper added inline comments.

================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:17944
+
+  // P extension
+#define EMIT_BUILTIN(NAME, INT) \
----------------
Please put this above the vector extension since the comment says "vector 
builtins are handled from here"


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:18201
+  BUILTIN_AAAB_WITH_V(kmmawt2_u)
   }
 
----------------
Please undef the macros when you're done with them.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:17944
+
+  // P extension
+#define EMIT_BUILTIN(NAME, INT) \
----------------
Jim wrote:
> I have concern here. It has lots of duplicate code if the code style is the 
> same as B in the top.
> And the name of macro is not good.
Each assignment to IntrinsicTypes invokes the SmallVector constructor so each 
one generates a call in the final binary unless the compiler does a good job of 
merging them.


================
Comment at: llvm/include/llvm/IR/IntrinsicsRISCV.td:1254
+
+multiclass RISCVUnary {
+  def "int_riscv_" # NAME   : RISCVUnaryScalar;
----------------
I haven't followed the P extension closely. Why do we need 2 kinds of 
intrinsics for most instructions?


================
Comment at: llvm/include/llvm/IR/IntrinsicsRISCV.td:1254
+
+multiclass RISCVUnary {
+  def "int_riscv_" # NAME   : RISCVUnaryScalar;
----------------
craig.topper wrote:
> I haven't followed the P extension closely. Why do we need 2 kinds of 
> intrinsics for most instructions?
RISCVUnary is a generic name, but the scalar and vector intrinsic inside make 
it P extension specific.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:762
+      for (unsigned Opc = 0; Opc < ISD::BUILTIN_OP_END; ++Opc)
+        setOperationAction(Opc, VT, Expand);
+
----------------
You probably need handling for insert_vector_elt and extract_vector_elt or some 
of the expansions will probably break.


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

https://reviews.llvm.org/D99158

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

Reply via email to