From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2153) id 7AA163857C44; Fri, 21 May 2021 08:40:38 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7AA163857C44 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Jakub Jelinek To: gcc-cvs@gcc.gnu.org Subject: [gcc r12-964] tree-optimization: Improve spaceship_replacement [PR94589] X-Act-Checkin: gcc X-Git-Author: Jakub Jelinek X-Git-Refname: refs/heads/master X-Git-Oldrev: f53aff92acef9d5291ffed72995adc33e91fa209 X-Git-Newrev: f1c777f40aa0b6941efc7440495a8d7e0cc2a1bb Message-Id: <20210521084038.7AA163857C44@sourceware.org> Date: Fri, 21 May 2021 08:40:38 +0000 (GMT) X-BeenThere: gcc-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 21 May 2021 08:40:38 -0000 https://gcc.gnu.org/g:f1c777f40aa0b6941efc7440495a8d7e0cc2a1bb commit r12-964-gf1c777f40aa0b6941efc7440495a8d7e0cc2a1bb Author: Jakub Jelinek Date: Fri May 21 10:39:50 2021 +0200 tree-optimization: Improve spaceship_replacement [PR94589] On Wed, May 19, 2021 at 01:30:31PM -0400, Jason Merrill via Gcc-patches wrote: > Here, when genericizing lexicographical_compare_three_way, we haven't yet > walked the operands, so (a == a) still sees ADDR_EXPR , but this is after > we've changed the type of a to REFERENCE_TYPE. When we try to fold (a == a) > by constexpr evaluation, the constexpr code doesn't understand trying to > take the address of a reference, and we end up crashing. > > Fixed by avoiding constexpr evaluation in genericize_spaceship, by using > fold_build2 instead of build_new_op on scalar operands. Class operands > should have been expanded during parsing. Unfortunately this slightly changed the IL and spaceship_replacement no longer pattern matches it. Here are 3 improvements that make it match: 1) as mentioned in the comment above spaceship_replacement, for strong_ordering, we are pattern matching something like: x == y ? 0 : x < y ? -1 : 1; and for partial_ordering x == y ? 0 : x < y ? -1 : x > y ? 1 : 2; but given the == comparison done first and the other comparisons only if == was false, we actually don't care if the other comparisons are < vs. <= (or > vs. >=), provided the operands of the comparison are the same; we know == is false when doing those and < vs. <= or > vs. >= have the same behavior for NaNs too 2) when y is an integral constant, we should treat x < 5 equivalently to x <= 4 etc. 3) the code punted if cond2_phi_edge wasn't a EDGE_TRUE_VALUE edge, but as the new IL shows, that isn't really needed; given 1) that > and >= are equivalent in the code, any of swapping the comparison operands, changing L[TE]_EXPR to G[TE]_EXPR or vice versa or swapping the EDGE_TRUE_VALUE / EDGE_FALSE_VALUE bits on the edges reverses one of the two comparisons 2021-05-21 Jakub Jelinek PR tree-optimization/94589 * tree-ssa-phiopt.c (spaceship_replacement): For integral rhs1 and rhs2, treat x <= 4 equivalently to x < 5 etc. In cmp1 and cmp2 (if not the same as cmp3) treat <= the same as < and >= the same as >. Don't require that cond2_phi_edge is true edge, instead take false/true edges into account based on cmp1/cmp2 comparison kinds. Diff: --- gcc/tree-ssa-phiopt.c | 75 +++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 64 insertions(+), 11 deletions(-) diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c index 8e8a08bc679..f133659a781 100644 --- a/gcc/tree-ssa-phiopt.c +++ b/gcc/tree-ssa-phiopt.c @@ -1988,8 +1988,16 @@ spaceship_replacement (basic_block cond_bb, basic_block middle_bb, gcond *cond1 = as_a (last_stmt (cond_bb)); enum tree_code cmp1 = gimple_cond_code (cond1); - if (cmp1 != LT_EXPR && cmp1 != GT_EXPR) - return false; + switch (cmp1) + { + case LT_EXPR: + case LE_EXPR: + case GT_EXPR: + case GE_EXPR: + break; + default: + return false; + } tree lhs1 = gimple_cond_lhs (cond1); tree rhs1 = gimple_cond_rhs (cond1); /* The optimization may be unsafe due to NaNs. */ @@ -2029,7 +2037,42 @@ spaceship_replacement (basic_block cond_bb, basic_block middle_bb, if (lhs2 == lhs1) { if (!operand_equal_p (rhs2, rhs1, 0)) - return false; + { + if ((cmp2 == EQ_EXPR || cmp2 == NE_EXPR) + && TREE_CODE (rhs1) == INTEGER_CST + && TREE_CODE (rhs2) == INTEGER_CST) + { + /* For integers, we can have cond2 x == 5 + and cond1 x < 5, x <= 4, x <= 5, x < 6, + x > 5, x >= 6, x >= 5 or x > 4. */ + if (tree_int_cst_lt (rhs1, rhs2)) + { + if (wi::ne_p (wi::to_wide (rhs1) + 1, wi::to_wide (rhs2))) + return false; + if (cmp1 == LE_EXPR) + cmp1 = LT_EXPR; + else if (cmp1 == GT_EXPR) + cmp1 = GE_EXPR; + else + return false; + } + else + { + gcc_checking_assert (tree_int_cst_lt (rhs2, rhs1)); + if (wi::ne_p (wi::to_wide (rhs2) + 1, wi::to_wide (rhs1))) + return false; + if (cmp1 == LT_EXPR) + cmp1 = LE_EXPR; + else if (cmp1 == GE_EXPR) + cmp1 = GT_EXPR; + else + return false; + } + rhs1 = rhs2; + } + else + return false; + } } else if (lhs2 == rhs1) { @@ -2061,20 +2104,30 @@ spaceship_replacement (basic_block cond_bb, basic_block middle_bb, || absu_hwi (tree_to_shwi (arg0)) != 1 || wi::to_widest (arg0) == wi::to_widest (arg1)) return false; - if (cmp2 != LT_EXPR && cmp2 != GT_EXPR) - return false; + switch (cmp2) + { + case LT_EXPR: + case LE_EXPR: + case GT_EXPR: + case GE_EXPR: + break; + default: + return false; + } /* if (x < y) goto phi_bb; else fallthru; if (x > y) goto phi_bb; else fallthru; bbx:; phi_bb:; is ok, but if x and y are swapped in one of the comparisons, or the comparisons are the same and operands not swapped, - or second goto phi_bb is not the true edge, it is not. */ + or the true and false edges are swapped, it is not. */ if ((lhs2 == lhs1) - ^ (cmp2 == cmp1) - ^ ((e1->flags & EDGE_TRUE_VALUE) != 0)) - return false; - if ((cond2_phi_edge->flags & EDGE_TRUE_VALUE) == 0) + ^ (((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))) return false; if (!single_pred_p (cond2_bb) || !cond_only_block_p (cond2_bb)) return false; @@ -2124,7 +2177,7 @@ spaceship_replacement (basic_block cond_bb, basic_block middle_bb, /* lhs1 one_cmp rhs1 results in phires of 1. */ enum tree_code one_cmp; - if ((cmp1 == LT_EXPR) + if ((cmp1 == LT_EXPR || cmp1 == LE_EXPR) ^ (!integer_onep ((e1->flags & EDGE_TRUE_VALUE) ? arg1 : arg0))) one_cmp = LT_EXPR; else