On Sat, May 16, 2026 at 02:06:35PM +0200, Disservin wrote: > Add missing AArch64 bitreverse expanders so __builtin_bitreverse* > can lower to existing rbit patterns.
I'll defer review to aarch64 maintainers, so just nits. > For QI and HI modes we widen to SI, apply rbit, then shift right by 24 > or 16 respectively. Unlike Clang, this emits an and for the zero-extend; > I am not entirely sure how to avoid this. I'm afraid the aarch64 backend tries hard to keep it. It emits the zero (or sign) extend for all arguments and then uses a SUBREG with the promoted flags as the argument to the expander. If gen_lowpart is used to generate paradoxical subreg, then just thge result of the earlier extend is used, if like in widen_operand gen_lowpart is used on force_reg, CSE "optimizes" it back so that the paradoxical subreg is gone. I think only define_peephole2 trying to match the zero/sign extend followed by bitreverse (with result of the extend dead after that) and then shifted right (with result of bitreverse dead after that) would do that. > gcc/testsuite/ChangeLog: > - * gcc.target/aarch64/bitreverse.c: New test. > > gcc/ChangeLog: > - * config/aarch64/aarch64.md (bitreverse<mode>2, bitreverseqi2, > bitreversehi2): New expanders. > - * config/aarch64/aarch64-simd.md (bitreverse<mode>2): New expander. > > Signed-off-by: Disservin <[email protected]> > --- > gcc/config/aarch64/aarch64-simd.md | 7 ++- > gcc/config/aarch64/aarch64.md | 32 ++++++++++++ > gcc/testsuite/gcc.target/aarch64/bitreverse.c | 50 +++++++++++++++++++ > 3 files changed, 88 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.target/aarch64/bitreverse.c > > diff --git a/gcc/config/aarch64/aarch64-simd.md > b/gcc/config/aarch64/aarch64-simd.md > index 2e142b1e1ee..da66bd36fe9 100644 > --- a/gcc/config/aarch64/aarch64-simd.md > +++ b/gcc/config/aarch64/aarch64-simd.md > @@ -400,6 +400,12 @@ > [(set_attr "type" "neon_rev<q>")] > ) > > +(define_expand "bitreverse<mode>2" > + [(set (match_operand:VB 0 "register_operand") > + (bitreverse:VB (match_operand:VB 1 "register_operand")))] > + "TARGET_SIMD" > + "") This can be defined but nothing will really use it until vectorizer is adjusted to handle vectorization of bitreverse. > --- a/gcc/config/aarch64/aarch64.md > +++ b/gcc/config/aarch64/aarch64.md > @@ -5786,6 +5786,38 @@ > [(set_attr "type" "rbit")] > ) > > +(define_expand "bitreverse<mode>2" > + [(set (match_operand:GPI 0 "register_operand") > + (bitreverse:GPI (match_operand:GPI 1 "register_operand")))] > + "" > + "") The "" "" part is not needed, but it is up to aarch64 maintainers which style they want. > + > +(define_expand "bitreverseqi2" > + [(set (match_operand:QI 0 "register_operand") > + (bitreverse:QI (match_operand:QI 1 "register_operand")))] > + "" > + { > + rtx t = gen_reg_rtx (SImode); > + emit_insn (gen_aarch64_rbitsi (t, gen_lowpart (SImode, operands[1]))); > + t = expand_simple_binop (SImode, LSHIFTRT, t, GEN_INT (24), > + NULL_RTX, false, OPTAB_DIRECT); > + emit_move_insn (operands[0], gen_lowpart (QImode, t)); > + DONE; > + }) > + > +(define_expand "bitreversehi2" > + [(set (match_operand:HI 0 "register_operand") > + (bitreverse:HI (match_operand:HI 1 "register_operand")))] > + "" > + { > + rtx t = gen_reg_rtx (SImode); > + emit_insn (gen_aarch64_rbitsi (t, gen_lowpart (SImode, operands[1]))); > + t = expand_simple_binop (SImode, LSHIFTRT, t, GEN_INT (16), > + NULL_RTX, false, OPTAB_DIRECT); > + emit_move_insn (operands[0], gen_lowpart (HImode, t)); > + DONE; > + }) These two IMHO should be done differently, will post a patch momentarily. > +} > \ No newline at end of file Please avoid these. Jakub
