On 10/4/19 2:09 PM, Jeff Law wrote:
You're right. Easier to refer to the before/after directly. Sometimes
diffs just suck.
OK
jeff
In testing this patch in isolation from the non-zero canonicalization
patch, I found one regression due to the fact that:
a) As discussed, two non-zero representations currently exist for
unsigned ranges.
b) ipa-prop.c has it's own hacked up value_range structure (ipa_vr)
which doesn't use any API. Since there is no agreed upon non-zero,
range-ops can sometimes (correctly) create an unsigned [1,MAX], and
ipa-prop.c is open-coding the check for a pointer non-zero to ~[0,0].
This seems like a latent bug.
I really have no idea, nor do I care (*), what we do with ipa-prop's
lack of API. For now, I have implemented ipa_vr::nonzero_p(), and used
it. When we agree on the non-zero normalization we can adjust this
method if necessary.
+bool
+ipa_vr::nonzero_p (tree expr_type) const
+{
+ if (type == VR_ANTI_RANGE && wi::eq_p (min, 0) && wi::eq_p (max, 0))
+ return true;
+
+ unsigned prec = TYPE_PRECISION (expr_type);
+ return (type == VR_RANGE
+ && wi::eq_p (min, wi::one (prec))
+ && wi::eq_p (max, wi::max_value (prec, TYPE_SIGN (expr_type))));
+}
...
else if (POINTER_TYPE_P (TREE_TYPE (ddef))
- && vr[i].type == VR_ANTI_RANGE
- && wi::eq_p (vr[i].min, 0)
- && wi::eq_p (vr[i].max, 0))
+ && vr[i].nonzero_p (TREE_TYPE (ddef)))
Attached is the final adjusted patch I have committed to trunk.
Aldy
(*) This can be loosely translated as "it'll eventually bite us in the
ass, at which point I'll end up addressing it." ;-)
commit 3a7c5a78fe9226daed2d5363a97f87081acd82c7
Author: Aldy Hernandez <al...@redhat.com>
Date: Thu Oct 3 14:53:12 2019 +0200
Disentangle range_fold_*ary_expr() into various independent pieces.
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index fbb1634cbae..83ad0017b5a 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,23 @@
+2019-10-07 Aldy Hernandez <al...@redhat.com>
+
+ * ipa-prop.c (ipa_vr::nonzero_p): New.
+ (ipcp_update_vr): Use nonzero_p instead of open-coding check for
+ non-zero range.
+ * ipa-prop.h (class ipa_vr): Add nonzero_p.
+ * tree-vrp.c (range_has_numeric_bounds_p): New.
+ (range_int_cst_p): Use range_has_numeric_bounds_p.
+ (get_range_op_handler): New.
+ (supported_types_p): New.
+ (defined_ranges_p): New.
+ (drop_undefines_to_varying): New.
+ (range_fold_binary_symbolics_p): New.
+ (range_fold_unary_symbolics_p): New.
+ (range_fold_unary_expr): Extract out into above functions.
+ (range_fold_binary_expr): Same.
+ (value_range_base::normalize_addresses): New.
+ (value_range_base::normalize_symbolics): Normalize addresses.
+ * tree-vrp.h (class value_range_base): Add normalize_addresses.
+
2019-10-07 Aldy Hernandez <al...@redhat.com>
* tree-vrp.c (value_range_base::singleton_p): Use
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index e79add12b1e..20a0bddcbab 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -5109,6 +5109,18 @@ ipcp_update_bits (struct cgraph_node *node)
}
}
+bool
+ipa_vr::nonzero_p (tree expr_type) const
+{
+ if (type == VR_ANTI_RANGE && wi::eq_p (min, 0) && wi::eq_p (max, 0))
+ return true;
+
+ unsigned prec = TYPE_PRECISION (expr_type);
+ return (type == VR_RANGE
+ && wi::eq_p (min, wi::one (prec))
+ && wi::eq_p (max, wi::max_value (prec, TYPE_SIGN (expr_type))));
+}
+
/* Update value range of formal parameters as described in
ipcp_transformation. */
@@ -5181,9 +5193,7 @@ ipcp_update_vr (struct cgraph_node *node)
TYPE_SIGN (type)));
}
else if (POINTER_TYPE_P (TREE_TYPE (ddef))
- && vr[i].type == VR_ANTI_RANGE
- && wi::eq_p (vr[i].min, 0)
- && wi::eq_p (vr[i].max, 0))
+ && vr[i].nonzero_p (TREE_TYPE (ddef)))
{
if (dump_file)
fprintf (dump_file, "Setting nonnull for %u\n", i);
diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index 0ff80854284..eb3397a6d81 100644
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -165,6 +165,7 @@ public:
enum value_range_kind type;
wide_int min;
wide_int max;
+ bool nonzero_p (tree) const;
};
/* A jump function for a callsite represents the values passed as actual
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 86e4dace073..d69cfb107cb 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -910,15 +910,21 @@ vrp_bitmap_equal_p (const_bitmap b1, const_bitmap b2)
&& bitmap_equal_p (b1, b2)));
}
+static bool
+range_has_numeric_bounds_p (const value_range_base *vr)
+{
+ return (vr->min ()
+ && TREE_CODE (vr->min ()) == INTEGER_CST
+ && TREE_CODE (vr->max ()) == INTEGER_CST);
+}
+
/* Return true if max and min of VR are INTEGER_CST. It's not necessary
a singleton. */
bool
range_int_cst_p (const value_range_base *vr)
{
- return (vr->kind () == VR_RANGE
- && TREE_CODE (vr->min ()) == INTEGER_CST
- && TREE_CODE (vr->max ()) == INTEGER_CST);
+ return (vr->kind () == VR_RANGE && range_has_numeric_bounds_p (vr));
}
/* Return true if VR is a INTEGER_CST singleton. */
@@ -1760,119 +1766,102 @@ extract_range_from_plus_minus_expr (value_range_base *vr,
vr->set (kind, min, max);
}
-/* Normalize a value_range for use in range_ops and return it. */
+/* Return the range-ops handler for CODE and EXPR_TYPE. If no
+ suitable operator is found, return NULL and set VR to VARYING. */
-static value_range_base
-normalize_for_range_ops (const value_range_base &vr)
+static const range_operator *
+get_range_op_handler (value_range_base *vr,
+ enum tree_code code,
+ tree expr_type)
{
- tree type = vr.type ();
+ const range_operator *op = range_op_handler (code, expr_type);
+ if (!op)
+ vr->set_varying (expr_type);
+ return op;
+}
+
+/* If the types passed are supported, return TRUE, otherwise set VR to
+ VARYING and return FALSE. */
- /* This will return ~[0,0] for [&var, &var]. */
- if (POINTER_TYPE_P (type) && !range_includes_zero_p (&vr))
+static bool
+supported_types_p (value_range_base *vr,
+ tree type0,
+ tree type1 = NULL)
+{
+ if (!value_range_base::supports_type_p (type0)
+ || (type1 && !value_range_base::supports_type_p (type1)))
{
- value_range_base temp;
- temp.set_nonzero (type);
- return temp;
+ vr->set_varying (type0);
+ return false;
}
- if (vr.symbolic_p ())
- return normalize_for_range_ops (vr.normalize_symbolics ());
- if (TREE_CODE (vr.min ()) == INTEGER_CST
- && TREE_CODE (vr.max ()) == INTEGER_CST)
- return vr;
- /* Anything not strictly numeric at this point becomes varying. */
- return value_range_base (vr.type ());
+ return true;
}
-/* Fold a binary expression of two value_range's with range-ops. */
+/* If any of the ranges passed are defined, return TRUE, otherwise set
+ VR to UNDEFINED and return FALSE. */
-void
-range_fold_binary_expr (value_range_base *vr,
- enum tree_code code,
- tree expr_type,
- const value_range_base *vr0_,
- const value_range_base *vr1_)
+static bool
+defined_ranges_p (value_range_base *vr,
+ const value_range_base *vr0,
+ const value_range_base *vr1 = NULL)
{
- if (!value_range_base::supports_type_p (expr_type)
- || (!vr0_->undefined_p ()
- && !value_range_base::supports_type_p (vr0_->type ()))
- || (!vr1_->undefined_p ()
- && !value_range_base::supports_type_p (vr1_->type ())))
- {
- vr->set_varying (expr_type);
- return;
- }
- if (vr0_->undefined_p () && vr1_->undefined_p ())
+ if (vr0->undefined_p () && (!vr1 || vr1->undefined_p ()))
{
vr->set_undefined ();
- return;
- }
- range_operator *op = range_op_handler (code, expr_type);
- if (!op)
- {
- vr->set_varying (expr_type);
- return;
+ return false;
}
+ return true;
+}
- /* Mimic any behavior users of extract_range_from_binary_expr may
- expect. */
- value_range_base vr0 = *vr0_, vr1 = *vr1_;
- if (vr0.undefined_p ())
- vr0.set_varying (expr_type);
- else if (vr1.undefined_p ())
- vr1.set_varying (expr_type);
+static value_range_base
+drop_undefines_to_varying (const value_range_base *vr, tree expr_type)
+{
+ if (vr->undefined_p ())
+ return value_range_base (expr_type);
+ else
+ return *vr;
+}
+
+/* If any operand is symbolic, perform a binary operation on them and
+ return TRUE, otherwise return FALSE. */
- /* Handle symbolics. */
- if (vr0.symbolic_p () || vr1.symbolic_p ())
+static bool
+range_fold_binary_symbolics_p (value_range_base *vr,
+ tree_code code,
+ tree expr_type,
+ const value_range_base *vr0,
+ const value_range_base *vr1)
+{
+ if (vr0->symbolic_p () || vr1->symbolic_p ())
{
if ((code == PLUS_EXPR || code == MINUS_EXPR))
{
- extract_range_from_plus_minus_expr (vr, code, expr_type,
- &vr0, &vr1);
- return;
+ extract_range_from_plus_minus_expr (vr, code, expr_type, vr0, vr1);
+ return true;
}
if (POINTER_TYPE_P (expr_type) && code == POINTER_PLUS_EXPR)
{
- extract_range_from_pointer_plus_expr (vr, code, expr_type,
- &vr0, &vr1);
- return;
+ extract_range_from_pointer_plus_expr (vr, code, expr_type, vr0, vr1);
+ return true;
}
+ const range_operator *op = get_range_op_handler (vr, code, expr_type);
+ *vr = op->fold_range (expr_type,
+ vr0->normalize_symbolics (),
+ vr1->normalize_symbolics ());
+ return true;
}
-
- /* Do the range-ops dance. */
- value_range_base n0 = normalize_for_range_ops (vr0);
- value_range_base n1 = normalize_for_range_ops (vr1);
- *vr = op->fold_range (expr_type, n0, n1);
+ return false;
}
-/* Fold a unary expression of a value_range with range-ops. */
+/* If operand is symbolic, perform a unary operation on it and return
+ TRUE, otherwise return FALSE. */
-void
-range_fold_unary_expr (value_range_base *vr,
- enum tree_code code, tree expr_type,
- const value_range_base *vr0,
- tree vr0_type)
+static bool
+range_fold_unary_symbolics_p (value_range_base *vr,
+ tree_code code,
+ tree expr_type,
+ const value_range_base *vr0)
{
- /* Mimic any behavior users of extract_range_from_unary_expr may
- expect. */
- if (!value_range_base::supports_type_p (expr_type)
- || !value_range_base::supports_type_p (vr0_type))
- {
- vr->set_varying (expr_type);
- return;
- }
- if (vr0->undefined_p ())
- {
- vr->set_undefined ();
- return;
- }
- range_operator *op = range_op_handler (code, expr_type);
- if (!op)
- {
- vr->set_varying (expr_type);
- return;
- }
-
- /* Handle symbolics. */
if (vr0->symbolic_p ())
{
if (code == NEGATE_EXPR)
@@ -1881,7 +1870,7 @@ range_fold_unary_expr (value_range_base *vr,
value_range_base zero;
zero.set_zero (vr0->type ());
range_fold_binary_expr (vr, MINUS_EXPR, expr_type, &zero, vr0);
- return;
+ return true;
}
if (code == BIT_NOT_EXPR)
{
@@ -1889,30 +1878,64 @@ range_fold_unary_expr (value_range_base *vr,
value_range_base minusone;
minusone.set (build_int_cst (vr0->type (), -1));
range_fold_binary_expr (vr, MINUS_EXPR, expr_type, &minusone, vr0);
- return;
+ return true;
}
+ const range_operator *op = get_range_op_handler (vr, code, expr_type);
*vr = op->fold_range (expr_type,
- normalize_for_range_ops (*vr0),
+ vr0->normalize_symbolics (),
value_range_base (expr_type));
- return;
- }
- if (CONVERT_EXPR_CODE_P (code) && (POINTER_TYPE_P (expr_type)
- || POINTER_TYPE_P (vr0->type ())))
- {
- /* This handles symbolic conversions such such as [25, x_4]. */
- if (!range_includes_zero_p (vr0))
- vr->set_nonzero (expr_type);
- else if (vr0->zero_p ())
- vr->set_zero (expr_type);
- else
- vr->set_varying (expr_type);
- return;
+ return true;
}
+ return false;
+}
- /* Do the range-ops dance. */
- value_range_base n0 = normalize_for_range_ops (*vr0);
- value_range_base n1 (expr_type);
- *vr = op->fold_range (expr_type, n0, n1);
+/* Perform a binary operation on a pair of ranges. */
+
+void
+range_fold_binary_expr (value_range_base *vr,
+ enum tree_code code,
+ tree expr_type,
+ const value_range_base *vr0_,
+ const value_range_base *vr1_)
+{
+ if (!supported_types_p (vr, expr_type)
+ || !defined_ranges_p (vr, vr0_, vr1_))
+ return;
+ const range_operator *op = get_range_op_handler (vr, code, expr_type);
+ if (!op)
+ return;
+
+ value_range_base vr0 = drop_undefines_to_varying (vr0_, expr_type);
+ value_range_base vr1 = drop_undefines_to_varying (vr1_, expr_type);
+ if (range_fold_binary_symbolics_p (vr, code, expr_type, &vr0, &vr1))
+ return;
+
+ *vr = op->fold_range (expr_type,
+ vr0.normalize_addresses (),
+ vr1.normalize_addresses ());
+}
+
+/* Perform a unary operation on a range. */
+
+void
+range_fold_unary_expr (value_range_base *vr,
+ enum tree_code code, tree expr_type,
+ const value_range_base *vr0,
+ tree vr0_type)
+{
+ if (!supported_types_p (vr, expr_type, vr0_type)
+ || !defined_ranges_p (vr, vr0))
+ return;
+ const range_operator *op = get_range_op_handler (vr, code, expr_type);
+ if (!op)
+ return;
+
+ if (range_fold_unary_symbolics_p (vr, code, expr_type, vr0))
+ return;
+
+ *vr = op->fold_range (expr_type,
+ vr0->normalize_addresses (),
+ value_range_base (expr_type));
}
/* Given a COND_EXPR COND of the form 'V OP W', and an SSA name V,
@@ -5997,7 +6020,24 @@ value_range::union_ (const value_range *other)
}
}
-/* Normalize symbolics into constants. */
+/* Normalize addresses into constants. */
+
+value_range_base
+value_range_base::normalize_addresses () const
+{
+ if (!POINTER_TYPE_P (type ()) || range_has_numeric_bounds_p (this))
+ return *this;
+
+ if (!range_includes_zero_p (this))
+ {
+ gcc_checking_assert (TREE_CODE (m_min) == ADDR_EXPR
+ || TREE_CODE (m_max) == ADDR_EXPR);
+ return range_nonzero (type ());
+ }
+ return value_range_base (type ());
+}
+
+/* Normalize symbolics and addresses into constants. */
value_range_base
value_range_base::normalize_symbolics () const
@@ -6008,7 +6048,7 @@ value_range_base::normalize_symbolics () const
bool min_symbolic = !is_gimple_min_invariant (min ());
bool max_symbolic = !is_gimple_min_invariant (max ());
if (!min_symbolic && !max_symbolic)
- return *this;
+ return normalize_addresses ();
// [SYM, SYM] -> VARYING
if (min_symbolic && max_symbolic)
diff --git a/gcc/tree-vrp.h b/gcc/tree-vrp.h
index d20d0043ba3..a3f9e90699d 100644
--- a/gcc/tree-vrp.h
+++ b/gcc/tree-vrp.h
@@ -86,6 +86,7 @@ public:
static bool supports_type_p (tree);
value_range_base normalize_symbolics () const;
+ value_range_base normalize_addresses () const;
static const unsigned int m_max_pairs = 2;
bool contains_p (tree) const;
