On Mon, Jul 27, 2026 at 3:16 PM Richard Earnshaw
<[email protected]> wrote:
>
> On 27/07/2026 10:17, Richard Earnshaw wrote:
> > On 26/07/2026 15:18, Shivam Gupta wrote:
> >> At -O1, a complemented XOR whose result is masked to bit zero is
> >> currently emitted as:
> >>
> >>     eor     w0, w0, w1
> >>     ubfx    x0, x0, 0, 1
> >>     eor     w0, w0, 1
> >>
> >> Recognize (and (not (xor ...)) 1) and split it into a complemented
> >> XOR followed by an AND, allowing the complemented XOR to use EON:
> >>
> >>     eon     w0, w0, w1
> >>     and     w0, w0, 1
> >>
> >> Tested on aarch64-unknown-linux-gnu with no regressions.
> >
> > Thanks for the patch, some suggestions/comments below.
> >
> >>
> >> gcc/ChangeLog:
> >>
> >>     * config/aarch64/aarch64.md (*aarch64_xor_not_and_one): New
> >>     define_insn_and_split.
> >>
> >> gcc/testsuite/ChangeLog:
> >>
> >>     * gcc.target/aarch64/xor-not-and-one.c: New test.
> >>
> >> Signed-off-by: Shivam Gupta <[email protected]>
> >> ---
> >>   gcc/config/aarch64/aarch64.md                 | 14 +++++++++++
> >>   .../gcc.target/aarch64/xor-not-and-one.c      | 23 +++++++++++++++++++
> >>   2 files changed, 37 insertions(+)
> >>   create mode 100644 gcc/testsuite/gcc.target/aarch64/xor-not-and-one.c
> >>
> >> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/
> >> aarch64.md
> >> index b2185c63819..1a61d7f85ca 100644
> >> --- a/gcc/config/aarch64/aarch64.md
> >> +++ b/gcc/config/aarch64/aarch64.md
> >> @@ -5263,6 +5263,20 @@
> >>   ;; Logical operations
> >>   ;; -------------------------------------------------------------------
> >> +(define_insn_and_split "*aarch64_xor_not_and_one<mode>"
> >
> > Combine can handle 3->2 patterns without the need for intermediate insn
> > patterns, so I think this should just be a define_split (with a suitable
> > scratch clobber operand to hold the intermediate result).
> >
> >> +  [(set (match_operand:GPI 0 "register_operand" "=r")
> >> +    (and:GPI
> >> +      (not:GPI
> >> +        (xor:GPI (match_operand:GPI 1 "register_operand" "r")
> >> +             (match_operand:GPI 2 "register_operand" "r")))
> >> +      (const_int 1)))]
> > The shape of this pattern is generic (eon + and), so there's no need to
> > restrict this to a literal 1.  In fact, there's no real need for the
> > split pattern to restrict it to a constant, and operand that is valid
> > for AND should be OK here.
> >
> > R.
> >
>
> Actually, I've just noticed that you said this was at -O1.  If I compile
> with -O2 (which is what we generally recommend for general use), we
> already generate the eon/and sequence, so I don't think we need anything
> more in this case; the problem is just that your optimization level is
> too low.
>

Hello,

Thank you for taking time to do the review.
I agree, I would drop the patch.

Regards,
Shivam

> The issue with having patterns that need later splitting is that it can
> cause combinatorial explosion problems in the machine description - we
> have to start adding yet more patterns to recognize multiple combinations.
>
> The compilers heuristics are usually very good at handling this
> complexity by splitting things as soon as possible.  So we try to stick
> closely to the 1 insn, 1 instruction rule and only add 1->2+ when there
> is something very critical that can't be handled by other optimizations.
>
> R.

Reply via email to