[PATCH] D116994: [RISCV] Add bfp and bfpw intrinsic in zbf extension

[WangLian via Phabricator via cfe-commits]

Jimerlife added inline comments.


================
Comment at: clang/include/clang/Basic/BuiltinsRISCV.def:36
 
+// zbf extension
+TARGET_BUILTIN(__builtin_riscv_bfp, "LiLiLi", "nc", "experimental-zbf")
----------------
craig.topper wrote:
> Capital Z
done


================
Comment at: clang/include/clang/Basic/BuiltinsRISCV.def:37
+// zbf extension
+TARGET_BUILTIN(__builtin_riscv_bfp, "LiLiLi", "nc", "experimental-zbf")
+TARGET_BUILTIN(__builtin_riscv_bfpw, "WiWiWi", "nc", "experimental-zbf,64bit")
----------------
craig.topper wrote:
> I think we should have `__builtin_riscv_bfp_32` and `__builtin_riscv_bfp_64`. It's more convenient and portable for software to be written in terms of number of bits being operated on rather than changing behavior based on xlen.
I have rewrite two intrinsics according to target machine


================
Comment at: llvm/include/llvm/IR/IntrinsicsRISCV.td:98
+  def int_riscv_bfp  : BitManipGPRGPRIntrinsics;
+  def int_riscv_bfpw : BitManipGPRGPRIntrinsics;
+
----------------
craig.topper wrote:
> We only need one intrinsic. `BitManipGPRGPRIntrinsics` is type overloaded. We can check the type in isel patterns.
Only save one intrinsic pattern.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116994