Message ID | 55974BF2.3060603@linaro.org |
---|---|
State | New |
Headers | show |
On Sat, Jul 04, 2015 at 12:58:58PM +1000, Kugan wrote: > Please find a patch that attempt to FIX PR66726 by factoring conversion > out of COND_EXPR as explained in the PR. > > Bootstrapped and regression tested on x86-64-none-linux-gnu with no new > regressions. Is this OK for trunk? > > Thanks, > Kugan > > > gcc/testsuite/ChangeLog: > > 2015-07-03 Kugan Vivekanandarajah <kuganv@linaro.org> > Jeff Law <law@redhat.com> > > PR middle-end/66726 > * gcc.dg/tree-ssa/pr66726.c: New test. I'd have scanned the details dump for "factor CONVERT_EXPR out" 1 to make sure that it's this part that takes care of it. > > gcc/ChangeLog: > > 2015-07-03 Kugan Vivekanandarajah <kuganv@linaro.org> > > PR middle-end/66726 > * tree-ssa-phiopt.c (factor_out_conditional_conversion): New function. > (tree_ssa_phiopt_worker): Call factor_out_conditional_conversion. > diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c > index d2a5cee..e8af086 100644 > --- a/gcc/tree-ssa-phiopt.c > +++ b/gcc/tree-ssa-phiopt.c > @@ -410,6 +413,108 @@ replace_phi_edge_with_variable (basic_block cond_block, > bb->index); > } > > +/* PR66726: Factor conversion out of COND_EXPR. If the argument of the PHI s/the argument/the arguments/ > + 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 def0 = NULL, def1 = NULL, new_stmt; > + tree new_arg0 = NULL_TREE, new_arg1 = NULL_TREE; > + tree temp, result; > + gimple_stmt_iterator gsi; > + > + /* One of the argument has to be SSA_NAME and other argument can s/the argument/the arguments/ > + be an SSA_NAME of INTEGER_CST. */ > + if ((TREE_CODE (arg0) != SSA_NAME > + && TREE_CODE (arg0) != INTEGER_CST) > + || (TREE_CODE (arg1) != SSA_NAME > + && TREE_CODE (arg1) != INTEGER_CST) > + || (TREE_CODE (arg0) == INTEGER_CST > + && TREE_CODE (arg1) == INTEGER_CST)) inconsistent space for the && lines above; The first should have a leading tab. > + return false; > + > + /* Handle only PHI statements with two arguments. TODO: If all > + other arguments to PHI are INTEGER_CST, we can handle more > + than two arguments too. */ > + if (gimple_phi_num_args (phi) != 2) > + return false; > + > + /* If arg0 is an SSA_NAME and the stmt which defines arg0 is > + ai CONVERT_STMT, use the LHS as new_arg0. */ s/ai/a/ > + if (TREE_CODE (arg0) == SSA_NAME) > + { > + def0 = SSA_NAME_DEF_STMT (arg0); > + if (!is_gimple_assign (def0) > + || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def0))) > + return false; > + new_arg0 = gimple_assign_rhs1 (def0); > + } > + > + /* If arg1 is an SSA_NAME and the stmt which defines arg0 is > + ai CONVERT_STMT, use the LHS as new_arg1. */ s/ai/a/ > + if (TREE_CODE (arg1) == SSA_NAME) > + { > + def1 = SSA_NAME_DEF_STMT (arg1); > + if (!is_gimple_assign (def1) > + || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def1))) > + return false; > + new_arg1 = gimple_assign_rhs1 (def1); > + } > + > + /* If arg0 is an INTEGER_CST, fold it to new type. */ > + if (TREE_CODE (arg0) != SSA_NAME) > + { > + if (!POINTER_TYPE_P (TREE_TYPE (new_arg1)) > + && int_fits_type_p (arg0, TREE_TYPE (new_arg1))) > + new_arg0 = fold_convert (TREE_TYPE (new_arg1), arg0); > + else > + return false; > + } > + > + /* If arg1 is an INTEGER_CST, fold it to new type. */ > + if (TREE_CODE (arg1) != SSA_NAME) > + { > + if (!POINTER_TYPE_P (TREE_TYPE (new_arg0)) > + && int_fits_type_p (arg1, TREE_TYPE (new_arg0))) > + new_arg1 = fold_convert (TREE_TYPE (new_arg0), arg1); > + else > + return false; > + } > + > + /* If types of new_arg0 and new_arg1 are different bailout. */ > + if (TREE_TYPE (new_arg0) != TREE_TYPE (new_arg1)) > + return false; > + > + /* Replace the PHI stmt with the new_arg0 and new_arg1. Also insert > + a new CONVER_STMT that converts the phi results. */ s/CONVER_STMT/CONVERT_STMT/ > + gsi = gsi_after_labels (gimple_bb (phi)); > + result = PHI_RESULT (phi); > + temp = make_ssa_name (TREE_TYPE (new_arg0), 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 CONVERT_EXPR out from COND_EXPR.\n"); > + fprintf (dump_file, "New PHI_RESULT is "); > + print_generic_expr (dump_file, temp, 0); > + fprintf (dump_file, " and new stmt with CONVERT_EXPR defines "); > + print_generic_expr (dump_file, result, 0); > + fprintf (dump_file, ".\n"); > + } > + > + gimple_phi_set_result (phi, temp); > + SET_PHI_ARG_DEF (phi, e0->dest_idx, new_arg0); > + SET_PHI_ARG_DEF (phi, e1->dest_idx, new_arg1); > + new_stmt = gimple_build_assign (result, CONVERT_EXPR, temp); > + gsi_insert_before (&gsi, new_stmt, GSI_SAME_STMT); > + 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. > @@ -2144,6 +2249,26 @@ gate_hoist_loads (void) > This pass also performs a fifth transformation of a slightly different > flavor. > > + Factor conversion in COND_EXPR > + ---------------------------------- excess trailing "----" ;) thanks, > + > + This transformation factors the conversion out of COND_EXPR with > + factor_out_conditional_conversion. > + > + For example: > + if (a <= CST) goto <bb 3>; else goto <bb 4>; > + <bb 3>: > + tmp = (int) a; > + <bb 4>: > + tmp = PHI <tmp, CST> > + > + Into: > + if (a <= CST) goto <bb 3>; else goto <bb 4>; > + <bb 3>: > + <bb 4>: > + a = PHI <a, CST> > + tmp = (int) a; > + > Adjacent Load Hoisting > ---------------------- >
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr66726.c b/gcc/testsuite/gcc.dg/tree-ssa/pr66726.c index e69de29..b636c8f 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/pr66726.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr66726.c @@ -0,0 +1,13 @@ + +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-phiopt2" } */ + +extern unsigned short mode_size[]; +int +oof (int mode) +{ + return (64 < mode_size[mode] ? 64 : mode_size[mode]); +} + +/* { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "phiopt2" } } */ + diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c index d2a5cee..e8af086 100644 --- a/gcc/tree-ssa-phiopt.c +++ b/gcc/tree-ssa-phiopt.c @@ -73,6 +73,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, @@ -342,6 +343,8 @@ tree_ssa_phiopt_worker (bool do_store_elim, bool do_hoist_loads) cfgchanged = true; else if (minmax_replacement (bb, bb1, e1, e2, phi, arg0, arg1)) cfgchanged = true; + else if (factor_out_conditional_conversion (e1, e2, phi, arg0, arg1)) + cfgchanged = true; } } @@ -410,6 +413,108 @@ replace_phi_edge_with_variable (basic_block cond_block, bb->index); } +/* PR66726: Factor conversion out of COND_EXPR. If the argument 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 def0 = NULL, def1 = NULL, new_stmt; + tree new_arg0 = NULL_TREE, new_arg1 = NULL_TREE; + tree temp, result; + gimple_stmt_iterator gsi; + + /* One of the argument has to be SSA_NAME and other argument can + be an SSA_NAME of INTEGER_CST. */ + if ((TREE_CODE (arg0) != SSA_NAME + && TREE_CODE (arg0) != INTEGER_CST) + || (TREE_CODE (arg1) != SSA_NAME + && TREE_CODE (arg1) != INTEGER_CST) + || (TREE_CODE (arg0) == INTEGER_CST + && TREE_CODE (arg1) == INTEGER_CST)) + return false; + + /* Handle only PHI statements with two arguments. TODO: If all + other arguments to PHI are INTEGER_CST, we can handle more + than two arguments too. */ + if (gimple_phi_num_args (phi) != 2) + return false; + + /* If arg0 is an SSA_NAME and the stmt which defines arg0 is + ai CONVERT_STMT, use the LHS as new_arg0. */ + if (TREE_CODE (arg0) == SSA_NAME) + { + def0 = SSA_NAME_DEF_STMT (arg0); + if (!is_gimple_assign (def0) + || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def0))) + return false; + new_arg0 = gimple_assign_rhs1 (def0); + } + + /* If arg1 is an SSA_NAME and the stmt which defines arg0 is + ai CONVERT_STMT, use the LHS as new_arg1. */ + if (TREE_CODE (arg1) == SSA_NAME) + { + def1 = SSA_NAME_DEF_STMT (arg1); + if (!is_gimple_assign (def1) + || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def1))) + return false; + new_arg1 = gimple_assign_rhs1 (def1); + } + + /* If arg0 is an INTEGER_CST, fold it to new type. */ + if (TREE_CODE (arg0) != SSA_NAME) + { + if (!POINTER_TYPE_P (TREE_TYPE (new_arg1)) + && int_fits_type_p (arg0, TREE_TYPE (new_arg1))) + new_arg0 = fold_convert (TREE_TYPE (new_arg1), arg0); + else + return false; + } + + /* If arg1 is an INTEGER_CST, fold it to new type. */ + if (TREE_CODE (arg1) != SSA_NAME) + { + if (!POINTER_TYPE_P (TREE_TYPE (new_arg0)) + && int_fits_type_p (arg1, TREE_TYPE (new_arg0))) + new_arg1 = fold_convert (TREE_TYPE (new_arg0), arg1); + else + return false; + } + + /* If types of new_arg0 and new_arg1 are different bailout. */ + if (TREE_TYPE (new_arg0) != TREE_TYPE (new_arg1)) + return false; + + /* Replace the PHI stmt with the new_arg0 and new_arg1. Also insert + a new CONVER_STMT that converts the phi results. */ + gsi = gsi_after_labels (gimple_bb (phi)); + result = PHI_RESULT (phi); + temp = make_ssa_name (TREE_TYPE (new_arg0), 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 CONVERT_EXPR out from COND_EXPR.\n"); + fprintf (dump_file, "New PHI_RESULT is "); + print_generic_expr (dump_file, temp, 0); + fprintf (dump_file, " and new stmt with CONVERT_EXPR defines "); + print_generic_expr (dump_file, result, 0); + fprintf (dump_file, ".\n"); + } + + gimple_phi_set_result (phi, temp); + SET_PHI_ARG_DEF (phi, e0->dest_idx, new_arg0); + SET_PHI_ARG_DEF (phi, e1->dest_idx, new_arg1); + new_stmt = gimple_build_assign (result, CONVERT_EXPR, temp); + gsi_insert_before (&gsi, new_stmt, GSI_SAME_STMT); + 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. @@ -2144,6 +2249,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 <bb 3>; else goto <bb 4>; + <bb 3>: + tmp = (int) a; + <bb 4>: + tmp = PHI <tmp, CST> + + Into: + if (a <= CST) goto <bb 3>; else goto <bb 4>; + <bb 3>: + <bb 4>: + a = PHI <a, CST> + tmp = (int) a; + Adjacent Load Hoisting ----------------------