[gcc r17-2925] phiopt: Fix up spaceship_replacement [PR126564]

Jakub Jelinek via Gcc-cvs <[email protected]> Tue, 4 Aug 2026 08:38:04 +0000 (GMT)
Newsgroups gmane.comp.gcc.cvs
Message-ID <[email protected]>
https://gcc.gnu.org/g:5e9087929abff340ff8b132da837372875c584aa

commit r17-2925-g5e9087929abff340ff8b132da837372875c584aa
Author: Jakub Jelinek <[email protected]>
Date:   Tue Aug 4 10:35:19 2026 +0200

    phiopt: Fix up spaceship_replacement [PR126564]
    
    spaceship_replacement is for optimization of code like
        <bb 2> :  // cond3_bb
        if (a_3(D) == b_5(D))
          goto <bb 6>; [50.00%]
        else
          goto <bb 3>; [50.00%]
    
        <bb 3> [local count: 536870913]:  // cond2_bb
        if (a_3(D) < b_5(D))
          goto <bb 6>; [50.00%]
        else
          goto <bb 4>; [50.00%]
    
        <bb 4> [local count: 268435456]:  // cond_bb
        if (a_3(D) > b_5(D))
          goto <bb 6>; [50.00%]
        else
          goto <bb 5>; [50.00%]
    
        <bb 5> [local count: 134217728]:  // middle_bb
    
        <bb 6> [local count: 1073741824]:  // phi_bb
        # SR.27_4 = PHI <0(2), -1(3), 1(4), -128(5)>
        _2 = SR.27_4 > 0;
    to a single comparison (i.e. say (a <=> b) > 0 in C++) (it handles
    also just 2 comparisons instead of 3, but this bug is about the 3
    comparisons).
    In
          if (e1->flags & EDGE_TRUE_VALUE)
            {
              if (tree_to_shwi (arg0) != -128
                  || absu_hwi (tree_to_shwi (arg1)) != 1
                  || wi::to_widest (arg1) == wi::to_widest (arg2))
                return false;
            }
          else if (tree_to_shwi (arg1) != -128
                   || absu_hwi (tree_to_shwi (arg0)) != 1
                   || wi::to_widest (arg0) == wi::to_widest (arg2))
            return false;
    (where e1 is 4->6 edge above, arg0 is -128(5), arg1 is 1(4),
    arg2 is -1(3), cond2_phi_edge is 3->6 edge above) we deal with
    the different cases of whether the TRUE edge goes directly to
    phi_bb or through the empty middle_bb in between.
    Right above the above checks is
          if ((cond2_phi_edge->flags & EDGE_FALSE_VALUE)
              && HONOR_NANS (TREE_TYPE (lhs1)))
            return false;
    so for HONOR_NANS, cond2_phi_edge must be TRUE edge, otherwise
    it can be either.  The problematic check that causes the miscompilation
    of the testcase below wants to verify that the two comparisons
    (cmp1 being code of a_3(D) > b_5(D) and cmp2 a_3(D) < b_5(D)))
    are actually different, not just non-removed useless duplications
    (which is what causes miscompilation of the testcase below).
    The lhs2 == lhs1 xored case is whether the 2 comparisons are
    x cmp1 y vs. x cmp2 y or x cmp1 y vs. y cmp2 x (earlier code verifies
    the operands aren't different in other way with the exception of
    integral comparisons and < 4 vs. <= 3 etc.).
    For the HONOR_NANS case where we know cond2_phi_edge is TRUE
    the other xor operand is whether both cmp2 and cmp1 are </<= or
    >/>= (note, we can treat LT_EXPR and LE_EXPR the same because
    the optimization requires an equality comparison first, so
    LT_EXPR vs. LE_EXPR doesn't matter).  But for !HONOR_NANS I wrote
    a condition checking both the comparison codes and corresponding
    edge flags.  That is wrong because whether e1 is TRUE or FALSE
    edge has been accounted already in the if (e1->flags & EDGE_TRUE_VALUE)
    code above, all we care about is whether cond2_phi_edge is EDGE_TRUE_VALUE
    or EDGE_FALSE_VALUE or the comparison codes of the two comparisons
    (and order of their arguments).
    So, instead this xors lhs2 == lhs1 with whether cmp{1,2} are the same
    with whether cond2_phi_edge is EDGE_FALSE_VALUE.
    For HONOR_NANS there is no difference because the last term will be false.
    The pr94589*.c tests already cover quite a lot of different cases that
    should or shouldn't be matched.
    
    For 15 and older the testcase will need to be tweaked slightly (see the PR),
    so that it tests miscompilation in those releases.
    
    2026-08-04  Jakub Jelinek  <[email protected]>
    
            PR tree-optimization/126564
            * tree-ssa-phiopt.cc (spaceship_replacement): Fix up condition
            when to punt because of redundant cmp1 with cmp2, xor in
            lhs1 == lhs2 with difference of cmp2 from cmp1 (ignoring
            LT_EXPR vs. LE_EXPR and GT_EXPR vs. GE_EXPR differences) and
            1 if cond2_phi_edge is EDGE_FALSE_VALUE.
    
            * gcc.dg/torture/pr126564.c: New test.
    
    Reviewed-by: Richard Biener <[email protected]>

Diff:
---
 gcc/testsuite/gcc.dg/torture/pr126564.c | 25 +++++++++++++++++++++++++
 gcc/tree-ssa-phiopt.cc                  | 12 +++---------
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/torture/pr126564.c b/gcc/testsuite/gcc.dg/torture/pr126564.c
new file mode 100644
index 000000000000..31077dbebe64
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr126564.c
@@ -0,0 +1,25 @@
+/* PR tree-optimization/126564 */
+/* { dg-do run } */
+
+[[gnu::noipa]] int
+foo (int x, int y)
+{
+  int c = -128;
+  if (x == y)
+    c = 0;
+  else if (x < y)
+    c = -1;
+  else if (x <= y)
+    c = 1;
+  return c > 0;
+}
+
+int
+main ()
+{
+  int i, j;
+  for (i = -3; i <= 3; i++)
+    for (j = -3; j <= 3; j++)
+      if (foo (i, j) != 0)
+	__builtin_abort ();
+}
diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
index d63a47cc615a..2a0fe0d643bc 100644
--- a/gcc/tree-ssa-phiopt.cc
+++ b/gcc/tree-ssa-phiopt.cc
@@ -2429,15 +2429,9 @@ spaceship_replacement (basic_block cond_bb, basic_block middle_bb,
 	 must be different for non-swapped operands and same for swapped
 	 operands.  */
       if ((lhs2 == lhs1)
-	  ^ (HONOR_NANS (TREE_TYPE (lhs1))
-	     ? ((cmp2 == LT_EXPR || cmp2 == LE_EXPR)
-		!= (cmp1 == LT_EXPR || cmp1 == LE_EXPR))
-	     : (((cond2_phi_edge->flags
-		  & ((cmp2 == LT_EXPR || cmp2 == LE_EXPR)
-		     ? EDGE_TRUE_VALUE : EDGE_FALSE_VALUE)) != 0)
-		!= ((e1->flags
-		     & ((cmp1 == LT_EXPR || cmp1 == LE_EXPR)
-			 ? EDGE_TRUE_VALUE : EDGE_FALSE_VALUE)) != 0))))
+	  ^ ((cmp2 == LT_EXPR || cmp2 == LE_EXPR)
+	     != (cmp1 == LT_EXPR || cmp1 == LE_EXPR))
+	  ^ ((cond2_phi_edge->flags & EDGE_FALSE_VALUE) != 0))
 	return false;
       if (!single_pred_p (cond2_bb) || !cond_only_block_p (cond2_bb))
 	return false;