Re: [PATCH] phiopt: Fix up spaceship_replacement [PR126564]

> Am 04.08.2026 um 09:53 schrieb Jakub Jelinek <[email protected]>:
>=20
> =EF=BB=BFHi!
>=20
> spaceship_replacement is for optimization of code like
>    <bb 2> :  // cond3_bb
>    if (a_3(D) =3D=3D b_5(D))
>      goto <bb 6>; [50.00%]
>    else
>      goto <bb 3>; [50.00%]
>=20
>    <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%]
>=20
>    <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%]
>=20
>    <bb 5> [local count: 134217728]:  // middle_bb
>=20
>    <bb 6> [local count: 1073741824]:  // phi_bb
>    # SR.27_4 =3D PHI <0(2), -1(3), 1(4), -128(5)>
>    _2 =3D SR.27_4 > 0;
> to a single comparison (i.e. say (a <=3D> 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) !=3D -128
>              || absu_hwi (tree_to_shwi (arg1)) !=3D 1
>              || wi::to_widest (arg1) =3D=3D wi::to_widest (arg2))
>            return false;
>        }
>      else if (tree_to_shwi (arg1) !=3D -128
>               || absu_hwi (tree_to_shwi (arg0)) !=3D 1
>               || wi::to_widest (arg0) =3D=3D 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 =3D=3D 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. <=3D 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 </<=3D or
>> />=3D (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 =3D=3D 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.
>=20
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok

Richard=20

> For 15 and older the testcase will need to be tweaked slightly (see the PR=
),
> so that it tests miscompilation in those releases.
>=20
> 2026-08-04  Jakub Jelinek  <[email protected]>
>=20
>    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 =3D=3D 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.
>=20
>    * gcc.dg/torture/pr126564.c: New test.
>=20
> --- gcc/tree-ssa-phiopt.cc.jj    2026-08-03 11:25:36.517969862 +0200
> +++ gcc/tree-ssa-phiopt.cc    2026-08-03 15:58:02.050898704 +0200
> @@ -2429,15 +2429,9 @@ spaceship_replacement (basic_block cond_
>     must be different for non-swapped operands and same for swapped
>     operands.  */
>       if ((lhs2 =3D=3D lhs1)
> -      ^ (HONOR_NANS (TREE_TYPE (lhs1))
> -         ? ((cmp2 =3D=3D LT_EXPR || cmp2 =3D=3D LE_EXPR)
> -        !=3D (cmp1 =3D=3D LT_EXPR || cmp1 =3D=3D LE_EXPR))
> -         : (((cond2_phi_edge->flags
> -          & ((cmp2 =3D=3D LT_EXPR || cmp2 =3D=3D LE_EXPR)
> -             ? EDGE_TRUE_VALUE : EDGE_FALSE_VALUE)) !=3D 0)
> -        !=3D ((e1->flags
> -             & ((cmp1 =3D=3D LT_EXPR || cmp1 =3D=3D LE_EXPR)
> -             ? EDGE_TRUE_VALUE : EDGE_FALSE_VALUE)) !=3D 0))))
> +      ^ ((cmp2 =3D=3D LT_EXPR || cmp2 =3D=3D LE_EXPR)
> +         !=3D (cmp1 =3D=3D LT_EXPR || cmp1 =3D=3D LE_EXPR))
> +      ^ ((cond2_phi_edge->flags & EDGE_FALSE_VALUE) !=3D 0))
>    return false;
>       if (!single_pred_p (cond2_bb) || !cond_only_block_p (cond2_bb))
>    return false;
> --- gcc/testsuite/gcc.dg/torture/pr126564.c.jj    2026-08-03 16:14:35.1674=
89718 +0200
> +++ gcc/testsuite/gcc.dg/torture/pr126564.c    2026-08-03 16:15:02.8771436=
94 +0200
> @@ -0,0 +1,25 @@
> +/* PR tree-optimization/126564 */
> +/* { dg-do run } */
> +
> +[[gnu::noipa]] int
> +foo (int x, int y)
> +{
> +  int c =3D -128;
> +  if (x =3D=3D y)
> +    c =3D 0;
> +  else if (x < y)
> +    c =3D -1;
> +  else if (x <=3D y)
> +    c =3D 1;
> +  return c > 0;
> +}
> +
> +int
> +main ()
> +{
> +  int i, j;
> +  for (i =3D -3; i <=3D 3; i++)
> +    for (j =3D -3; j <=3D 3; j++)
> +      if (foo (i, j) !=3D 0)
> +    __builtin_abort ();
> +}
>=20
>    Jakub
>=20