[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;