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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a subscriber: dtemirbulatov.
spatel added inline comments.


================
Comment at: llvm/include/llvm/IR/IntrinsicsRISCV.td:99
+
+def int_riscv_bmatflip : BitmanipGprIntrinsic;
+
----------------
xbolva00 wrote:
> s.egerton wrote:
> > 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?
> InstCombine could benefit from smin/smax/umin/umax intrinsic
> 
> @spatel ?
I agree, and I raised that possibility ~3 years ago (and it was debated well before that time too):
http://lists.llvm.org/pipermail/llvm-dev/2016-November/106868.html
...but the consensus was that we could pattern match our way out of it (and adjust the cost models to deal with it), so the benefit of adding intrinsics was not worth the cost. 

If there's new evidence to suggest we should have those intrinsics, please send a note to llvm-dev. Then, it can be seen by more people. It's not clear to me how this patch review would be changed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66479