On 04/07/15 18:51, Bernhard Reutner-Fischer wrote:
> 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.
Thanks for the comments. Please see the updated patch with the fixes.
Kugan
>
>>
>> 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
>> ----------------------
>>
>
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr66726.c
b/gcc/testsuite/gcc.dg/tree-ssa/pr66726.c
index e69de29..93f1ace 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-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 CONVERT_EXPR out" 1 "phiopt1" } }
*/
+
diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
index d2a5cee..12ab9ee 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 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 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 arguments 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
+ a 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
+ a 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 CONVERT_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
----------------------
