Hi,

On 16/08/16 20:58, Richard Biener wrote:
> On Tue, Aug 16, 2016 at 9:39 AM, kugan
> <kugan.vivekanandarajah@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?

Thanks,
Kugan


gcc/ChangeLog:

2016-08-17  Kugan Vivekanandarajah  <kuganv@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.