> From: "Roger Sayle" <[email protected]> > Date: Tue, 4 Aug 2026 19:43:39 +0100
> Hi H-P and Jeff, > I appreciate that folks are on vacation at the moment, but I thought I'd > post this for the record, there's no rush/urgency on a review. Thanks! > This patch has been tested on a cross-compiler to cris-elf with > "make" and "make check RUNTESTFLAGS="--target_board=cris-sim" with > no new failures. Ok for mainline? It helps patch reviewing (and even more when adopting and committing) if you post the result of "git format-patch", not anything else like the output of "git diff". Minor stuff needs fixing, though I think I'll adopt this one. The requested changes are minor enough that I won't add myself as author. The only main issue is, that I'm curious why there are different orders for the "not" part of the combined operations in your rotsi_16_not as opposed to the other bit-negated variants, like bswapsi_not: > +(define_insn "<acc><anz><anzvc>rotsi2_16_not<setcc><setnz><setnzvc>" > + [(set (match_operand:SI 0 "register_operand" "=r") > + (not:SI > + (rotate:SI (match_operand:SI 1 "register_operand" "0") > + (const_int 16)))) > +(define_insn "<acc><anz><anzvc>bitreversesi2_not<setcc><setnz><setnzvc>" > + [(set (match_operand:SI 0 "register_operand" "=r") > + (bitreverse:SI > + (not:SI (match_operand:SI 1 "register_operand" "0")))) Thoughts? Was either order a deliberate choice and if so, what was the reasoning? I'd say the second one is the canonical representation, except I can't find *specific* support for that in md.texi. It covers "not" moved into the operands of "and" and "or" (transforming the operation per DeMorgan's), not general bitwise operations. Maybe just a doc omission? Anyway, an unexpected inconsistency. > 2026-08-04 Roger Sayle <[email protected]> > > gcc/ChangeLog > * gcc/config/cris/cris.md (<...>bswapsi2_not<...>): New define_insn. > (cris_swap_bits): Rename to... > (<...>bitreversesi2<...>): Renamed from cris_swap_bits. > (<...>bitreversesi2_not<...>): New define_insn. > (<...>rotsi2_16<...>): Likewise. > (<...>rotsi2_16_not<...>): Likewise. Not a fan of the abbreviated names. I prefer spelling out <acc><anz><anzvc>bswapsi2_not<setcc><setnz><setnzvc> ...except that the <acc><anz><anzvc> subst-attr prefixes there (and in most other places) are not needed. You only need that on patterns that are supposed to be named, in order to anonymize the define_subst:ed variants, by prefixing the name with "*". Strictly speaking, it's not *needed*, but it's preferable to not emit a lot of unused gen_<name> functions. > diff --git a/gcc/config/cris/cris.md b/gcc/config/cris/cris.md > +(define_insn "<acc><anz><anzvc>bswapsi2_not<setcc><setnz><setnzvc>" (Per above, just "*bswapsi2_not<setcc><setnz><setnzvc>".) > +;; Takes 2-cycles but is shorter than a BITREVERSE and a NOT. Good catch, I had to look that one up. Though, I'd say "one cycle extra". BTW: the extra cycle is not modelled in the simulator colocated with gdb. I'll chew on this, to minimize the amount of Columboism on my part and likely commit it later today. Thanks for the patch! brgds, H-P
