Message ID | 20110131145256.GC30899@tyan-ft48-01.lab.bos.redhat.com |
---|---|
State | New |
Headers | show |
On Mon, Jan 31, 2011 at 3:52 PM, Jakub Jelinek <jakub@redhat.com> wrote: > Hi! > > The middle-end considers conversions between unsigned size_t sized > types and sizetype as useless, so my recent fix in bit_value_binop_1 > causes the following testcase to regress. Making sizetype conversions not > useless or making it work more sanely (not saying TYPE_UNSIGNED when it > is actually signed) unfortunately breaks too much and is not appropriate > for stage4, so this patch basically reverts that check in, and just looks > at operand type for comparisons. In addition to that if it sees sign > mismatches between lhs and rhs1 for right shifts or rhs1 and rhs2 for > comparisons, it plays safe and doesn't try to optimize. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Ok. Thanks, Richard. > 2011-01-31 Jakub Jelinek <jakub@redhat.com> > Richard Guenther <rguenther@suse.de> > > PR tree-optimization/47538 > * tree-ssa-ccp.c (bit_value_binop_1): For uns computation use > type instead of r1type, except for comparisons. For right > shifts and comparisons punt if there are mismatches in > sizetype vs. non-sizetype types. > > * gcc.c-torture/execute/pr47538.c: New test. > > --- gcc/tree-ssa-ccp.c.jj 2011-01-31 11:46:43.000000000 +0100 > +++ gcc/tree-ssa-ccp.c 2011-01-31 12:21:32.759546325 +0100 > @@ -1768,8 +1768,8 @@ bit_value_binop_1 (enum tree_code code, > tree r1type, double_int r1val, double_int r1mask, > tree r2type, double_int r2val, double_int r2mask) > { > - bool uns = (TREE_CODE (r1type) == INTEGER_TYPE > - && TYPE_IS_SIZETYPE (r1type) ? 0 : TYPE_UNSIGNED (r1type)); > + bool uns = (TREE_CODE (type) == INTEGER_TYPE > + && TYPE_IS_SIZETYPE (type) ? 0 : TYPE_UNSIGNED (type)); > /* Assume we'll get a constant result. Use an initial varying value, > we fall back to varying in the end if necessary. */ > *mask = double_int_minus_one; > @@ -1836,6 +1836,13 @@ bit_value_binop_1 (enum tree_code code, > } > else if (shift < 0) > { > + /* ??? We can have sizetype related inconsistencies in > + the IL. */ > + if ((TREE_CODE (r1type) == INTEGER_TYPE > + && (TYPE_IS_SIZETYPE (r1type) > + ? 0 : TYPE_UNSIGNED (r1type))) != uns) > + break; > + > shift = -shift; > *mask = double_int_rshift (r1mask, shift, > TYPE_PRECISION (type), !uns); > @@ -1946,6 +1953,14 @@ bit_value_binop_1 (enum tree_code code, > if (double_int_negative_p (r1mask) || double_int_negative_p (r2mask)) > break; > > + /* For comparisons the signedness is in the comparison operands. */ > + uns = (TREE_CODE (r1type) == INTEGER_TYPE > + && TYPE_IS_SIZETYPE (r1type) ? 0 : TYPE_UNSIGNED (r1type)); > + /* ??? We can have sizetype related inconsistencies in the IL. */ > + if ((TREE_CODE (r2type) == INTEGER_TYPE > + && TYPE_IS_SIZETYPE (r2type) ? 0 : TYPE_UNSIGNED (r2type)) != uns) > + break; > + > /* If we know the most significant bits we know the values > value ranges by means of treating varying bits as zero > or one. Do a cross comparison of the max/min pairs. */ > --- gcc/testsuite/gcc.c-torture/execute/pr47538.c.jj 2011-01-31 11:50:24.275638525 +0100 > +++ gcc/testsuite/gcc.c-torture/execute/pr47538.c 2011-01-31 11:50:24.275638525 +0100 > @@ -0,0 +1,73 @@ > +/* PR tree-optimization/47538 */ > + > +struct S > +{ > + double a, b, *c; > + unsigned long d; > +}; > + > +__attribute__((noinline, noclone)) void > +foo (struct S *x, const struct S *y) > +{ > + const unsigned long n = y->d + 1; > + const double m = 0.25 * (y->b - y->a); > + x->a = y->a; > + x->b = y->b; > + if (n == 1) > + { > + x->c[0] = 0.; > + } > + else if (n == 2) > + { > + x->c[1] = m * y->c[0]; > + x->c[0] = 2.0 * x->c[1]; > + } > + else > + { > + double o = 0.0, p = 1.0; > + unsigned long i; > + > + for (i = 1; i <= n - 2; i++) > + { > + x->c[i] = m * (y->c[i - 1] - y->c[i + 1]) / (double) i; > + o += p * x->c[i]; > + p = -p; > + } > + x->c[n - 1] = m * y->c[n - 2] / (n - 1.0); > + o += p * x->c[n - 1]; > + x->c[0] = 2.0 * o; > + } > +} > + > +int > +main (void) > +{ > + struct S x, y; > + double c[4] = { 10, 20, 30, 40 }, d[4], e[4] = { 118, 118, 118, 118 }; > + > + y.a = 10; > + y.b = 6; > + y.c = c; > + x.c = d; > + y.d = 3; > + __builtin_memcpy (d, e, sizeof d); > + foo (&x, &y); > + if (d[0] != 0 || d[1] != 20 || d[2] != 10 || d[3] != -10) > + __builtin_abort (); > + y.d = 2; > + __builtin_memcpy (d, e, sizeof d); > + foo (&x, &y); > + if (d[0] != 60 || d[1] != 20 || d[2] != -10 || d[3] != 118) > + __builtin_abort (); > + y.d = 1; > + __builtin_memcpy (d, e, sizeof d); > + foo (&x, &y); > + if (d[0] != -20 || d[1] != -10 || d[2] != 118 || d[3] != 118) > + __builtin_abort (); > + y.d = 0; > + __builtin_memcpy (d, e, sizeof d); > + foo (&x, &y); > + if (d[0] != 0 || d[1] != 118 || d[2] != 118 || d[3] != 118) > + __builtin_abort (); > + return 0; > +} > > Jakub >
--- gcc/tree-ssa-ccp.c.jj 2011-01-31 11:46:43.000000000 +0100 +++ gcc/tree-ssa-ccp.c 2011-01-31 12:21:32.759546325 +0100 @@ -1768,8 +1768,8 @@ bit_value_binop_1 (enum tree_code code, tree r1type, double_int r1val, double_int r1mask, tree r2type, double_int r2val, double_int r2mask) { - bool uns = (TREE_CODE (r1type) == INTEGER_TYPE - && TYPE_IS_SIZETYPE (r1type) ? 0 : TYPE_UNSIGNED (r1type)); + bool uns = (TREE_CODE (type) == INTEGER_TYPE + && TYPE_IS_SIZETYPE (type) ? 0 : TYPE_UNSIGNED (type)); /* Assume we'll get a constant result. Use an initial varying value, we fall back to varying in the end if necessary. */ *mask = double_int_minus_one; @@ -1836,6 +1836,13 @@ bit_value_binop_1 (enum tree_code code, } else if (shift < 0) { + /* ??? We can have sizetype related inconsistencies in + the IL. */ + if ((TREE_CODE (r1type) == INTEGER_TYPE + && (TYPE_IS_SIZETYPE (r1type) + ? 0 : TYPE_UNSIGNED (r1type))) != uns) + break; + shift = -shift; *mask = double_int_rshift (r1mask, shift, TYPE_PRECISION (type), !uns); @@ -1946,6 +1953,14 @@ bit_value_binop_1 (enum tree_code code, if (double_int_negative_p (r1mask) || double_int_negative_p (r2mask)) break; + /* For comparisons the signedness is in the comparison operands. */ + uns = (TREE_CODE (r1type) == INTEGER_TYPE + && TYPE_IS_SIZETYPE (r1type) ? 0 : TYPE_UNSIGNED (r1type)); + /* ??? We can have sizetype related inconsistencies in the IL. */ + if ((TREE_CODE (r2type) == INTEGER_TYPE + && TYPE_IS_SIZETYPE (r2type) ? 0 : TYPE_UNSIGNED (r2type)) != uns) + break; + /* If we know the most significant bits we know the values value ranges by means of treating varying bits as zero or one. Do a cross comparison of the max/min pairs. */ --- gcc/testsuite/gcc.c-torture/execute/pr47538.c.jj 2011-01-31 11:50:24.275638525 +0100 +++ gcc/testsuite/gcc.c-torture/execute/pr47538.c 2011-01-31 11:50:24.275638525 +0100 @@ -0,0 +1,73 @@ +/* PR tree-optimization/47538 */ + +struct S +{ + double a, b, *c; + unsigned long d; +}; + +__attribute__((noinline, noclone)) void +foo (struct S *x, const struct S *y) +{ + const unsigned long n = y->d + 1; + const double m = 0.25 * (y->b - y->a); + x->a = y->a; + x->b = y->b; + if (n == 1) + { + x->c[0] = 0.; + } + else if (n == 2) + { + x->c[1] = m * y->c[0]; + x->c[0] = 2.0 * x->c[1]; + } + else + { + double o = 0.0, p = 1.0; + unsigned long i; + + for (i = 1; i <= n - 2; i++) + { + x->c[i] = m * (y->c[i - 1] - y->c[i + 1]) / (double) i; + o += p * x->c[i]; + p = -p; + } + x->c[n - 1] = m * y->c[n - 2] / (n - 1.0); + o += p * x->c[n - 1]; + x->c[0] = 2.0 * o; + } +} + +int +main (void) +{ + struct S x, y; + double c[4] = { 10, 20, 30, 40 }, d[4], e[4] = { 118, 118, 118, 118 }; + + y.a = 10; + y.b = 6; + y.c = c; + x.c = d; + y.d = 3; + __builtin_memcpy (d, e, sizeof d); + foo (&x, &y); + if (d[0] != 0 || d[1] != 20 || d[2] != 10 || d[3] != -10) + __builtin_abort (); + y.d = 2; + __builtin_memcpy (d, e, sizeof d); + foo (&x, &y); + if (d[0] != 60 || d[1] != 20 || d[2] != -10 || d[3] != 118) + __builtin_abort (); + y.d = 1; + __builtin_memcpy (d, e, sizeof d); + foo (&x, &y); + if (d[0] != -20 || d[1] != -10 || d[2] != 118 || d[3] != 118) + __builtin_abort (); + y.d = 0; + __builtin_memcpy (d, e, sizeof d); + foo (&x, &y); + if (d[0] != 0 || d[1] != 118 || d[2] != 118 || d[3] != 118) + __builtin_abort (); + return 0; +}