[PATCH] D103691: [RISCV] Add support for BITREVERSE for RVP

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2031-2034
+    // If RVP is enabled, lower BITREVERSE to customized BITREV node.
+    if (Subtarget.hasStdExtP() && Op.getOpcode() == ISD::BITREVERSE)
+      return DAG.getNode(RISCVISD::BITREV, DL, VT, Op.getOperand(0),
+                         DAG.getConstant(Imm, DL, VT));
----------------
jrtc27 wrote:
> Is BITREVI or bitmanip's REV better? Why do two extensions need to define the same instruction differently; can there not be a common sub-extension implemented by both (e.g. just the REV form of GREV), like how scalar crypto is using parts of bitmanip?
It looks BITREVI reverses a range of bits while GREVI reverses nibbles, bytes, words, dwords, etc.?

I think you can also handle i32 bitreverse on RV64 by limiting the BITREVI range to the lower 32 bits? I would like to see that handled in this patch since we handle i32 with GREVIW.

We should probably do i8/i16 for this too, but we don't do it for GREVI so I'm less concerned about matching it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103691