Re: [PATCH 1/2] match.pd: recognise A > B ? A - B : B - A as abs (A - B) [PR50856]

On Wed, Aug 19, 2026 at 1:10 AM Richard Biener <[email protected]> wrote:
>
> On Tue, 18 Aug 2026, Andrea Pinski wrote:
>
> > On Tue, Aug 18, 2026 at 12:55 PM Dominic P <[email protected]> wrote:
> > >
> > > The absolute-difference idiom is already folded to abs (A - B) when the
> > > comparison is written against the subtraction itself, e.g.
> > > (A - B) > 0 ? (A - B) : (B - A).  The very common alternative spelling that
> > > compares the operands directly, A > B ? A - B : B - A, was not recognised, so
> > > phiopt kept two independent subtractions which the backend if-converted into a
> > > compare plus two predicated subtracts (three instructions on Arm) instead of
> > > the two-instruction "subs; rsb<cc>" abs sequence.
> > >
> > > For a signed, non-wrapping integer type A > B is equivalent to A - B > 0, so the
> > > idiom (and its >=, < and <= variants) folds to [-]abs (A - B).  Add the two
> > > simplifications next to the existing (A - B) cmp 0 family, guarded on
> > > !TYPE_OVERFLOW_WRAPS so that -fwrapv does not trigger the fold.
> > >
> > > As a two-patch series with the following widened-operand extension,
> > > bootstrapped on x86_64-pc-linux-gnu with the stage2/stage3 comparison
> > > successful, and regtested there with gcc.dg/dg.exp and
> > > gcc.dg/tree-ssa/tree-ssa.exp: no unexpected results.
> >
> > Couple of things. First Eikansh posted a patch less than a month ago
> > for this which was under review too:
> > https://inbox.sourceware.org/gcc-patches/[email protected]/T/#m31fbbf878192412099d1c0c9ae066bccc519fb44
> >
> > >
> > > Assisted-by: Claude Opus 5 (Anthropic)
> >
> > Second I think GCC really should have a policy of rejecting patches
> > using LLMs for easy issues. LLVM already has a similar policy. So this
> > should not be so controversial. The main reason is these easy issues
> > are here for learning GCC code base.
>
> True, though I somewhat lean towards progress is better than having
> (easy) bugs.
Progress is hard to measure though. Progress in learning how to
maintain and review and such is just as important as progress in code
generation improvements :).


 > Even with using LLMs this can be a learning process - it
