On Dec 18, 2018, Jeff Law <l...@redhat.com> wrote:
>> Although such overflow tests could be uniformly simplified to compares
>> against a constant, the original code would only perform such
>> simplifications when the test could be resolved to an equality test
>> against zero. I've thus avoided introducing compares against other
>> constants, and instead added code that will only simplify overflow
>> tests that weren't simplified before when the condition can be
>> evaluated at compile time.
> That limitation was precisely what my (unsubmitted) patch was trying
> to address :-)
This patch is what I was getting at in my earlier email.
These transformations are already performed elsewhere, e.g. when
forwprop is enabled, but given sufficiently complex code to begin with,
as in the pr83239 testcases, forwprop presumably runs too early to be
able to simplify the tests and then non-early vrp comes to the rescue.
Presumably with more convoluted tests than the ones I'm introducing,
forwprop would be unable to infer the ranges, and then we'd really
depend on vrp, but I didn't dig deep enough to try and create a testcase
that wouldn't be optimized by forwprop, only by vrp. That's why the
tests disable forwprop.
[PR86153] simplify vrp overflow simplifications
It turns out there was apparently no reason to avoid simplifying every
overflow comparison to a compare with a constant, it was not
profitable because earlier VRP couldn't deal with that as well as it
does now.
So, make the transformation unconditionally, even in cases we'd have
transformed differently before my previous patch, and let the
now-better optimizations resolve them to boolean constants or to
equality tests when possible.
The only significant difference is that where we'd turn A>B after
B=A+1 into B!=0, we'll now turn it into A!=-1u. That might seem
worse, but considering that test canonicalization will have moved the
(probably) earliest SSA version to the first operand, that form is
more likely to allow the later SSA definition, presumably in terms of
the earlier one, to be completely removed, which would have otherwise
required propagation of the assignment to B into the compare, which is
possible in equality tests, but not in other kinds of overflow tests.
Regstrapped on x86_64- and i686-linux-gnu. Ok to install?
for gcc/ChangeLog
PR testsuite/86153
PR middle-end/83239
* vr-values.c
(vr_values::vrp_evaluate_conditional_warnv_with_ops): Simplify
the handling of overflow comparisons.
for gcc/testsuite/ChangeLog
PR testsuite/86153
PR middle-end/83239
* gcc.dg/vrp-overflow-2.c: New.
---
gcc/testsuite/gcc.dg/vrp-overflow-2.c | 35 ++++++++++++++++++
gcc/vr-values.c | 66 +++------------------------------
2 files changed, 40 insertions(+), 61 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/vrp-overflow-2.c
diff --git a/gcc/testsuite/gcc.dg/vrp-overflow-2.c
b/gcc/testsuite/gcc.dg/vrp-overflow-2.c
new file mode 100644
index 000000000000..a905471bcaa1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vrp-overflow-2.c
@@ -0,0 +1,35 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fno-tree-forwprop" } */
+
+void __attribute__((noreturn)) undefined ();
+
+int tij (unsigned i)
+{
+ unsigned j = i + 1;
+
+ if (j == 0)
+ return 0;
+
+ if (i > j)
+ undefined ();
+
+ return 1;
+}
+
+int tji (unsigned i)
+{
+ unsigned j = i - 1;
+
+ if (i == 0)
+ return 0;
+
+ if (j > i)
+ undefined ();
+
+ return 1;
+}
+
+int main (int argc, char *argv[]) {
+ tij (argc);
+ tji (argc);
+}
diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index d71a703ab550..49c5da9cb515 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -2305,70 +2305,14 @@ vr_values::vrp_evaluate_conditional_warnv_with_ops
(enum tree_code code,
&& !POINTER_TYPE_P (TREE_TYPE (op0)))
return NULL_TREE;
- /* If OP0 CODE OP1 is an overflow comparison, if it can be expressed
- as a simple equality test, then prefer that over its current form
- for evaluation.
-
- An overflow test which collapses to an equality test can always be
- expressed as a comparison of one argument against zero. Overflow
- occurs when the chosen argument is zero and does not occur if the
- chosen argument is not zero. */
+ /* If OP0 CODE OP1 is an overflow comparison, it can be expressed as
+ a test involving only one of the operands and a constant, so
+ prefer that over its current form for evaluation. */
tree x;
if (overflow_comparison_p (code, op0, op1, use_equiv_p, &x))
{
- wide_int max = wi::max_value (TYPE_PRECISION (TREE_TYPE (op0)),
UNSIGNED);
- /* B = A - 1; if (A < B) -> B = A - 1; if (A == 0)
- B = A - 1; if (A > B) -> B = A - 1; if (A != 0)
- B = A + 1; if (B < A) -> B = A + 1; if (B == 0)
- B = A + 1; if (B > A) -> B = A + 1; if (B != 0) */
- if (integer_zerop (x))
- {
- op1 = x;
- code = (code == LT_EXPR || code == LE_EXPR) ? EQ_EXPR : NE_EXPR;
- }
- /* B = A + 1; if (A > B) -> B = A + 1; if (B == 0)
- B = A + 1; if (A < B) -> B = A + 1; if (B != 0)
- B = A - 1; if (B > A) -> B = A - 1; if (A == 0)
- B = A - 1; if (B < A) -> B = A - 1; if (A != 0) */
- else if (wi::to_wide (x) == max - 1)
- {
- op0 = op1;
- op1 = wide_int_to_tree (TREE_TYPE (op0), 0);
- code = (code == GT_EXPR || code == GE_EXPR) ? EQ_EXPR : NE_EXPR;
- }
- else
- {
- value_range vro, vri;
- if (code == GT_EXPR || code == GE_EXPR)
- {
- vro.set (VR_ANTI_RANGE, TYPE_MIN_VALUE (TREE_TYPE (op0)), x);
- vri.set (VR_RANGE, TYPE_MIN_VALUE (TREE_TYPE (op0)), x);
- }
- else if (code == LT_EXPR || code == LE_EXPR)
- {
- vro.set (VR_RANGE, TYPE_MIN_VALUE (TREE_TYPE (op0)), x);
- vri.set (VR_ANTI_RANGE, TYPE_MIN_VALUE (TREE_TYPE (op0)), x);
- }
- else
- gcc_unreachable ();
- value_range *vr0 = get_value_range (op0);
- /* If vro, the range for OP0 to pass the overflow test, has
- no intersection with *vr0, OP0's known range, then the
- overflow test can't pass, so return the node for false.
- If it is the inverted range, vri, that has no
- intersection, then the overflow test must pass, so return
- the node for true. In other cases, we could proceed with
- a simplified condition comparing OP0 and X, with LE_EXPR
- for previously LE_ or LT_EXPR and GT_EXPR otherwise, but
- the comments next to the enclosing if suggest it's not
- generally profitable to do so. */
- vro.intersect (vr0);
- if (vro.undefined_p ())
- return boolean_false_node;
- vri.intersect (vr0);
- if (vri.undefined_p ())
- return boolean_true_node;
- }
+ op1 = x;
+ code = (code == LT_EXPR || code == LE_EXPR) ? LE_EXPR : GT_EXPR;
}
if ((ret = vrp_evaluate_conditional_warnv_with_ops_using_ranges
--
Alexandre Oliva, freedom fighter https://FSFLA.org/blogs/lxo
Be the change, be Free! FSF Latin America board member
GNU Toolchain Engineer Free Software Evangelist
Hay que enGNUrecerse, pero sin perder la terGNUra jamás-GNUChe
