On Wed, Aug 17, 2016 at 4:27 AM, kugan
<kugan.vivekanandara...@linaro.org> wrote:
> Hi,
>
>
> On 16/08/16 20:58, Richard Biener wrote:
>>
>> On Tue, Aug 16, 2016 at 9:39 AM, kugan
>> <kugan.vivekanandara...@linaro.org> wrote:
>>>
>>> Hi,
>>>
>>>> as said the refactoring that would be appreciated is to split out the
>>>> update_value_range calls
>>>> from the worker functions so you can call the respective functions
>>>> from the DOM implementations.
>>>> That they are globbed in vrp_visit_stmt currently is due to the API of
>>>> the SSA propagator.
>>>
>>>
>>>
>>> Here is a patch that just splits out the update_value_range calls
>>> visit_stmts. Bootstrapped and regression tested on x86_64-linux with no
>>> new
>>> regressions.
>>>
>>> I also verified few random fdump-tree-vrp1-details from stage2 to make
>>> sure
>>> they are same.
>>>
>>> Is this OK for trunk?
>>
>>
>> For vrp_visit_assignment_or_call please defer the question whether the
>> update
>> is interesting (for the internal call stuff) to the caller and always
>> return new_vr.
>>
>> Also do not perform the "not handled stmt" handling here but make the
>> return
>> value reflect whether we handled the stmt or not and put
>>
>> /* Every other statement produces no useful ranges. */
>> FOR_EACH_SSA_TREE_OPERAND (def, stmt, iter, SSA_OP_DEF)
>> set_value_range_to_varying (get_value_range (def));
>>
>> into the caller (as that's also a lattice-updating thing).
>>
>> +static enum ssa_prop_result
>> +vrp_visit_stmt (gimple *stmt, edge *taken_edge_p, tree *output_p)
>> +{
>> + value_range vr = VR_INITIALIZER;
>> + tree lhs = gimple_get_lhs (stmt);
>> + bool vr_found = vrp_visit_stmt_worker (stmt, taken_edge_p,
>> + output_p, &vr);
>> +
>> + if (lhs)
>> + {
>> + if (vr_found
>> + && update_value_range (lhs, &vr))
>> + {
>> + *output_p = lhs;
>>
>> I think rather than computing LHS here you should use *output_p.
>>
>> Otherwise this looks good though I'd rename the _worker variants
>> to extract_range_from_phi_node, extract_range_from_stmt and
>> extract_range_from_assignment_or_call.
>>
>
> Please find the patch attached which addresses the comments above. Bootstrap
> and regression testing is ongoing. Is this of if there is no new
> regressions?
Yes, this looks good to me.
Thanks,
Richard.
> Thanks,
> Kugan
>
>
> gcc/ChangeLog:
>
> 2016-08-17 Kugan Vivekanandarajah <kug...@linaro.org>
>
> * tree-vrp.c (vrp_visit_assignment_or_call): Changed to Return VR.
> (vrp_visit_cond_stmt): Just sets TAKEN_EDGE_P.
> (vrp_visit_switch_stmt): Likewise.
> (extract_range_from_stmt): Factored out from vrp_visit_stmt.
> (extract_range_from_phi_node): Factored out from vrp_visit_phi_stmt.
> (vrp_visit_stmt): Use extract_range_from_stmt.
> (vrp_visit_phi_node): Use extract_range_from_phi_node.
>
