| Message ID | 7983915.vF9qLkl6hX@polaris |
|---|---|
| State | New |
| Headers | show |
On Wed, Apr 19, 2017 at 08:20:40AM +0200, Eric Botcazou wrote: > --- tree-vrp.c (revision 246960) > +++ tree-vrp.c (working copy) > @@ -2461,7 +2461,19 @@ extract_range_from_binary_expr_1 (value_ > else if (min_op0) > wmin = min_op0; > else if (min_op1) > - wmin = minus_p ? wi::neg (min_op1) : min_op1; > + { > + if (minus_p) > + { > + wmin = wi::neg (min_op1); > + > + /* Check for overflow. */ > + if (wi::cmp (0, min_op1, sgn) > + != wi::cmp (wmin, 0, sgn)) I know this attempts to be a copy of what is used elsewhere, but at least there it is a result of wi::sub etc. Wouldn't it be simpler to if (sgn == SIGNED && wi::neg_p (min_op1) && wi::neg_p (wmin)) min_ovf = 1; else if (sgn == UNSIGNED && wi::ne_p (min_op1, 0)) min_ovf = -1; I mean, for SIGNED if min_op1 is 0, then wmin is 0 to and we want min_ovf = 0; If it is positive, wmin will be surely negative and again we want min_ovf = 0. Only if it is negative and its negation is negative too we want min_ovf = 1 (i.e. wi::cmps (0, most_negative) result). For UNSIGNED, if min_op1 is 0, again all 3 wi::cmp will yield 0 and min_ovf = 0. If it is non-zero, it is > 0, therefore it the first wi::cmp will return -1, the second wi::cmp returns 1 and the third one -1. Is that what we want (e.g. the UNSIGNED case to overflow pretty much always except for 0 which should be optimized away anyway)? Or, shouldn't we just set if (!min_op0 && min_op1 && minus_p) min_op0 = build_int_cst (expr_type, 0); before the if (min_op0 && min_op1) case and don't duplicate that? Jakub
Index: tree-vrp.c =================================================================== --- tree-vrp.c (revision 246960) +++ tree-vrp.c (working copy) @@ -2461,7 +2461,19 @@ extract_range_from_binary_expr_1 (value_ else if (min_op0) wmin = min_op0; else if (min_op1) - wmin = minus_p ? wi::neg (min_op1) : min_op1; + { + if (minus_p) + { + wmin = wi::neg (min_op1); + + /* Check for overflow. */ + if (wi::cmp (0, min_op1, sgn) + != wi::cmp (wmin, 0, sgn)) + min_ovf = wi::cmp (0, min_op1, sgn); + } + else + wmin = min_op1; + } else wmin = wi::shwi (0, prec); @@ -2489,7 +2501,19 @@ extract_range_from_binary_expr_1 (value_ else if (max_op0) wmax = max_op0; else if (max_op1) - wmax = minus_p ? wi::neg (max_op1) : max_op1; + { + if (minus_p) + { + wmax = wi::neg (max_op1); + + /* Check for overflow. */ + if (wi::cmp (0, max_op1, sgn) + != wi::cmp (wmax, 0, sgn)) + max_ovf = wi::cmp (0, max_op1, sgn); + } + else + wmax = max_op1; + } else wmax = wi::shwi (0, prec);
Hi, this is a regression on the mainline, but the underlying issue is present on the branches too, and similar to PR tree-optimization/79666, but this time for the invariant part instead of the symbolic one. In extract_range_from_binary_expr_1, when the invariant part of the symbolic computation overflows, we drop to varying: /* If we have overflow for the constant part and the resulting range will be symbolic, drop to VR_VARYING. */ if ((min_ovf && sym_min_op0 != sym_min_op1) || (max_ovf && sym_max_op0 != sym_max_op1)) { set_value_range_to_varying (vr); return; } But we fail to compute the overflow when the operation is MINUS_EXPR and the first operand is 0, i.e. we can silently negate INT_MIN and fail to bail out. Fixed by computing overflow in this case too. Tested on x86_64-suse-linux, OK for mainline and 6 branch? 2017-04-19 Eric Botcazou <ebotcazou@adacore.com> PR tree-optimization/80426 * tree-vrp.c (extract_range_from_binary_expr_1): For an additive operation on symbolic operands, also compute the overflow for the invariant part when the operation degenerates into a negation. 2017-04-19 Eric Botcazou <ebotcazou@adacore.com> * gcc.c-torture/execute/20170419-1.c: New test.