[RFA] patch 4/n Refactor bits of vrp_visit_assignment_or_call -- correct patch attached this time

No nyquil tonight, so the proper patch is attached this time...

--



So the next group of changes is focused on breaking down evrp into an
analysis engine and the actual optimization pass.  The analysis engine
can be embedded into other dom walker passes quite easily.  I've done it
for the sprintf warnings as well as the main DOM optimizer locally.

Separating analysis from optimization for edge ranges and PHI ranges is
easy.  Doing so for statements isn't terribly hard either, but does
require a tiny bit of refactoring elsewhere.

Which brings us to this patch.

If we look at evrp_dom_walker::before_dom_children we'll see this in the
statement processing code:

      else if (stmt_interesting_for_vrp (stmt))
        {
          edge taken_edge;
          value_range vr = VR_INITIALIZER;
          extract_range_from_stmt (stmt, &taken_edge, &output, &vr);
          if (output
              && (vr.type == VR_RANGE || vr.type == VR_ANTI_RANGE))
            {
              update_value_range (output, &vr);
              vr = *get_value_range (output);

              /* Mark stmts whose output we fully propagate for removal.  */

etc.

Conceptually this fragment is part of the analysis side.  But the
subsequent code (optimization side) wants to know the "output" of the
statement.  I'm not keen on calling extract_range_from_stmt on both the
analysis side and the optimization side.

So this patch factors out a bit of extract_range_from_stmt and its child
vrp_visit_assignment_or_call into a routine that will return the proper
SSA_NAME.  So the analysis side calls extract_range_from_stmt and the
optimization side calls the new "get_output_for_vrp".

And of course to avoid duplication we use get_output_for_vrp from within
vrp_visit_assignment_or_call.


Bootstrapped and regression tested on x86_64.  OK for the trunk?

Jeff
* vr-values.h (get_output_for_vrp): Prototype.
	* vr-values.c (get_output_for_vrp): New function extracted from
	vrp_visit_assignment_or_call and extract_range_from_stmt.
	(vrp_visit_assignment_or_call): Use get_output_for_vrp.  Simplify.

On Fri, Nov 17, 2017 at 5:17 AM, Jeff Law <law@redhat.com> wrote:
> No nyquil tonight, so the proper patch is attached this time...
>
> --
>
>
>
> So the next group of changes is focused on breaking down evrp into an
> analysis engine and the actual optimization pass.  The analysis engine
> can be embedded into other dom walker passes quite easily.  I've done it
> for the sprintf warnings as well as the main DOM optimizer locally.
>
> Separating analysis from optimization for edge ranges and PHI ranges is
> easy.  Doing so for statements isn't terribly hard either, but does
> require a tiny bit of refactoring elsewhere.
>
> Which brings us to this patch.
>
> If we look at evrp_dom_walker::before_dom_children we'll see this in the
> statement processing code:
>
>       else if (stmt_interesting_for_vrp (stmt))
>         {
>           edge taken_edge;
>           value_range vr = VR_INITIALIZER;
>           extract_range_from_stmt (stmt, &taken_edge, &output, &vr);
>           if (output
>               && (vr.type == VR_RANGE || vr.type == VR_ANTI_RANGE))
>             {
>               update_value_range (output, &vr);
>               vr = *get_value_range (output);
>
>               /* Mark stmts whose output we fully propagate for removal.  */
>
> etc.
>
> Conceptually this fragment is part of the analysis side.  But the
> subsequent code (optimization side) wants to know the "output" of the
> statement.  I'm not keen on calling extract_range_from_stmt on both the
> analysis side and the optimization side.
>
> So this patch factors out a bit of extract_range_from_stmt and its child
> vrp_visit_assignment_or_call into a routine that will return the proper
> SSA_NAME.  So the analysis side calls extract_range_from_stmt and the
> optimization side calls the new "get_output_for_vrp".
>
> And of course to avoid duplication we use get_output_for_vrp from within
> vrp_visit_assignment_or_call.
>
>
> Bootstrapped and regression tested on x86_64.  OK for the trunk?

Ok.

Richard.

> Jeff
>
>
>         * vr-values.h (get_output_for_vrp): Prototype.
>         * vr-values.c (get_output_for_vrp): New function extracted from
>         vrp_visit_assignment_or_call and extract_range_from_stmt.
>         (vrp_visit_assignment_or_call): Use get_output_for_vrp.  Simplify.
>
> diff --git a/gcc/vr-values.c b/gcc/vr-values.c
> index d4434de..3e760a3 100644
> --- a/gcc/vr-values.c
> +++ b/gcc/vr-values.c
> @@ -1955,19 +1955,18 @@ vrp_valueize_1 (tree name)
>      }
>    return name;
>  }
> -/* Visit assignment STMT.  If it produces an interesting range, record
> -   the range in VR and set LHS to OUTPUT_P.  */
>
> -void
> -vr_values::vrp_visit_assignment_or_call (gimple *stmt, tree *output_p,
> -                                        value_range *vr)
> +/* Given STMT, an assignment or call, return its LHS if the type
> +   of the LHS is suitable for VRP analysis, else return NULL_TREE.  */
> +
> +tree
> +get_output_for_vrp (gimple *stmt)
>  {
> -  tree lhs;
> -  enum gimple_code code = gimple_code (stmt);
> -  lhs = gimple_get_lhs (stmt);
> -  *output_p = NULL_TREE;
> +  if (!is_gimple_assign (stmt) && !is_gimple_call (stmt))
> +    return NULL_TREE;
>
>    /* We only keep track of ranges in integral and pointer types.  */
> +  tree lhs = gimple_get_lhs (stmt);
>    if (TREE_CODE (lhs) == SSA_NAME
>        && ((INTEGRAL_TYPE_P (TREE_TYPE (lhs))
>            /* It is valid to have NULL MIN/MAX values on a type.  See
> @@ -1975,8 +1974,25 @@ vr_values::vrp_visit_assignment_or_call (gimple *stmt, tree *output_p,
>            && TYPE_MIN_VALUE (TREE_TYPE (lhs))
>            && TYPE_MAX_VALUE (TREE_TYPE (lhs)))
>           || POINTER_TYPE_P (TREE_TYPE (lhs))))
> +    return lhs;
> +
> +  return NULL_TREE;
> +}
> +
> +/* Visit assignment STMT.  If it produces an interesting range, record
> +   the range in VR and set LHS to OUTPUT_P.  */
> +
> +void
> +vr_values::vrp_visit_assignment_or_call (gimple *stmt, tree *output_p,
> +                                        value_range *vr)
> +{
> +  tree lhs = get_output_for_vrp (stmt);
> +  *output_p = lhs;
> +
> +  /* We only keep track of ranges in integral and pointer types.  */
> +  if (lhs)
>      {
> -      *output_p = lhs;
> +      enum gimple_code code = gimple_code (stmt);
>
>        /* Try folding the statement to a constant first.  */
>        x_vr_values = this;
> diff --git a/gcc/vr-values.h b/gcc/vr-values.h
> index 20bd6c5..24f013a 100644
> --- a/gcc/vr-values.h
> +++ b/gcc/vr-values.h
> @@ -118,4 +118,5 @@ class vr_values
>
>  #define VR_INITIALIZER { VR_UNDEFINED, NULL_TREE, NULL_TREE, NULL }
>
> +extern tree get_output_for_vrp (gimple *);
>  #endif /* GCC_VR_VALUES_H */
>