Re: [PATCH] aarch64: Add missing CC clobber to max/min-of-add/sub patterns [PR116815]

On Tue, Aug 4, 2026 at 1:30=E2=80=AFAM <[email protected]> wrote:
>
> From: Kyrylo Tkachov <[email protected]>
>
> *aarch64_plus_within_<optab><mode>3_<ovf_commutate> and
> *aarch64_minus_within_<optab><mode>3 split into a flag-setting ADDS or SU=
BS
> followed by a CSEL, but their insn patterns do not say that they write th=
e
> condition codes.
>
> For
>
>   unsigned f (unsigned a, unsigned b, unsigned c, unsigned d)
>   {
>     unsigned s =3D a + b;
>     unsigned m =3D s > a ? s : a;
>     return (c < d && a < b) ? m : d;
>   }
>
> combine produces
>
>   (parallel [(set (reg:SI 0 x0)
>                   (umax:SI (plus:SI (reg:SI 107) (reg:SI 108))
>                            (reg:SI 107)))
>              (clobber (scratch:SI))])
>
> which claims to leave the flags alone.  The compare feeding the enclosing
> CCMP chain is therefore treated as still live and is removed, and split1
> then emits an ADDS that overwrites the flags the outer CSEL reads:
>
>   adds  w1, w0, w1
>   csel  w1, w1, w0, cc
>   csel  w0, w1, w3, cc
>
> so f (5, 7, 9, 2) returns 12 rather than 2.
>
> Add the (clobber (reg:CC CC_REGNUM)) that the neighbouring
> *aarch64_minmax_plus pattern already carries.  The comparison is then kep=
t:
>
>   cmp   w2, w3
>   ccmp  w0, w1, 2, cc
>   bcs   .L2
>   adds  w1, w0, w1
>   csel  w3, w1, w0, cc
>
> Bootstrapped and tested on aarch64-none-linux-gnu.
> Pushing to trunk and later to the affected branches.
>
> gcc/ChangeLog:
>
>         PR middle-end/116815
>         * config/aarch64/aarch64.md
>         (*aarch64_plus_within_<optab><mode>3_<ovf_commutate>): Add a
>         clobber of CC_REGNUM.
>         (*aarch64_minus_within_<optab><mode>3): Likewise.

Ok

