On July 4, 2015 2:32:11 PM GMT+02:00, Kugan <kugan.vivekanandara...@linaro.org>
wrote:
>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.
I meant in addition, just to be sure.
Patch itself looks plausible to me but I cannot approve it.
Thanks,
>
>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
>>> ----------------------
>>>
>>
