| Message ID | 5710C60B.2080308@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Fri, Apr 15, 2016 at 12:44 PM, kugan <kugan.vivekanandarajah@linaro.org> wrote: > As pointed out by Richard, for signed & sign-bit-CST value range should be > [-INF, 0] range, not a [-INF, INF] range as happens now. > > This patch fixes this. I bootstrapped and regression tested for > x86-64-linux-gnu with no new regression. Is this OK for statege-1. I think you need to check vr0 for singleton-constant == sign_bit as well, there is nothing that canonicalizes this during propagation. Otherwise ok. Thanks, Richard. > Thanks, > Kugan > > > gcc/ChangeLog: > > 2016-04-14 Kugan Vivekanandarajah <kuganv@linaro.org> > > PR middle-end/68217 > * tree-vrp.c (extract_range_from_binary_expr_1): In case of signed & > sign-bit-CST, > generate [-INF, 0] instead of [-INF, INF]. > > > gcc/testsuite/ChangeLog: > > 2016-04-14 Kugan Vivekanandarajah <kuganv@linaro.org> > > PR middle-end/68217 > * gcc.dg/pr68217.c: New test.
Hi Richard, Thanks for the review. On 27/04/16 00:14, Richard Biener wrote: > On Fri, Apr 15, 2016 at 12:44 PM, kugan > <kugan.vivekanandarajah@linaro.org> wrote: >> As pointed out by Richard, for signed & sign-bit-CST value range should be >> [-INF, 0] range, not a [-INF, INF] range as happens now. >> >> This patch fixes this. I bootstrapped and regression tested for >> x86-64-linux-gnu with no new regression. Is this OK for statege-1. > > I think you need to check vr0 for singleton-constant == sign_bit as well, > there is nothing that canonicalizes this during propagation. > > Otherwise ok. Committed as r238846 with the above change. Thanks, Kugan > > Thanks, > Richard. > >> Thanks, >> Kugan >> >> >> gcc/ChangeLog: >> >> 2016-04-14 Kugan Vivekanandarajah <kuganv@linaro.org> >> >> PR middle-end/68217 >> * tree-vrp.c (extract_range_from_binary_expr_1): In case of signed & >> sign-bit-CST, >> generate [-INF, 0] instead of [-INF, INF]. >> >> >> gcc/testsuite/ChangeLog: >> >> 2016-04-14 Kugan Vivekanandarajah <kuganv@linaro.org> >> >> PR middle-end/68217 >> * gcc.dg/pr68217.c: New test.
diff --git a/gcc/testsuite/gcc.dg/pr68217.c b/gcc/testsuite/gcc.dg/pr68217.c index e69de29..8197363 100644 --- a/gcc/testsuite/gcc.dg/pr68217.c +++ b/gcc/testsuite/gcc.dg/pr68217.c @@ -0,0 +1,14 @@ + +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-vrp1" } */ + +int foo (void) +{ + volatile int a = -1; + long long b = -9223372036854775807LL - 1; // LLONG_MIN + long long x = (a & b); // x == 0x8000000000000000 + if (x < 1LL) { ; } else { __builtin_abort(); } + return 0; +} + +/* { dg-final { scan-tree-dump "x_*: \\[-INF, 0\\]" "vrp1" } } */ diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c index bbdf9ce..3a021f3 100644 --- a/gcc/tree-vrp.c +++ b/gcc/tree-vrp.c @@ -3115,6 +3115,22 @@ extract_range_from_binary_expr_1 (value_range *vr, if (int_cst_range1 && tree_int_cst_sgn (vr1.min) >= 0) wmax = wi::min (wmax, vr1.max, TYPE_SIGN (expr_type)); max = wide_int_to_tree (expr_type, wmax); + cmp = compare_values (min, max); + /* PR68217: In case of signed & sign-bit-CST should + result in [-INF, 0] instead of [-INF, INF]. */ + if (cmp == -2 || cmp == 1) + { + wide_int sign_bit = + wi::set_bit_in_zero (TYPE_PRECISION (expr_type) - 1, + TYPE_PRECISION (expr_type)); + if (!TYPE_UNSIGNED (expr_type) + && value_range_constant_singleton (&vr1) + && !wi::cmps (vr1.min, sign_bit)) + { + min = TYPE_MIN_VALUE (expr_type), + max = build_int_cst (expr_type, 0); + } + } } else if (code == BIT_IOR_EXPR) {