Re: [PATCH 1/2] match.pd: recognise A > B ? A - B : B - A as abs (A - B) [PR50856]
Andrea Pinski <[email protected]>
| Newsgroups | gmane.comp.gcc.patches |
|---|---|
| Message-ID | <CALvbMcCpNrDt=5oCkvKSMiwHazLezP-nK6oyjSNpii+oGmnosg@mail.gmail.com> |
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. 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. 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 >