[PATCH] D66479: [RISCV] Add LLVM intrinsics for the Bit Manipulation extension

[Scott Egerton via Phabricator via llvm-commits]

s.egerton added inline comments.


================
Comment at: llvm/include/llvm/IR/IntrinsicsRISCV.td:75
+class BitmanipGprIntrinsic
+    : Intrinsic<[llvm_anyint_ty],
+                [LLVMMatchType<0>],
----------------
These intrinsics all use llvm_anyint_ty. The arguments can currently only be either 32 bit or 64 bit and nothing else. Is there better way to achieve this without using llvm_anyint_ty?


================
Comment at: llvm/include/llvm/IR/IntrinsicsRISCV.td:99
+
+def int_riscv_bmatflip : BitmanipGprIntrinsic;
+
----------------
I haven't added intrinsics for every new instruction. Either because they are too simple (andn, orn, xnor, min and max) or because there is already an IR intrinsic that has the same function (clz, ctz, pcnt, ror, rol, fsl and fsr).
Are there any other IR intrinsics that it would be better to remove?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoB.td:475
+let Predicates = [HasStdExtZbbOrZbp, IsRV32] in {
+  def : Pat<(int_riscv_rev GPR:$rs), (GREVI GPR:$rs, 0b11111)>;
+  def : Pat<(int_riscv_rev2 GPR:$rs), (GREVI GPR:$rs, 0b11110)>;
----------------
I considered using a multiclass for these GREVI patterns and a few other similar patterns in this file. Would this be a better approach?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66479