On Sat, Feb 4, 2017 at 3:52 PM, Jeff Law <l...@redhat.com> wrote:
> This is the first of a 4 part series to address the issues around 79095.
>
> This patch addresses improvements in determining ranges of binary
> expressions in three ways.
>
> First if we are otherwise unable to find a range for the result of a
> MINUS_EXPR, if we know the arguments are not equal, then we know the
> resultant range is ~[0,0].
>
> Second, for EXACT_DIV_EXPR, if the numerator has the range ~[0,0], then
> resultant range is currently [TYPE_MIN/DENOM,TYPE_MAX/DENOM].  That is
> rarely a useful range.   A resultant range of ~[0,0] is actually more useful
> since it often tells us something important about the difference of two
> pointers.
>
> Finally, when vrp2 discovers an updated range for an object that had a range
> discovered by vrp1, if the new range is ~[0,0], prefer that new range in
> some cases.  This is needed to avoid losing the newly discovered ~[0,0]
> range for EXACT_DIV_EXPR.
>
> Bootstrapped and regression tested with the other patches in this series.
> OK for the trunk?
>
> Jeff
>
>         * tree-vrp.c (extract_range_from_binary_expr): For EXACT_DIV_EXPR,
>         if the numerator has the range ~[0,0] make the resultant range
>         ~[0,0].  For MINUS_EXPR with no derived range, if the operands are
>         known to be not equal, then the resulting range is ~[0,0].
>         (intersect_ranges): In some cases prefer ~[0,0].
>
> commit b7baf46ab62e28d2dbc22e9dcd4404926d59df18
> Author: Jeff Law <l...@torsion.usersys.redhat.com>
> Date:   Fri Feb 3 15:45:58 2017 -0500
>
>     Improved ranges
>
> diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
> index b429217..3338d8b 100644
> --- a/gcc/tree-vrp.c
> +++ b/gcc/tree-vrp.c
> @@ -3298,6 +3298,37 @@ extract_range_from_binary_expr (value_range *vr,
>
>        extract_range_from_binary_expr_1 (vr, code, expr_type, &n_vr0, &vr1);
>      }
> +
> +  /* EXACT_DIV_EXPR is typically used for pointer subtraction;
> +     as a result a ~[0,0] may be better than what has already
> +     been computed.
> +
> +     In particular if numerator has the range ~[0,0], then the
> +     result range is going to be something like
> +     [MININT/DIVISOR,MAXINT/DIVISOR], which is rarely useful.
> +
> +     So instead make the result range ~[0,0].  */
> +  if (code == EXACT_DIV_EXPR
> +      && TREE_CODE (op0) == SSA_NAME
> +      && vr0.type == VR_ANTI_RANGE
> +      && vr0.min == vr0.max
> +      && integer_zerop (vr0.min))
> +    set_value_range_to_nonnull (vr, TREE_TYPE (op0));

The above belongs in extract_range_from_binary_expr_1, in principle the
cases below as well (though there's pre-existing VARYING result handling).
The _1 ones are supposed to be the actual range computations while
the routine you patched is responsible for interfacing with a lattice.  The
_1 routines can be used from code outside of VRP.

> +  /* If we didn't derive a range for MINUS_EXPR, and
> +     op1's range is ~[op0,op0] or vice-versa, then we
> +     can derive a non-null range.  This happens often for
> +     pointer subtraction.  */
> +  if (vr->type == VR_VARYING
> +      && code == MINUS_EXPR
> +      && TREE_CODE (op0) == SSA_NAME
> +      && ((vr0.type == VR_ANTI_RANGE
> +          && symbolic_range_based_on_p (&vr0, op1)
> +          && vr0.min == vr0.max)
> +         || (vr1.type == VR_ANTI_RANGE
> +             && symbolic_range_based_on_p (&vr1, op0)
> +             && vr1.min == vr1.max)))
> +      set_value_range_to_nonnull (vr, TREE_TYPE (op0));
>  }
>
>  /* Extract range information from a unary operation CODE based on
> @@ -8620,6 +8651,12 @@ intersect_ranges (enum value_range_type *vr0type,
>           else if (vrp_val_is_min (vr1min)
>                    && vrp_val_is_max (vr1max))
>             ;
> +         /* Choose the anti-range if it is ~[0,0], that range is special
> +            enough to special case.  */
> +         else if (*vr0type == VR_ANTI_RANGE
> +                  && *vr0min == *vr0max
> +                  && integer_zerop (*vr0min))
> +           ;

Huh.  If I spotted the place of the change correctly then we cannot arrive
here with vr0 == ~[0,0] as *vr0type is VR_RANGE.  In the case covered
we'd have the only case intersecting [-1, 1] and ~[0,0] that you'd change
to ~[0,0] instead of [-1,1] which generally would be a bad choice (apart
from your implementation error as vr1 is the anti-range here).

Richard.

>           /* Else choose the range.  */
>           else
>             {
>

Reply via email to