Hi Jie Mei,
> BITSWAP is a byte-wise bitreverse in MIPSr6. For example,
> BITSWAP(0x44332211) => 0x22cc4488.
> DBITSWAP is similar to BITSWAP but for 64 bit.
...
+
+;;
+;;  Swaps (reverses) bits in each byte
+;;
+
+(define_insn "<GPR:d>bitswap"
+  [(set (match_operand:GPR 0 "register_operand" "=d")
+       (unspec:GPR [(use (match_operand:GPR 1 "register_operand" "d"))]
+                   UNSPEC_BITSWAP))]
+  "ISA_HAS_<GPR:D>BITSWAP"
+  "<GPR:d>bitswap\t%0,%1"
+  [(set_attr "type" "bitswap")
+   (set_attr "mode" "<GPR:MODE>")])


This is not a review, just a suggestion/observation.
Instead of introducing a new UNSPEC, have you considered representing the
semantics of this instruction as (bswap:GPR (bitreverse:GPR x)) or
equivalently
(bitreverse:GPR (bswap:GPR x))?

The advantages are that this would allow the RTL optimizers to perform more
simplifications, such a pre-computing this operation on (integer) constant
operands
at compile-time, and (in future) eliminating a BITSWAP followed by a
BITSWAP.
I believe simplify-rtx, should already be able to figure out the
popcount(bitswap(x))
is just popcount(x).

Philosophically, this is related to a proposed patch of mine from 2021:
https://gcc.gnu.org/pipermail/gcc-patches/2021-December/586578.html
that tries to reduce the number of UNSPEC used by the MIPS backend, to
allow the RTL optimizers to make more simplifications (also related to
bswap).


Just a suggestion (perhaps for a follow-up patch).  Don't let these comments
interfere with the usual backend patch review.

Cheers,
Roger
--


Reply via email to