On Tue, Feb 7, 2017 at 7:31 PM, Jeff Law <l...@redhat.com> wrote:
>
> This patch addresses issues Richi raised from V1. Specifically it moves
> EXACT_DIV_EXPR handling into extract_range_from_binary_expr_1 and less
> aggressively uses ~[0,0] when intersecting ranges -- only doing so when
> vr1's range is > 65536 elements. That number is highly arbitrary. I'm
> certainly open to other values, not sure if a PARAM makes sense or not, but
> can certainly do that if folks think it would be useful. There were other
> minor nits Richi pointed out that were fixed too.
>
> Bootstrapped and regression tested as part of the full patch series.
>
> OK for the trunk?
>
>
> * tree-vrp.c (extract_range_from_binary_expr_1): For EXACT_DIV_EXPR,
> if the numerator has the range ~[0,0] make the resultant range
> ~[0,0].
> (extract_range_from_binary_expr): For MINUS_EXPR with no derived
> range,
> if the operands are known to be not equal, then the resulting range
> is ~[0,0].
> (difference_larger_than): New function.
> (intersect_ranges): If the new range is ~[0,0] and the old range is
> wide, then prefer ~[0,0].
>
> diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
> index b429217..ad8173c 100644
> --- a/gcc/tree-vrp.c
> +++ b/gcc/tree-vrp.c
> @@ -2264,6 +2264,19 @@ extract_range_from_binary_expr_1 (value_range *vr,
> if (vr0.type == VR_ANTI_RANGE
> && ranges_from_anti_range (&vr0, &vrtem0, &vrtem1))
> {
> + /* We get imprecise results from ranges_from_anti_range when
> + code is EXACT_DIV_EXPR. We could mask out bits in the resulting
> + range, but then we also need to hack up vrp_meet. It's just
> + easier to special case when vr0 is ~[0,0] for EXACT_DIV_EXPR. */
> + if (code == EXACT_DIV_EXPR
> + && vr0.type == VR_ANTI_RANGE
> + && vr0.min == vr0.max
> + && integer_zerop (vr0.min))
> + {
> + set_value_range_to_nonnull (vr, expr_type);
> + return;
> + }
> +
Please move this before the surrounding if.
> extract_range_from_binary_expr_1 (vr, code, expr_type, &vrtem0,
> vr1_);
> if (vrtem1.type != VR_UNDEFINED)
> {
> @@ -3298,6 +3311,21 @@ extract_range_from_binary_expr (value_range *vr,
>
> extract_range_from_binary_expr_1 (vr, code, expr_type, &n_vr0, &vr1);
> }
> +
> + /* 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)
Just seeing this again this also covers ~[op1 - 1, op1 - 1] and you are
just lucky because of the vr0.min == vr0.max equality compare and
both op1 - 1 hopefully not tree-shared (I'm not confident in that...).
So I think it would be better written as simply
&& vr0.min == op1
together with vr0.min == vr0.max you'd get what you are after.
> + && vr0.min == vr0.max)
> + || (vr1.type == VR_ANTI_RANGE
> + && symbolic_range_based_on_p (&vr1, op0)
Likewise of course.
> + && vr1.min == vr1.max)))
> + set_value_range_to_nonnull (vr, TREE_TYPE (op0));
> }
>
> /* Extract range information from a unary operation CODE based on
> @@ -8448,6 +8476,17 @@ give_up:
> *vr0max = NULL_TREE;
> }
>
> +/* Return TRUE if MAX - MIN (both trees) is larger than LIMIT
> (HOST_WIDE_INT)
> + or overflows, FALSE otherwise. */
> +
> +static bool
> +difference_larger_than (tree max, tree min, HOST_WIDE_INT limit)
> +{
> + bool overflow;
> + wide_int res = wi::sub (max, min, SIGNED, &overflow);
> + return wi::gt_p (res, limit, UNSIGNED) || overflow;
> +}
> +
> /* Intersect the two value-ranges { *VR0TYPE, *VR0MIN, *VR0MAX } and
> { VR1TYPE, VR0MIN, VR0MAX } and store the result
> in { *VR0TYPE, *VR0MIN, *VR0MAX }. This may not be the smallest
> @@ -8620,6 +8659,15 @@ 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 when vr1's range is relatively wide. */
> + else if (*vr0type == VR_ANTI_RANGE
That's already known to be true here.
> + && *vr0min == *vr0max
> + && integer_zerop (*vr0min)
> + && TREE_CODE (vr1max) == INTEGER_CST
> + && TREE_CODE (vr1min) == INTEGER_CST
> + && difference_larger_than (vr1max, vr1min, 65536))
> + ;
in the case that interests us for the PR what is vr1?
> /* Else choose the range. */
> else
> {
>
