On 08/29/2018 12:32 PM, David Malcolm wrote:
On Wed, 2018-08-29 at 06:54 -0400, Aldy Hernandez wrote:
Never say never. Just when I thought I was done...
It looks like I need the special casing we do for pointer types in
extract_range_from_binary_expr_1.
The code is simple enough that we could just duplicate it anywhere
we
need it, but I hate code duplication and keeping track of multiple
versions of the same thing.
No change in functionality.
Tested on x86-64 Linux with all languages.
OK?
A couple of nits I spotted:
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index f20730a85ba..228f20b5203 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -1275,6 +1275,32 @@ set_value_range_with_overflow (value_range &vr,
}
}
+/* Value range wrapper for wide_int_range_pointer. */
+
+static void
+vrp_range_pointer (value_range *vr,
+ enum tree_code code, tree type,
+ value_range *vr0, value_range *vr1)
Looking at the signature of the function, I wondered what the source
and destination of the information is...
vr being the destination and vr0/vr1 being the sources are standard
operating procedure within tree-vrp.c. All the functions are basically
that, that's why I haven't bothered.
Could vr0 and vr1 be const?
...which would require extract_range_into_wide_ints to take a const
value_range *
Yes, but that would require changing all of tree-vrp.c to take const
value_range's. For instance, range_int_cst_p and a slew of other
functions throughout.
... which would require range_int_cst_p to take a const value_range *
(I *think* that's where the yak-shaving would end)
+{
+ signop sign = TYPE_SIGN (type);
+ unsigned prec = TYPE_PRECISION (type);
+ wide_int vr0_min, vr0_max;
+ wide_int vr1_min, vr1_max;
+
+ extract_range_into_wide_ints (vr0, sign, prec, vr0_min, vr0_max);
+ extract_range_into_wide_ints (vr1, sign, prec, vr1_min, vr1_max);
+ wide_int_range_nullness n;
+ n = wide_int_range_pointer (code, sign, vr0_min, vr0_max, vr1_min,
vr1_max);
+ if (n == WIDE_INT_RANGE_UNKNOWN)
+ set_value_range_to_varying (vr);
+ else if (n == WIDE_INT_RANGE_NULL)
+ set_value_range_to_null (vr, type);
+ else if (n == WIDE_INT_RANGE_NONNULL)
+ set_value_range_to_nonnull (vr, type);
+ else
+ gcc_unreachable ();
+}
+
Would it be better to use a "switch (n)" here, rather than a series of
"if"/"else if" for each enum value?
I prefer ifs for a small amount of options, but having the compiler warn
on missing enum alternatives is nice, so I've changed it. Notice
there's more code now though :-(.
Aldy
gcc/
* tree-vrp.c (vrp_range_pointer): New.
(extract_range_from_binary_expr_1): Call vrp_range_pointer.
* wide-int-range.cc (wide_int_range_pointer): New.
* wide-int-range.h (enum wide_int_range_nullness): New.
(wide_int_range_pointer): New.
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index f20730a85ba..b0dad0701b7 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -1275,6 +1275,38 @@ set_value_range_with_overflow (value_range &vr,
}
}
+/* Value range wrapper for wide_int_range_pointer. */
+
+static void
+vrp_range_pointer (value_range *vr,
+ enum tree_code code, tree type,
+ value_range *vr0, value_range *vr1)
+{
+ signop sign = TYPE_SIGN (type);
+ unsigned prec = TYPE_PRECISION (type);
+ wide_int vr0_min, vr0_max;
+ wide_int vr1_min, vr1_max;
+
+ extract_range_into_wide_ints (vr0, sign, prec, vr0_min, vr0_max);
+ extract_range_into_wide_ints (vr1, sign, prec, vr1_min, vr1_max);
+ wide_int_range_nullness n;
+ n = wide_int_range_pointer (code, sign, vr0_min, vr0_max, vr1_min, vr1_max);
+ switch (n)
+ {
+ case WIDE_INT_RANGE_UNKNOWN:
+ set_value_range_to_varying (vr);
+ break;
+ case WIDE_INT_RANGE_NULL:
+ set_value_range_to_null (vr, type);
+ break;
+ case WIDE_INT_RANGE_NONNULL:
+ set_value_range_to_nonnull (vr, type);
+ break;
+ default:
+ gcc_unreachable ();
+ }
+}
+
/* Extract range information from a binary operation CODE based on
the ranges of each of its operands *VR0 and *VR1 with resulting
type EXPR_TYPE. The resulting range is stored in *VR. */
@@ -1416,47 +1448,10 @@ extract_range_from_binary_expr_1 (value_range *vr,
}
/* Now evaluate the expression to determine the new range. */
+
if (POINTER_TYPE_P (expr_type))
{
- if (code == MIN_EXPR || code == MAX_EXPR)
- {
- /* For MIN/MAX expressions with pointers, we only care about
- 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))
- set_value_range_to_nonnull (vr, expr_type);
- else if (range_is_null (&vr0) && range_is_null (&vr1))
- set_value_range_to_null (vr, expr_type);
- else
- set_value_range_to_varying (vr);
- }
- else if (code == POINTER_PLUS_EXPR)
- {
- /* 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))
- set_value_range_to_nonnull (vr, expr_type);
- else if (range_is_null (&vr0) && range_is_null (&vr1))
- set_value_range_to_null (vr, expr_type);
- else
- set_value_range_to_varying (vr);
- }
- else if (code == BIT_AND_EXPR)
- {
- /* 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))
- set_value_range_to_nonnull (vr, expr_type);
- else if (range_is_null (&vr0) || range_is_null (&vr1))
- set_value_range_to_null (vr, expr_type);
- else
- set_value_range_to_varying (vr);
- }
- else
- set_value_range_to_varying (vr);
-
+ vrp_range_pointer (vr, code, expr_type, &vr0, &vr1);
return;
}
diff --git a/gcc/wide-int-range.cc b/gcc/wide-int-range.cc
index 3cdcede04cd..9ab1b04c4dd 100644
--- a/gcc/wide-int-range.cc
+++ b/gcc/wide-int-range.cc
@@ -733,3 +733,60 @@ wide_int_range_div (wide_int &wmin, wide_int &wmax,
extra_range_p = false;
return true;
}
+
+/* Given a binary operator (CODE) on two pointer ranges, return if the
+ result will be zero, non-zero, or unknown. */
+
+enum wide_int_range_nullness
+wide_int_range_pointer (enum tree_code code,
+ signop sign,
+ const wide_int &vr0_min,
+ const wide_int &vr0_max,
+ const wide_int &vr1_min,
+ const wide_int &vr1_max)
+{
+ unsigned prec = vr0_min.get_precision ();
+ if (code == MIN_EXPR || code == MAX_EXPR)
+ {
+ /* For MIN/MAX expressions with pointers, we only care about
+ 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 (!wide_int_range_includes_zero_p (vr0_min, vr0_max, sign)
+ && !wide_int_range_includes_zero_p (vr1_min, vr1_max, sign))
+ return WIDE_INT_RANGE_NONNULL;
+ else if (wide_int_range_zero_p (vr0_min, vr0_max, prec)
+ && wide_int_range_zero_p (vr1_min, vr1_max, prec))
+ return WIDE_INT_RANGE_NULL;
+ else
+ return WIDE_INT_RANGE_UNKNOWN;
+ }
+ else if (code == POINTER_PLUS_EXPR)
+ {
+ /* For pointer types, we are really only interested in asserting
+ whether the expression evaluates to non-NULL. */
+ if (!wide_int_range_includes_zero_p (vr0_min, vr0_max, sign)
+ || !wide_int_range_includes_zero_p (vr1_min, vr1_max, sign))
+ return WIDE_INT_RANGE_NONNULL;
+ else if (wide_int_range_zero_p (vr0_min, vr0_max, prec)
+ && wide_int_range_zero_p (vr1_min, vr1_max, prec))
+ return WIDE_INT_RANGE_NULL;
+ else
+ return WIDE_INT_RANGE_UNKNOWN;
+ }
+ else if (code == BIT_AND_EXPR)
+ {
+ /* For pointer types, we are really only interested in asserting
+ whether the expression evaluates to non-NULL. */
+ if (!wide_int_range_includes_zero_p (vr0_min, vr0_max, sign)
+ && !wide_int_range_includes_zero_p (vr1_min, vr1_max, sign))
+ return WIDE_INT_RANGE_NONNULL;
+ else if (wide_int_range_zero_p (vr0_min, vr0_max, prec)
+ || wide_int_range_zero_p (vr1_min, vr1_max, prec))
+ return WIDE_INT_RANGE_NULL;
+ else
+ return WIDE_INT_RANGE_UNKNOWN;
+ }
+ else
+ return WIDE_INT_RANGE_UNKNOWN;
+}
diff --git a/gcc/wide-int-range.h b/gcc/wide-int-range.h
index 427ef34c6b4..ecdd961df36 100644
--- a/gcc/wide-int-range.h
+++ b/gcc/wide-int-range.h
@@ -20,6 +20,14 @@ along with GCC; see the file COPYING3. If not see
#ifndef GCC_WIDE_INT_RANGE_H
#define GCC_WIDE_INT_RANGE_H
+/* For an operation on pointers, this specifies whether the resulting
+ operation is null, non-null, or unknown. */
+enum wide_int_range_nullness {
+ WIDE_INT_RANGE_UNKNOWN = 0,
+ WIDE_INT_RANGE_NULL,
+ WIDE_INT_RANGE_NONNULL
+};
+
extern bool wide_int_range_cross_product (wide_int &res_lb, wide_int &res_ub,
enum tree_code code, signop sign,
const wide_int &, const wide_int &,
@@ -110,6 +118,12 @@ extern bool wide_int_range_div (wide_int &wmin, wide_int &wmax,
bool overflow_wraps,
bool &extra_range_p,
wide_int &extra_min, wide_int &extra_max);
+enum wide_int_range_nullness wide_int_range_pointer (enum tree_code code,
+ signop sign,
+ const wide_int &vr0_min,
+ const wide_int &vr0_max,
+ const wide_int &vr1_min,
+ const wide_int &vr1_max);
/* Return TRUE if shifting by range [MIN, MAX] is undefined behavior. */
