From d9c03fc27d8147a9401a29739694b214df48a9a2 Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Fri, 1 Apr 2022 11:50:41 +0200 Subject: [PATCH] phiopt: Improve value_replacement [PR104645] The following patch fixes the P1 regression by reusing existing value_replacement code. That function already has code to handle simple preparation statements (casts, and +,&,|,^ binary assignments) before a final binary assignment (which can be much wider range of ops). When we have e.g. if (y_3(D) == 0) goto ; else goto ; : y_4 = y_3(D) & 31; _1 = (int) y_4; _6 = x_5(D) r<< _1; : # _2 = PHI the preparation statements y_4 = y_3(D) & 31; and _1 = (int) y_4; are handled by constant evaluation, passing through y_3(D) = 0 initially and propagating that through the assignments with checking that UB isn't invoked. But the final _6 = x_5(D) r<< _1; assign is handled differently, either through neutral_element_p or absorbing_element_p. In the first function below we now have: [local count: 1073741824]: if (i_2(D) != 0) goto ; [50.00%] else goto ; [50.00%] [local count: 536870913]: _3 = i_2(D) & 1; iftmp.0_4 = (int) _3; [local count: 1073741824]: # iftmp.0_1 = PHI where in GCC 11 we had: : if (i_3(D) != 0) goto ; [INV] else goto ; [INV] : i.1_1 = (int) i_3(D); iftmp.0_5 = i.1_1 & 1; : # iftmp.0_2 = PHI Current value_replacement can handle the latter as the last stmt of middle_bb is a binary op that in this case satisfies absorbing_element_p. But the former we can't handle, as the last stmt in middle_bb is a cast. The patch makes it work in that case by pretending all of middle_bb are the preparation statements and there is no binary assign at the end, so everything is handled through the constant evaluation. We simply set at the start of middle_bb the lhs of comparison virtually to the rhs, propagate it through and at the end see if virtually the arg0 of the PHI is equal to arg1 of it. For GCC 13, I think we just should throw away all the neutral/absorbing element stuff and do the constant evaluation of the whole middle_bb and handle that way all the ops we currently handle in neutral/absorbing element. 2022-04-01 Jakub Jelinek PR tree-optimization/104645 * tree-ssa-phiopt.cc (value_replacement): If assign has CONVERT_EXPR_CODE_P rhs_code, treat it like a preparation statement with constant evaluation. * gcc.dg/tree-ssa/pr104645.c: New test. --- gcc/testsuite/gcc.dg/tree-ssa/pr104645.c | 28 ++++++++++++++ gcc/tree-ssa-phiopt.cc | 63 +++++++++++++++++++++++++------- 2 files changed, 77 insertions(+), 14 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr104645.c diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr104645.c b/gcc/testsuite/gcc.dg/tree-ssa/pr104645.c new file mode 100644 index 0000000..83c1dd4 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr104645.c @@ -0,0 +1,28 @@ +/* PR tree-optimization/104645 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ +/* { dg-final { scan-tree-dump-not " = PHI <" "optimized" } } */ + +int +foo (unsigned i) +{ + return i ? i % 2 : 0; +} + +int +bar (unsigned i) +{ + int b = 0; + if (i) + { + unsigned a = i & 1; + b = a; + } + return b; +} + +int +baz (unsigned i) +{ + return i ? i + 4 : 4; +} diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc index 562468b..4a0c9dd 100644 --- a/gcc/tree-ssa-phiopt.cc +++ b/gcc/tree-ssa-phiopt.cc @@ -1395,11 +1395,22 @@ value_replacement (basic_block cond_bb, basic_block middle_bb, gimple *assign = gsi_stmt (gsi); if (!is_gimple_assign (assign) - || gimple_assign_rhs_class (assign) != GIMPLE_BINARY_RHS || (!INTEGRAL_TYPE_P (TREE_TYPE (arg0)) && !POINTER_TYPE_P (TREE_TYPE (arg0)))) return 0; + if (gimple_assign_rhs_class (assign) != GIMPLE_BINARY_RHS) + { + /* If last stmt of the middle_bb is a conversion, handle it like + a preparation statement through constant evaluation with + checking for UB. */ + enum tree_code sc = gimple_assign_rhs_code (assign); + if (CONVERT_EXPR_CODE_P (sc)) + assign = NULL; + else + return 0; + } + /* Punt if there are (degenerate) PHIs in middle_bb, there should not be. */ if (!gimple_seq_empty_p (phi_nodes (middle_bb))) return 0; @@ -1430,7 +1441,8 @@ value_replacement (basic_block cond_bb, basic_block middle_bb, int prep_cnt; for (prep_cnt = 0; ; prep_cnt++) { - gsi_prev_nondebug (&gsi); + if (prep_cnt || assign) + gsi_prev_nondebug (&gsi); if (gsi_end_p (gsi)) break; @@ -1450,7 +1462,8 @@ value_replacement (basic_block cond_bb, basic_block middle_bb, || !INTEGRAL_TYPE_P (TREE_TYPE (lhs)) || !INTEGRAL_TYPE_P (TREE_TYPE (rhs1)) || !single_imm_use (lhs, &use_p, &use_stmt) - || use_stmt != (prep_cnt ? prep_stmt[prep_cnt - 1] : assign)) + || ((prep_cnt || assign) + && use_stmt != (prep_cnt ? prep_stmt[prep_cnt - 1] : assign))) return 0; switch (gimple_assign_rhs_code (g)) { @@ -1483,10 +1496,6 @@ value_replacement (basic_block cond_bb, basic_block middle_bb, >= 3 * estimate_num_insns (cond, &eni_time_weights)) return 0; - tree lhs = gimple_assign_lhs (assign); - tree rhs1 = gimple_assign_rhs1 (assign); - tree rhs2 = gimple_assign_rhs2 (assign); - enum tree_code code_def = gimple_assign_rhs_code (assign); tree cond_lhs = gimple_cond_lhs (cond); tree cond_rhs = gimple_cond_rhs (cond); @@ -1516,16 +1525,39 @@ value_replacement (basic_block cond_bb, basic_block middle_bb, return 0; } + tree lhs, rhs1, rhs2; + enum tree_code code_def; + if (assign) + { + lhs = gimple_assign_lhs (assign); + rhs1 = gimple_assign_rhs1 (assign); + rhs2 = gimple_assign_rhs2 (assign); + code_def = gimple_assign_rhs_code (assign); + } + else + { + gcc_assert (prep_cnt > 0); + lhs = cond_lhs; + rhs1 = NULL_TREE; + rhs2 = NULL_TREE; + code_def = ERROR_MARK; + } + if (((code == NE_EXPR && e1 == false_edge) || (code == EQ_EXPR && e1 == true_edge)) && arg0 == lhs - && ((arg1 == rhs1 - && operand_equal_for_phi_arg_p (rhs2, cond_lhs) - && neutral_element_p (code_def, cond_rhs, true)) - || (arg1 == rhs2 + && ((assign == NULL + && operand_equal_for_phi_arg_p (arg1, cond_rhs)) + || (assign + && arg1 == rhs1 + && operand_equal_for_phi_arg_p (rhs2, cond_lhs) + && neutral_element_p (code_def, cond_rhs, true)) + || (assign + && arg1 == rhs2 && operand_equal_for_phi_arg_p (rhs1, cond_lhs) && neutral_element_p (code_def, cond_rhs, false)) - || (operand_equal_for_phi_arg_p (arg1, cond_rhs) + || (assign + && operand_equal_for_phi_arg_p (arg1, cond_rhs) && ((operand_equal_for_phi_arg_p (rhs2, cond_lhs) && absorbing_element_p (code_def, cond_rhs, true, rhs2)) || (operand_equal_for_phi_arg_p (rhs1, cond_lhs) @@ -1555,8 +1587,11 @@ value_replacement (basic_block cond_bb, basic_block middle_bb, gsi_from = gsi_for_stmt (prep_stmt[i]); gsi_move_before (&gsi_from, &gsi); } - gsi_from = gsi_for_stmt (assign); - gsi_move_before (&gsi_from, &gsi); + if (assign) + { + gsi_from = gsi_for_stmt (assign); + gsi_move_before (&gsi_from, &gsi); + } replace_phi_edge_with_variable (cond_bb, e1, phi, lhs); return 2; } -- 2.7.4