> really depends what "Assisted-by: Claude Opus 5" actually means
> and I'd welcome some more elaboration on such annotations from
> contributors.
>
> > Also our current policy about LLMs can be found at
> > https://gcc.gnu.org/ai-policy.html ; it does not cover easy issues
> > currently. But it seems like we are in a need of one here considering
> > most of the easy issues are going to be small enough.
> >
> > The other thing is I think you should mention which parts were
> > assisted by the LLM; was only the testcases or the whole thing?
> > Our current policy suggest we can only legally insignificant
> > contributions but the GNU policy on legally significant says:
> > "Keep in mind, however, that a series of minor changes by the same
> > person can add up to a significant contribution."
> > So I am not sure if after say one or 2 patches using LLMs for the
> > match patterns can be accepted any more from the same person. This is
> > the vague part of the policy which does need clarifications.
>
> Why should it only become legally significant if it's from the same
> person (or same LLM?) or to the same file?  In the extreme it should
> be multiple LLM contributions to GCC as-whole add up, and I guess
> we're way past a "legally insignificant" threshold there, if you
> think -- narrowed to copyright -- that patches like this would ever
> be able to infringe on somebody elses copyright (not being
> copyrightable should not be a problem).
>
> Richard.
>
> >
> > Thanks,
> > Andrea
> >
> >
> > >
> > >         PR tree-optimization/50856
> > >
> > > gcc/ChangeLog:
> > >
> > >         * match.pd: Fold A > B ? A - B : B - A (and the >=, < and <=
> > >         spellings) into [-]abs (A - B) for signed non-wrapping integer
> > >         types.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > >         * gcc.target/arm/pr50856.c: New test.
> > >         * gcc.dg/tree-ssa/absdiff-1.c: New test.
> > >
> > > Signed-off-by: Dominic P <[email protected]>
> > > ---
> > >  gcc/match.pd                              | 26 +++++++++++++++++
> > >  gcc/testsuite/gcc.dg/tree-ssa/absdiff-1.c | 12 ++++++++
> > >  gcc/testsuite/gcc.target/arm/pr50856.c    | 35 +++++++++++++++++++++++
> > >  3 files changed, 73 insertions(+)
> > >  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/absdiff-1.c
> > >  create mode 100644 gcc/testsuite/gcc.target/arm/pr50856.c
> > >
> > > diff --git a/gcc/match.pd b/gcc/match.pd
> > > index 18d295ad4cb..04ddb82c76e 100644
> > > --- a/gcc/match.pd
> > > +++ b/gcc/match.pd
> > > @@ -7601,6 +7601,32 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > >        (convert (negate (absu:utype @0))))
> > >        (negate (abs @0)))))
> > >   )
> > > + /* A > B ? A - B : B - A   and   A >= B ? A - B : B - A
> > > +    same as abs (A - B) for signed non-wrapping types, where the
> > > +    no-overflow assumption gives A > B <=> A - B > 0.  (PR50856)  */
> > > + (for cmp (gt ge)
> > > +  (simplify
> > > +   (cnd (cmp @1 @2) (minus@0 @1 @2) (minus @2 @1))
> > > +   (if (!HONOR_SIGNED_ZEROS (type)
> > > +       && !TYPE_UNSIGNED (type)
> > > +       && ANY_INTEGRAL_TYPE_P (type)
> > > +       && !TYPE_OVERFLOW_WRAPS (type)
> > > +       && (!VECTOR_TYPE_P (type)
> > > +           || target_supports_op_p (type, ABS_EXPR, optab_vector)))
> > > +    (abs @0))))
> > > + /* A < B ? A - B : B - A   and   A <= B ? A - B : B - A
> > > +    same as -abs (A - B) for signed non-wrapping types.  (PR50856)  */
> > > + (for cmp (lt le)
> > > +  (simplify
> > > +   (cnd (cmp @1 @2) (minus@0 @1 @2) (minus @2 @1))
> > > +   (if (!HONOR_SIGNED_ZEROS (type)
> > > +       && !TYPE_UNSIGNED (type)
> > > +       && ANY_INTEGRAL_TYPE_P (type)
> > > +       && !TYPE_OVERFLOW_WRAPS (type)
> > > +       && (!VECTOR_TYPE_P (type)
> > > +           || (target_supports_op_p (type, ABS_EXPR, optab_vector)
> > > +               && target_supports_op_p (type, NEGATE_EXPR, optab_vector))))
> > > +    (negate (abs @0)))))
> > >  )
> > >
> > >  /* X >=/> 0 ? Y + X : Y - X
> > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/absdiff-1.c b/gcc/testsuite/gcc.dg/tree-ssa/absdiff-1.c
> > > new file mode 100644
> > > index 00000000000..4d59b93429b
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.dg/tree-ssa/absdiff-1.c
> > > @@ -0,0 +1,12 @@
> > > +/* A > B ? A - B : B - A folds to abs (A - B) for signed non-wrapping
> > > +   operands, in all four comparison directions.  PR50856.  */
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-O2 -fdump-tree-optimized" } */
> > > +
> > > +int ad_gt (int a, int b) { return a > b ? a - b : b - a; }
> > > +int ad_ge (int a, int b) { return a >= b ? a - b : b - a; }
> > > +long ad_lt (long a, long b) { return a < b ? a - b : b - a; }
> > > +int ad_le (int a, int b) { return a <= b ? a - b : b - a; }
> > > +
> > > +/* { dg-final { scan-tree-dump-times "ABS_EXPR" 4 "optimized" } } */
> > > +/* { dg-final { scan-tree-dump-times "= -" 2 "optimized" } } */
> > > diff --git a/gcc/testsuite/gcc.target/arm/pr50856.c b/gcc/testsuite/gcc.target/arm/pr50856.c
> > > new file mode 100644
> > > index 00000000000..adae32b09ea
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/arm/pr50856.c
> > > @@ -0,0 +1,35 @@
> > > +/* PR tree-optimization/50856: the signed absolute-difference idiom
> > > +   a > b ? a - b : b - a  (and the >=, and the mirror < / <= giving
> > > +   -abs) should be recognised as ABS_EXPR (a - b) so the ARM backend
> > > +   emits the 2-instruction "subs; rsb<cc>" sequence instead of a
> > > +   compare followed by two predicated subtracts.  */
> > > +
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-O2 -marm" } */
> > > +/* { dg-require-effective-target arm32 } */
> > > +/* { dg-final { scan-tree-dump-times "ABS_EXPR" 3 "optimized" } } */
> > > +/* { dg-additional-options "-fdump-tree-optimized" } */
> > > +
> > > +int
> > > +absdiff_gt (int a, int b)
> > > +{
> > > +  return a > b ? a - b : b - a;
> > > +}
> > > +
> > > +int
> > > +absdiff_ge (int a, int b)
> > > +{
> > > +  return a >= b ? a - b : b - a;
> > > +}
> > > +
> > > +long
> > > +labsdiff_gt (long a, long b)
> > > +{
> > > +  return a > b ? a - b : b - a;
> > > +}
> > > +
> > > +/* The abs sequence is "subs rN, ..; rsb<cc> rN, rN, #0"; make sure we
> > > +   no longer emit the compare + two predicated subtracts form.  */
> > > +/* { dg-final { scan-assembler-not "subgt" } } */
> > > +/* { dg-final { scan-assembler-not "suble" } } */
> > > +/* { dg-final { scan-assembler "rsb" } } */
> > > --
> > > 2.55.0
> > >
> >
>
> --
> Richard Biener <[email protected]>
> SUSE Software Solutions Germany GmbH,
> Frankenstrasse 146, 90461 Nuernberg, Germany;
> GF: Jochen Jaser, Andrew McDonald, Abhinav Puri; (HRB 36809, AG Nuernberg)