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.

Reply via email to