This is the mail archive of the
`gcc-patches@gcc.gnu.org`
mailing list for the GCC project.

Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|

Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |

Other format: | [Raw text] |

*From*: Alexandre Oliva <aoliva at redhat dot com>*To*: Jeff Law <law at redhat dot com>*Cc*: gcc-patches at gcc dot gnu dot org*Date*: Wed, 19 Dec 2018 09:04:03 -0200*Subject*: Re: [PR86153] simplify more overflow tests in VRP*References*: <oro99j9k41.fsf@lxoliva.fsfla.org> <67b3141e-ea22-1587-6617-72176acdc71a@redhat.com>

On Dec 18, 2018, Jeff Law <law@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

**Follow-Ups**:**Re: [PR86153] simplify more overflow tests in VRP***From:*Alexandre Oliva

**References**:**[PR86153] simplify more overflow tests in VRP***From:*Alexandre Oliva

**Re: [PR86153] simplify more overflow tests in VRP***From:*Jeff Law

Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|

Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |