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 <kug...@linaro.org>
> Jeff Law <l...@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 <kug...@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
> ----------------------
>
