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. Thanks, Richard. > Thanks, > Kugan > > gcc/ChangeLog: > > 2016-08-16 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. > (vrp_visit_stmt_worker): Factored out from vrp_visit_stmt. > (vrp_visit_phi_node_worker): Factored out from vrp_visit_phi_stmt. > (vrp_visit_stmt): Use vrp_visit_stmt_worker. > (vrp_visit_phi_node): Use vrp_visit_phi_node_worker.