[gcc r17-2923] aarch64: Add missing CC clobber to max/min-of-add/sub patterns [PR116815]

Kyrylo Tkachov via Gcc-cvs <[email protected]> Tue, 4 Aug 2026 08:29:16 +0000 (GMT)
Newsgroups gmane.comp.gcc.cvs
Message-ID <[email protected]>
https://gcc.gnu.org/g:c62176c0ea0d8139161ebb82fabfa0a02e493e2e

commit r17-2923-gc62176c0ea0d8139161ebb82fabfa0a02e493e2e
Author: Kyrylo Tkachov <[email protected]>
Date:   Thu Jul 30 10:36:40 2026 -0700

    aarch64: Add missing CC clobber to max/min-of-add/sub patterns [PR116815]
    
    *aarch64_plus_within_<optab><mode>3_<ovf_commutate> and
    *aarch64_minus_within_<optab><mode>3 split into a flag-setting ADDS or SUBS
    followed by a CSEL, but their insn patterns do not say that they write the
    condition codes.
    
    For
    
      unsigned f (unsigned a, unsigned b, unsigned c, unsigned d)
      {
        unsigned s = a + b;
        unsigned m = 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 kept:
    
      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.
    
    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.
    
    gcc/testsuite/ChangeLog:
    
            PR middle-end/116815
            * gcc.target/aarch64/pr116815-4.c: New test.
    
    Signed-off-by: Kyrylo Tkachov <[email protected]>

Diff:
---
 gcc/config/aarch64/aarch64.md                 |  6 +-
 gcc/testsuite/gcc.target/aarch64/pr116815-4.c | 94 +++++++++++++++++++++++++++
 2 files changed, 98 insertions(+), 2 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index b2185c638195..59af2bd20882 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 "=r"))]
+   (clobber (match_scratch:GPI 3 "=r"))
+   (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 "=r"))]
+   (clobber (match_scratch:GPI 3 "=r"))
+   (clobber (reg:CC CC_REGNUM))]
   "!TARGET_CSSC"
   "#"
   "&& 1"
diff --git a/gcc/testsuite/gcc.target/aarch64/pr116815-4.c b/gcc/testsuite/gcc.target/aarch64/pr116815-4.c
new file mode 100644
index 000000000000..6c640f585425
--- /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 enclosing
+   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 = a + b;
+  unsigned m = 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 = a + b;
+  unsigned m = 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 = a - b;
+  unsigned m = 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 = a - b;
+  unsigned m = 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 = a + b;
+  unsigned long long m = 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 = a - b;
+  unsigned long long m = 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) != 2)
+    __builtin_abort ();
+  if (umin_plus (5, 7, 9, 2) != 2)
+    __builtin_abort ();
+  if (umax_minus (5, 7, 9, 2) != 2)
+    __builtin_abort ();
+  if (umin_minus (5, 7, 9, 2) != 2)
+    __builtin_abort ();
+  if (umax_plus_di (5, 7, 9, 2) != 2)
+    __builtin_abort ();
+  if (umin_minus_di (5, 7, 9, 2) != 2)
+    __builtin_abort ();
+
+  /* a < b is false, so these must return d too.  */
+  if (umax_plus (7, 5, 1, 2) != 2)
+    __builtin_abort ();
+  if (umax_minus (7, 5, 1, 2) != 2)
+    __builtin_abort ();
+
+  /* Both true: the max/min result is selected.  */
+  if (umax_plus (5, 7, 1, 2) != 12)
+    __builtin_abort ();
+  if (umin_plus (5, 7, 1, 2) != 5)
+    __builtin_abort ();
+
+  return 0;
+}