On 6/11/19 9:45 AM, Richard Biener wrote:
On Tue, Jun 11, 2019 at 12:40 PM Aldy Hernandez <al...@redhat.com> wrote:
This patch cleans up the various contains, may_contain, and
value_inside_range variants we have throughout, in favor of one--
contains_p. There should be no changes in functionality.
I have added a note to range_includes_zero_p, perhaps as a personal
question than anything else. This function was/is returning true for
UNDEFINED. From a semantic sense, that doesn't make sense. UNDEFINED
is really the empty set. Is the functionality wrong, or should we call
this function something else? Either way, I'm fine removing the comment
but I'm genuinely curious.
So this affects for example this hunk:
- if (vr && !range_includes_p (vr, 1))
+ if (vr && (!vr->contains_p (build_one_cst (TREE_TYPE (name)))
+ && !vr->undefined_p ()))
{
I think it's arbitrary how we compute X in UNDEFINED and I'm fine
with changing the affected predicates to return false. This means
not testing for !vr->undefined_p here.
Excellent.
Note I very much dislike the build_one_cst () call here so please
provide an overload hiding this.
Good idea. I love it.
Not sure why you keep range_includes_zero_p.
I wasn't sure if there was some subtle reason why we were including
undefined.
OK pending tests?
commit 67ce79ed3cc0729650873b1c409d9a4c5156aad9
Author: Aldy Hernandez <al...@redhat.com>
Date: Tue Jun 4 09:11:22 2019 +0200
value_range_base::contains_p and may_contains_p.
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index b9f032e2de3..df3b9abc126 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,27 @@
+2019-06-11 Aldy Hernandez <al...@redhat.com>
+
+ * gimple-loop-versioning.cc (prune_loop_conditions): Use
+ value_range_base::contains_p.
+ * gimple-ssa-evrp-analyze.c (set_ssa_range_info): Use
+ value_range_base::contains_p.
+ * tree-vrp.c (value_range_base::may_contain_p): Remove.
+ (value_inside_range): Make private to value_range_base class.
+ (value_range_base::contains_p): New.
+ (extract_range_from_binary_expr): Use contains_p.
+ (extract_range_from_unary_expr): Same.
+ (union_helper): Same.
+ (vrp_prop::vrp_finalize): Same.
+ * tree-vrp.h (value_range_base): Remove may_contain_p. Add
+ contains_p. Add value_inside_range.
+ (range_includes_p): Remove.
+ (value_inside_range): Remove.
+ (range_includes_zero_p): Remove.
+ (range_includes_p): Use value_range_base::contains_p.
+ * vr-values.c (compare_range_with_value): Same.
+ (vrp_stmt_computes_nonzero): Use value_range_base::contains_p.
+ (extract_range_basic): Same.
+ (compare_range_with_value): Same.
+
2019-06-11 Aldy Hernandez <al...@redhat.com>
* gimple-ssa-evrp.c (evrp_dom_walker::before_dom_children): Use
diff --git a/gcc/gimple-loop-versioning.cc b/gcc/gimple-loop-versioning.cc
index 2f7cda9c8cb..41c2ae33d68 100644
--- a/gcc/gimple-loop-versioning.cc
+++ b/gcc/gimple-loop-versioning.cc
@@ -1488,7 +1488,7 @@ loop_versioning::prune_loop_conditions (struct loop *loop, vr_values *vrs)
{
tree name = ssa_name (i);
value_range *vr = vrs->get_value_range (name);
- if (vr && !range_includes_p (vr, 1))
+ if (vr && !vr->contains_p (1))
{
if (dump_enabled_p ())
dump_printf_loc (MSG_NOTE, find_loop_location (loop),
diff --git a/gcc/gimple-ssa-evrp-analyze.c b/gcc/gimple-ssa-evrp-analyze.c
index bb4e2d6e798..92e2325e9f5 100644
--- a/gcc/gimple-ssa-evrp-analyze.c
+++ b/gcc/gimple-ssa-evrp-analyze.c
@@ -118,8 +118,7 @@ evrp_range_analyzer::set_ssa_range_info (tree lhs, value_range *vr)
wi::to_wide (vr->min ()),
wi::to_wide (vr->max ()));
}
- else if (POINTER_TYPE_P (TREE_TYPE (lhs))
- && range_includes_zero_p (vr) == 0)
+ else if (POINTER_TYPE_P (TREE_TYPE (lhs)) && !vr->contains_p (0))
set_ptr_nonnull (lhs);
}
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index e5000dbcd08..945ecdf250e 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -282,25 +282,6 @@ value_range::set_varying ()
set (VR_VARYING, NULL, NULL, NULL);
}
-/* Return TRUE if it is possible that range contains VAL. */
-
-bool
-value_range_base::may_contain_p (tree val) const
-{
- if (varying_p ())
- return true;
-
- if (undefined_p ())
- return true;
-
- if (m_kind == VR_ANTI_RANGE)
- {
- int res = value_inside_range (val, min (), max ());
- return res == 0 || res == -2;
- }
- return value_inside_range (val, min (), max ()) != 0;
-}
-
void
value_range::equiv_clear ()
{
@@ -1126,7 +1107,7 @@ compare_values (tree val1, tree val2)
function. */
int
-value_inside_range (tree val, tree min, tree max)
+value_range_base::value_inside_range (tree val, tree min, tree max) const
{
int cmp1, cmp2;
@@ -1143,15 +1124,37 @@ value_inside_range (tree val, tree min, tree max)
return !cmp2;
}
+/* Return TRUE if range contains VAL. */
+
+bool
+value_range_base::contains_p (tree val) const
+{
+ if (varying_p ())
+ return true;
+
+ if (undefined_p ())
+ return false;
-/* Return TRUE if *VR includes the value X. */
+ if (m_kind == VR_ANTI_RANGE)
+ {
+ int res = value_inside_range (val, min (), max ());
+ return res == 0 || res == -2;
+ }
+ return value_inside_range (val, min (), max ()) != 0;
+}
+
+/* Return TRUE if range contains VAL. */
bool
-range_includes_p (const value_range_base *vr, HOST_WIDE_INT x)
+value_range_base::contains_p (int val) const
{
- if (vr->varying_p () || vr->undefined_p ())
+ if (varying_p ())
return true;
- return vr->may_contain_p (build_int_cst (vr->type (), x));
+
+ if (undefined_p ())
+ return false;
+
+ return contains_p (build_int_cst (type (), val));
}
/* Value range wrapper for wide_int_range_set_zero_nonzero_bits.
@@ -1618,7 +1621,7 @@ extract_range_from_binary_expr (value_range_base *vr,
nullness, if both are non null, then the result is nonnull.
If both are null, then the result is null. Otherwise they
are varying. */
- if (!range_includes_zero_p (&vr0) && !range_includes_zero_p (&vr1))
+ if (!vr0.contains_p (0) && !vr1.contains_p (0))
vr->set_nonzero (expr_type);
else if (vr0.zero_p () && vr1.zero_p ())
vr->set_zero (expr_type);
@@ -1642,8 +1645,7 @@ extract_range_from_binary_expr (value_range_base *vr,
doesn't either. As the second operand is sizetype (unsigned),
consider all ranges where the MSB could be set as possible
subtractions where the result might be NULL. */
- if ((!range_includes_zero_p (&vr0)
- || !range_includes_zero_p (&vr1))
+ if ((!vr0.contains_p (0) || !vr1.contains_p (0))
&& !TYPE_OVERFLOW_WRAPS (expr_type)
&& (flag_delete_null_pointer_checks
|| (range_int_cst_p (&vr1)
@@ -1658,7 +1660,7 @@ extract_range_from_binary_expr (value_range_base *vr,
{
/* For pointer types, we are really only interested in asserting
whether the expression evaluates to non-NULL. */
- if (!range_includes_zero_p (&vr0) && !range_includes_zero_p (&vr1))
+ if (!vr0.contains_p (0) && !vr1.contains_p (0))
vr->set_nonzero (expr_type);
else if (vr0.zero_p () || vr1.zero_p ())
vr->set_zero (expr_type);
@@ -2096,7 +2098,7 @@ extract_range_from_unary_expr (value_range_base *vr,
storing a magic constant to a pointer. */
if (POINTER_TYPE_P (type) || POINTER_TYPE_P (op0_type))
{
- if (!range_includes_zero_p (&vr0))
+ if (!vr0.contains_p (0))
vr->set_nonzero (type);
else if (vr0.zero_p ())
vr->set_zero (type);
@@ -6114,9 +6116,7 @@ value_range_base::union_helper (const value_range_base *vr0,
/* Failed to find an efficient meet. Before giving up and setting
the result to VARYING, see if we can at least derive a useful
anti-range. */
- if (tem.varying_p ()
- && range_includes_zero_p (vr0) == 0
- && range_includes_zero_p (vr1) == 0)
+ if (tem.varying_p () && !vr0->contains_p (0) && !vr1->contains_p (0))
{
tem.set_nonzero (vr0->type ());
return tem;
@@ -6639,8 +6639,7 @@ vrp_prop::vrp_finalize (bool warn_array_bounds_p)
if (!name || !vr->constant_p ())
continue;
- if (POINTER_TYPE_P (TREE_TYPE (name))
- && range_includes_zero_p (vr) == 0)
+ if (POINTER_TYPE_P (TREE_TYPE (name)) && !vr->contains_p (0))
set_ptr_nonnull (name);
else if (!POINTER_TYPE_P (TREE_TYPE (name)))
set_range_info (name, *vr);
diff --git a/gcc/tree-vrp.h b/gcc/tree-vrp.h
index 1c4af7b68fb..5100e88b95d 100644
--- a/gcc/tree-vrp.h
+++ b/gcc/tree-vrp.h
@@ -69,7 +69,8 @@ public:
/* Misc methods. */
tree type () const;
- bool may_contain_p (tree) const;
+ bool contains_p (tree) const;
+ bool contains_p (int) const;
void set_and_canonicalize (enum value_range_kind, tree, tree);
bool zero_p () const;
bool nonzero_p () const;
@@ -94,6 +95,9 @@ protected:
friend void gt_ggc_mx (value_range_base *&);
friend void gt_pch_nx (value_range_base &);
friend void gt_pch_nx (value_range_base *, gt_pointer_operator, void *);
+
+private:
+ int value_inside_range (tree, tree, tree) const;
};
/* Note value_range cannot currently be used with GC memory, only
@@ -252,7 +256,6 @@ struct assert_info
extern void register_edge_assert_for (tree, edge, enum tree_code,
tree, tree, vec<assert_info> &);
extern bool stmt_interesting_for_vrp (gimple *);
-extern bool range_includes_p (const value_range_base *, HOST_WIDE_INT);
extern bool infer_value_range (gimple *, tree, tree_code *, tree *);
extern bool vrp_bitmap_equal_p (const_bitmap, const_bitmap);
@@ -265,7 +268,6 @@ extern int compare_values_warnv (tree, tree, bool *);
extern int operand_less_p (tree, tree);
extern bool vrp_val_is_min (const_tree);
extern bool vrp_val_is_max (const_tree);
-extern int value_inside_range (tree, tree, tree);
extern tree vrp_val_min (const_tree);
extern tree vrp_val_max (const_tree);
@@ -293,12 +295,4 @@ extern tree get_single_symbol (tree, bool *, tree *);
extern void maybe_set_nonzero_bits (edge, tree);
extern value_range_kind determine_value_range (tree, wide_int *, wide_int *);
-/* Return TRUE if *VR includes the value zero. */
-
-inline bool
-range_includes_zero_p (const value_range_base *vr)
-{
- return range_includes_p (vr, 0);
-}
-
#endif /* GCC_TREE_VRP_H */
diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 97235a56c50..bf7666a052b 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -340,7 +340,7 @@ vr_values::vrp_stmt_computes_nonzero (gimple *stmt)
&& !TYPE_OVERFLOW_WRAPS (TREE_TYPE (expr))))
{
value_range *vr = get_value_range (TREE_OPERAND (base, 0));
- if (!range_includes_zero_p (vr))
+ if (!vr->contains_p (0))
return true;
}
/* If MEM_REF has a "positive" offset, consider it non-NULL
@@ -1115,7 +1115,7 @@ vr_values::extract_range_basic (value_range *vr, gimple *stmt)
{
value_range *vr0 = get_value_range (arg);
/* If arg is non-zero, then ffs or popcount are non-zero. */
- if (range_includes_zero_p (vr0) == 0)
+ if (!vr0->contains_p (0))
mini = 1;
/* If some high bits are known to be zero,
we can decrease the maximum. */
@@ -1624,8 +1624,7 @@ compare_range_with_value (enum tree_code comp, value_range *vr, tree val,
|| comp == LE_EXPR)
return NULL_TREE;
- /* ~[VAL_1, VAL_2] OP VAL is known if VAL_1 <= VAL <= VAL_2. */
- if (value_inside_range (val, vr->min (), vr->max ()) == 1)
+ if (!vr->contains_p (val))
return (comp == NE_EXPR) ? boolean_true_node : boolean_false_node;
return NULL_TREE;
