From: Kugan Vivekanandarajah Date: Sun, 12 Jul 2015 11:22:42 +0000 (+0000) Subject: re PR tree-optimization/66726 (missed optimization, factor conversion out of COND_EXPR) X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=9844173510418bfd505c1022de7105c4fa4dd906;p=gcc.git re PR tree-optimization/66726 (missed optimization, factor conversion out of COND_EXPR) gcc/testsuite/ChangeLog: 2015-07-12 Kugan Vivekanandarajah Jeff Law PR middle-end/66726 * g++.dg/tree-ssa/pr66726.c: New test. * gcc.dg/tree-ssa/pr66726-2.c: New test. * gcc.dg/tree-ssa/pr66726.c: New test. gcc/ChangeLog: 2015-07-12 Kugan Vivekanandarajah PR middle-end/66726 * tree-ssa-phiopt.c(factor_out_conditional_conversion): New function. tree_ssa_phiopt_worker): Call it. Co-Authored-By: Jeff Law From-SVN: r225722 --- diff --git a/gcc/ChangeLog b/gcc/ChangeLog index c435e30997d..694ee96b15e 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,9 @@ +2015-07-12 Kugan Vivekanandarajah + + PR middle-end/66726 + * tree-ssa-phiopt.c(factor_out_conditional_conversion): New function. + tree_ssa_phiopt_worker): Call it. + 2015-07-12 Kugan Vivekanandarajah * cse.c (cse_insn): Calculate src_eqv for ZERO_EXTRACT. diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index fe6d5e848bd..3d939b1f677 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,11 @@ +2015-07-12 Kugan Vivekanandarajah + Jeff Law + + PR middle-end/66726 + * g++.dg/tree-ssa/pr66726.c: New test. + * gcc.dg/tree-ssa/pr66726-2.c: New test. + * gcc.dg/tree-ssa/pr66726.c: New test. + 2015-07-11 Paolo Carlini PR c++/58734 diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr66726.c b/gcc/testsuite/g++.dg/tree-ssa/pr66726.c new file mode 100644 index 00000000000..9b3bd8fa9ca --- /dev/null +++ b/gcc/testsuite/g++.dg/tree-ssa/pr66726.c @@ -0,0 +1,36 @@ + +/* { dg-do run } */ +/* { dg-options "-O2" } */ + +/* Execution test for converting VIEW_CONVERT_EXPR. */ + +struct cpp_num { + bool f; +}; + +extern cpp_num __attribute__((noinline)) +foo (cpp_num lhs, + cpp_num rhs) +{ + lhs.f = lhs.f || rhs.f; + return lhs; +} + +cpp_num lhs, rhs, r; + +int main () +{ + + lhs.f = false; + rhs.f = false; + r = foo (lhs, rhs); + if (r.f) + __builtin_abort (); + + + lhs.f = false; + rhs.f = true; + r = foo (lhs, rhs); + if (!r.f) + __builtin_abort (); +} diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr66726-2.c b/gcc/testsuite/gcc.dg/tree-ssa/pr66726-2.c new file mode 100644 index 00000000000..ab43d4835d2 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr66726-2.c @@ -0,0 +1,19 @@ + +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-phiopt1-details" } */ + +extern void bar (char, char); +int +foo (char b) +{ + char a; + a = b; + b = 'b'; + bar (a, b); + b = a; + if (b == 0) + a++; + return a + b; +} + +/* { dg-final { scan-tree-dump-times "factor conversion out" 0 "phiopt1" } } */ diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr66726.c b/gcc/testsuite/gcc.dg/tree-ssa/pr66726.c new file mode 100644 index 00000000000..a4c74182059 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr66726.c @@ -0,0 +1,15 @@ + +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-phiopt1-details" } */ + +extern unsigned short mode_size[]; + +int +oof (int mode) +{ + return (64 < mode_size[mode] ? 64 : mode_size[mode]); +} + +/* { dg-final { scan-tree-dump-times "factor conversion out" 1 "phiopt1" } } */ +/* { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "phiopt1" } } */ + diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c index 666e0d946e9..cab20bcd0c8 100644 --- a/gcc/tree-ssa-phiopt.c +++ b/gcc/tree-ssa-phiopt.c @@ -61,6 +61,7 @@ along with GCC; see the file COPYING3. If not see static unsigned int tree_ssa_phiopt_worker (bool, bool); static bool conditional_replacement (basic_block, basic_block, edge, edge, gphi *, tree, tree); +static bool factor_out_conditional_conversion (edge, edge, gphi *, tree, tree); static int value_replacement (basic_block, basic_block, edge, edge, gimple, tree, tree); static bool minmax_replacement (basic_block, basic_block, @@ -323,6 +324,19 @@ tree_ssa_phiopt_worker (bool do_store_elim, bool do_hoist_loads) node. */ gcc_assert (arg0 != NULL && arg1 != NULL); + if (factor_out_conditional_conversion (e1, e2, phi, arg0, arg1)) + { + /* factor_out_conditional_conversion may create a new PHI in + BB2 and eliminate an existing PHI in BB2. Recompute values + that may be affected by that change. */ + phis = phi_nodes (bb2); + phi = single_non_singleton_phi_for_edges (phis, e1, e2); + gcc_assert (phi); + arg0 = gimple_phi_arg_def (phi, e1->dest_idx); + arg1 = gimple_phi_arg_def (phi, e2->dest_idx); + gcc_assert (arg0 != NULL && arg1 != NULL); + } + /* Do the replacement of conditional if it can be done. */ if (conditional_replacement (bb, bb1, e1, e2, phi, arg0, arg1)) cfgchanged = true; @@ -398,6 +412,134 @@ replace_phi_edge_with_variable (basic_block cond_block, bb->index); } +/* PR66726: Factor conversion out of COND_EXPR. If the arguments of the PHI + stmt are CONVERT_STMT, factor out the conversion and perform the conversion + to the result of PHI stmt. */ + +static bool +factor_out_conditional_conversion (edge e0, edge e1, gphi *phi, + tree arg0, tree arg1) +{ + gimple arg0_def_stmt = NULL, arg1_def_stmt = NULL, new_stmt; + tree new_arg0 = NULL_TREE, new_arg1 = NULL_TREE; + tree temp, result; + gphi *newphi; + gimple_stmt_iterator gsi, gsi_for_def; + source_location locus = gimple_location (phi); + enum tree_code convert_code; + + /* Handle only PHI statements with two arguments. TODO: If all + other arguments to PHI are INTEGER_CST or if their defining + statement have the same unary operation, we can handle more + than two arguments too. */ + if (gimple_phi_num_args (phi) != 2) + return false; + + /* First canonicalize to simplify tests. */ + if (TREE_CODE (arg0) != SSA_NAME) + { + std::swap (arg0, arg1); + std::swap (e0, e1); + } + + if (TREE_CODE (arg0) != SSA_NAME + || (TREE_CODE (arg1) != SSA_NAME + && TREE_CODE (arg1) != INTEGER_CST)) + return false; + + /* Check if arg0 is an SSA_NAME and the stmt which defines arg0 is + a conversion. */ + arg0_def_stmt = SSA_NAME_DEF_STMT (arg0); + if (!is_gimple_assign (arg0_def_stmt) + || !gimple_assign_cast_p (arg0_def_stmt)) + return false; + + /* Use the RHS as new_arg0. */ + convert_code = gimple_assign_rhs_code (arg0_def_stmt); + new_arg0 = gimple_assign_rhs1 (arg0_def_stmt); + if (convert_code == VIEW_CONVERT_EXPR) + new_arg0 = TREE_OPERAND (new_arg0, 0); + + if (TREE_CODE (arg1) == SSA_NAME) + { + /* Check if arg1 is an SSA_NAME and the stmt which defines arg1 + is a conversion. */ + arg1_def_stmt = SSA_NAME_DEF_STMT (arg1); + if (!is_gimple_assign (arg1_def_stmt) + || gimple_assign_rhs_code (arg1_def_stmt) != convert_code) + return false; + + /* Use the RHS as new_arg1. */ + new_arg1 = gimple_assign_rhs1 (arg1_def_stmt); + if (convert_code == VIEW_CONVERT_EXPR) + new_arg1 = TREE_OPERAND (new_arg1, 0); + } + else + { + /* If arg1 is an INTEGER_CST, fold it to new type. */ + if (INTEGRAL_TYPE_P (TREE_TYPE (new_arg0)) + && int_fits_type_p (arg1, TREE_TYPE (new_arg0))) + { + if (gimple_assign_cast_p (arg0_def_stmt)) + new_arg1 = fold_convert (TREE_TYPE (new_arg0), arg1); + else + return false; + } + else + return false; + } + + /* If arg0/arg1 have > 1 use, then this transformation actually increases + the number of expressions evaluated at runtime. */ + if (!has_single_use (arg0) + || (arg1_def_stmt && !has_single_use (arg1))) + return false; + + /* If types of new_arg0 and new_arg1 are different bailout. */ + if (!types_compatible_p (TREE_TYPE (new_arg0), TREE_TYPE (new_arg1))) + return false; + + /* Create a new PHI stmt. */ + result = PHI_RESULT (phi); + temp = make_ssa_name (TREE_TYPE (new_arg0), NULL); + newphi = create_phi_node (temp, gimple_bb (phi)); + + if (dump_file && (dump_flags & TDF_DETAILS)) + { + fprintf (dump_file, "PHI "); + print_generic_expr (dump_file, gimple_phi_result (phi), 0); + fprintf (dump_file, + " changed to factor conversion out from COND_EXPR.\n"); + fprintf (dump_file, "New stmt with CAST that defines "); + print_generic_expr (dump_file, result, 0); + fprintf (dump_file, ".\n"); + } + + /* Remove the old cast(s) that has single use. */ + gsi_for_def = gsi_for_stmt (arg0_def_stmt); + gsi_remove (&gsi_for_def, true); + if (arg1_def_stmt) + { + gsi_for_def = gsi_for_stmt (arg1_def_stmt); + gsi_remove (&gsi_for_def, true); + } + + add_phi_arg (newphi, new_arg0, e0, locus); + add_phi_arg (newphi, new_arg1, e1, locus); + + /* Create the conversion stmt and insert it. */ + if (convert_code == VIEW_CONVERT_EXPR) + temp = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (result), temp); + new_stmt = gimple_build_assign (result, convert_code, temp); + gsi = gsi_after_labels (gimple_bb (phi)); + gsi_insert_before (&gsi, new_stmt, GSI_SAME_STMT); + + /* Remove he original PHI stmt. */ + gsi = gsi_for_stmt (phi); + gsi_remove (&gsi, true); + return true; +} + /* The function conditional_replacement does the main work of doing the conditional replacement. Return true if the replacement is done. Otherwise return false. @@ -2130,6 +2272,26 @@ gate_hoist_loads (void) This pass also performs a fifth transformation of a slightly different flavor. + Factor conversion in COND_EXPR + ------------------------------ + + This transformation factors the conversion out of COND_EXPR with + factor_out_conditional_conversion. + + For example: + if (a <= CST) goto ; else goto ; + : + tmp = (int) a; + : + tmp = PHI + + Into: + if (a <= CST) goto ; else goto ; + : + : + a = PHI + tmp = (int) a; + Adjacent Load Hoisting ----------------------