>
> gcc/testsuite/ChangeLog:
>
>         PR middle-end/116815
>         * gcc.target/aarch64/pr116815-4.c: New test.
>
> Signed-off-by: Kyrylo Tkachov <[email protected]>
> ---
>  gcc/config/aarch64/aarch64.md                 |  6 +-
>  gcc/testsuite/gcc.target/aarch64/pr116815-4.c | 94 +++++++++++++++++++
>  2 files changed, 98 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/pr116815-4.c
>
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.m=
d
> index b2185c63819..59af2bd2088 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -4554,7 +4554,8 @@
>           (plus:GPI (match_operand:GPI 1 "register_operand" "r")
>                     (match_operand:GPI 2 "register_operand" "r"))
>           (match_dup ovf_commutate)))
> -   (clobber (match_scratch:GPI 3 "=3Dr"))]
> +   (clobber (match_scratch:GPI 3 "=3Dr"))
> +   (clobber (reg:CC CC_REGNUM))]
>    "!TARGET_CSSC"
>    "#"
>    "&& 1"
> @@ -4584,7 +4585,8 @@
>           (minus:GPI (match_operand:GPI 1 "register_operand" "r")
>                      (match_operand:GPI 2 "register_operand" "r"))
>           (match_dup 1)))
> -   (clobber (match_scratch:GPI 3 "=3Dr"))]
> +   (clobber (match_scratch:GPI 3 "=3Dr"))
> +   (clobber (reg:CC CC_REGNUM))]
>    "!TARGET_CSSC"
>    "#"
>    "&& 1"
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr116815-4.c b/gcc/testsuit=
e/gcc.target/aarch64/pr116815-4.c
> new file mode 100644
> index 00000000000..6c640f58542
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/pr116815-4.c
> @@ -0,0 +1,94 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +/* PR middle-end/116815 */
> +
> +/* The max/min-of-add/sub patterns split into a flag-setting ADDS/SUBS
> +   followed by a CSEL, so they have to declare that they clobber the
> +   condition codes.  Without that clobber the compare feeding an enclosi=
ng
> +   CCMP chain is treated as still live across the insn and gets deleted,
> +   so the wrong value is selected.  */
> +
> +#pragma GCC target "+nocssc"
> +
> +__attribute__ ((noipa)) unsigned
> +umax_plus (unsigned a, unsigned b, unsigned c, unsigned d)
> +{
> +  unsigned s =3D a + b;
> +  unsigned m =3D s > a ? s : a;
> +  return (c < d && a < b) ? m : d;
> +}
> +
> +__attribute__ ((noipa)) unsigned
> +umin_plus (unsigned a, unsigned b, unsigned c, unsigned d)
> +{
> +  unsigned s =3D a + b;
> +  unsigned m =3D s < a ? s : a;
> +  return (c < d && a < b) ? m : d;
> +}
> +
> +__attribute__ ((noipa)) unsigned
> +umax_minus (unsigned a, unsigned b, unsigned c, unsigned d)
> +{
> +  unsigned s =3D a - b;
> +  unsigned m =3D s > a ? s : a;
> +  return (c < d && a < b) ? m : d;
> +}
> +
> +__attribute__ ((noipa)) unsigned
> +umin_minus (unsigned a, unsigned b, unsigned c, unsigned d)
> +{
> +  unsigned s =3D a - b;
> +  unsigned m =3D s < a ? s : a;
> +  return (c < d && a < b) ? m : d;
> +}
> +
> +__attribute__ ((noipa)) unsigned long long
> +umax_plus_di (unsigned long long a, unsigned long long b,
> +             unsigned long long c, unsigned long long d)
> +{
> +  unsigned long long s =3D a + b;
> +  unsigned long long m =3D s > a ? s : a;
> +  return (c < d && a < b) ? m : d;
> +}
> +
> +__attribute__ ((noipa)) unsigned long long
> +umin_minus_di (unsigned long long a, unsigned long long b,
> +              unsigned long long c, unsigned long long d)
> +{
> +  unsigned long long s =3D a - b;
> +  unsigned long long m =3D s < a ? s : a;
> +  return (c < d && a < b) ? m : d;
> +}
> +
> +int
> +main (void)
> +{
> +  /* c < d is false, so every call must return d.  */
> +  if (umax_plus (5, 7, 9, 2) !=3D 2)
> +    __builtin_abort ();
> +  if (umin_plus (5, 7, 9, 2) !=3D 2)
> +    __builtin_abort ();
> +  if (umax_minus (5, 7, 9, 2) !=3D 2)
> +    __builtin_abort ();
> +  if (umin_minus (5, 7, 9, 2) !=3D 2)
> +    __builtin_abort ();
> +  if (umax_plus_di (5, 7, 9, 2) !=3D 2)
> +    __builtin_abort ();
> +  if (umin_minus_di (5, 7, 9, 2) !=3D 2)
> +    __builtin_abort ();
> +
> +  /* a < b is false, so these must return d too.  */
> +  if (umax_plus (7, 5, 1, 2) !=3D 2)
> +    __builtin_abort ();
> +  if (umax_minus (7, 5, 1, 2) !=3D 2)
> +    __builtin_abort ();
> +
> +  /* Both true: the max/min result is selected.  */
> +  if (umax_plus (5, 7, 1, 2) !=3D 12)
> +    __builtin_abort ();
> +  if (umin_plus (5, 7, 1, 2) !=3D 5)
> +    __builtin_abort ();
> +
> +  return 0;
> +}
> --
> 2.50.1 (Apple Git-155)
>