Take the following example: int f(int i, int j) { if (i <= j - 1) if (i >= 0) { return i >= j; } return 0; } Note this might show up somewhere but I don't know if it actually does (I made this up while looking into some missed optimizations due the patch which fixes PR 23666). The problem if I am looking at this correctly is that: Visiting statement: D.1631_3 = j_2 - 1; Found new range for D.1631_3: VARYING we say the range for D.1631_3 is varying but shouldn't we say the range is [j_2 - 1, j_2 - 1] ? That should fix the issue as we then we get the upper bound correctly as shown by: int f(int i, int j) { if (i < j) if (i >= 0) { return i >= j; } return 0; } Note as I said before, I don't know how many times this shows but I don't doubt that people would write i <= j -1 instead of i < j all the time to show what it really does for integers and loops.
Even the simple code like: int f(int i, int j ) { int k; k = i+ - 1; return k < i; } Does not get VRP to optimize it which means we don't do that much symbolic ranges as we should. From looking at things, the only time we get a symbolic range is from scev and never make it ourselves which seems wrong.
extract_range_from_binary_expr should do symbolic ranges for stuff like this (or at least it seems like it should).
This needs PR 25148 fixed to do the correct thing for comment #1 as we get the wrong answer for a + -1 < a, we get false when we should get true.
I run into a different regression if we try to compile the first example in comment #0.
Note this was the simple fix which exposes those latent bugs as far as I can see that should work, we get the correct range but the rest of VRP goes bonkers: Index: tree-vrp.c =================================================================== --- tree-vrp.c (revision 107604) +++ tree-vrp.c (working copy) @@ -1226,7 +1226,14 @@ extract_range_from_binary_expr (value_ra || symbolic_range_p (&vr0) || symbolic_range_p (&vr1)) { - set_value_range_to_varying (vr); + if ((code != PLUS_EXPR + && code != MINUS_EXPR) + || !is_gimple_min_invariant (op1)) + set_value_range_to_varying (vr); + else + { + set_value_range (vr, VR_RANGE, expr, expr, NULL); + } return; }
(In reply to comment #5) > Note this was the simple fix which exposes those latent bugs as far as I can > see that should work, we get the correct range but the rest of VRP goes > bonkers: I should also note it does not do the correct thing for -fwrapv either. For the testcase in comment #1: int f(int i, int j ) { int k; k = i+ - 1; return k < i; } i = INT_MIN k will equal INT_MAX so the comparision will be wrong to return true as INT_MAX is not less than INT_MIN. VRP does not like symbolic ranges at all.
Confirmed.
Now the patch for 25148 fixes the wrong answer for the testcase in comment #2 if the patch in comment #5 is applied. It needs Index: tree-ssa-propagate.c =================================================================== *** tree-ssa-propagate.c (revision 113273) --- tree-ssa-propagate.c (working copy) *************** substitute_and_fold (prop_value_t *prop_ *** 1127,1133 **** if (use_ranges_p) did_replace = fold_predicate_in (stmt); ! if (prop_value) { /* Only replace real uses if we couldn't fold the statement using value range information (value range --- 1127,1134 ---- if (use_ranges_p) did_replace = fold_predicate_in (stmt); ! if (prop_value ! && is_gimple_val (prop_value)) { /* Only replace real uses if we couldn't fold the statement using value range information (value range though to not ICE for -fwrapv. Also the patches don't handle the testcase in comment #1.
The testcase in comment #1 is fixed by comparison canonicalization of i <= j - 1 to i < j. Of course it fails again if we use a temporary for j - 1 like in the testcases in other comments.
The first testcase in the original report is fixed by comparison canonicalization, even if a temporary is used (forwprop re-instantiates the canonicalized comparison). The testcase in comment #1 is fixed by FRE simplifying binary expressions: Value numbering D.1178_3 stmt = D.1178_3 = k_2 < i_1(D); RHS k_2 < i_1(D) simplified to 1 has constants 0 So none of the testcases is useful to show missing optimizations in VRP anymore.
Thus, closing. VRP isn't the best place to deal with symbolic ranges anyway.