On 02/08/2017 05:45 AM, Richard Biener wrote:
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.
Yea. I wasn't sure about best location. Before the surrounding IF
works for me.
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.
You're right. Fixed.
+ && vr0.min == vr0.max)
+ || (vr1.type == VR_ANTI_RANGE
+ && symbolic_range_based_on_p (&vr1, op0)
Likewise of course.
Likewise.
+ && 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.
Yes. Removed.
+ && *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?
[-2305843009213693952, 2305843009213693951]
Jeff
