Message ID | 20110422134120.GV17079@tyan-ft48-01.lab.bos.redhat.com |
---|---|
State | New |
Headers | show |
On Fri, Apr 22, 2011 at 3:41 PM, Jakub Jelinek <jakub@redhat.com> wrote: > Hi! > > On the following testcase associate_plusminus optimizes > A + ~A into INTEGER_CST { -1, -1 } with type unsigned short, which > confuses enough following passes on (int) cast of that into > assuming it is -1 instead of 65535. Ick. > Fixed by using build_int_cst_type, which is what e.g. fold-const.c > uses when optimizing X + ~X. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk > and 4.6 (where the bug is just latent)? 4.5 didn't have > associate_plusminus. Ok for both. build_int_cst looks seriously broken, I think we want to make it an alias of build_int_cst_type, or rather call that from build_int_cst. Thanks, Richard. > 2011-04-22 Jakub Jelinek <jakub@redhat.com> > > PR tree-optimization/48717 > * tree-ssa-forwprop.c (associate_plusminus): For A + ~A and > ~A + A optimizations use build_int_cst_type instead of build_int_cst. > > * gcc.c-torture/execute/pr48717.c: New test. > > --- gcc/tree-ssa-forwprop.c.jj 2011-04-01 23:09:21.000000000 +0200 > +++ gcc/tree-ssa-forwprop.c 2011-04-22 11:57:10.000000000 +0200 > @@ -1815,7 +1815,7 @@ associate_plusminus (gimple stmt) > { > /* ~A + A -> -1. */ > code = INTEGER_CST; > - rhs1 = build_int_cst (TREE_TYPE (rhs2), -1); > + rhs1 = build_int_cst_type (TREE_TYPE (rhs2), -1); > rhs2 = NULL_TREE; > gimple_assign_set_rhs_with_ops (&gsi, code, rhs1, NULL_TREE); > gcc_assert (gsi_stmt (gsi) == stmt); > @@ -1915,7 +1915,7 @@ associate_plusminus (gimple stmt) > { > /* A + ~A -> -1. */ > code = INTEGER_CST; > - rhs1 = build_int_cst (TREE_TYPE (rhs1), -1); > + rhs1 = build_int_cst_type (TREE_TYPE (rhs1), -1); > rhs2 = NULL_TREE; > gimple_assign_set_rhs_with_ops (&gsi, code, rhs1, NULL_TREE); > gcc_assert (gsi_stmt (gsi) == stmt); > --- gcc/testsuite/gcc.c-torture/execute/pr48717.c.jj 2011-04-22 11:59:23.000000000 +0200 > +++ gcc/testsuite/gcc.c-torture/execute/pr48717.c 2011-04-22 11:58:48.000000000 +0200 > @@ -0,0 +1,26 @@ > +/* PR tree-optimization/48717 */ > + > +extern void abort (void); > + > +int v = 1, w; > + > +unsigned short > +foo (unsigned short x, unsigned short y) > +{ > + return x + y; > +} > + > +void > +bar (void) > +{ > + v = foo (~w, w); > +} > + > +int > +main () > +{ > + bar (); > + if (v != (unsigned short) -1) > + abort (); > + return 0; > +} > > Jakub >
On Fri, Apr 22, 2011 at 04:50:26PM +0200, Richard Guenther wrote: > build_int_cst looks seriously broken, I think we want to make it > an alias of build_int_cst_type, or rather call that from > build_int_cst. The comment above build_int_cst_type talks about it: "We cannot however make this a default behavior of build_int_cst without more intrusive changes, since there are parts of gcc that rely on the extra precision of the integer constants." Jakub
--- gcc/tree-ssa-forwprop.c.jj 2011-04-01 23:09:21.000000000 +0200 +++ gcc/tree-ssa-forwprop.c 2011-04-22 11:57:10.000000000 +0200 @@ -1815,7 +1815,7 @@ associate_plusminus (gimple stmt) { /* ~A + A -> -1. */ code = INTEGER_CST; - rhs1 = build_int_cst (TREE_TYPE (rhs2), -1); + rhs1 = build_int_cst_type (TREE_TYPE (rhs2), -1); rhs2 = NULL_TREE; gimple_assign_set_rhs_with_ops (&gsi, code, rhs1, NULL_TREE); gcc_assert (gsi_stmt (gsi) == stmt); @@ -1915,7 +1915,7 @@ associate_plusminus (gimple stmt) { /* A + ~A -> -1. */ code = INTEGER_CST; - rhs1 = build_int_cst (TREE_TYPE (rhs1), -1); + rhs1 = build_int_cst_type (TREE_TYPE (rhs1), -1); rhs2 = NULL_TREE; gimple_assign_set_rhs_with_ops (&gsi, code, rhs1, NULL_TREE); gcc_assert (gsi_stmt (gsi) == stmt); --- gcc/testsuite/gcc.c-torture/execute/pr48717.c.jj 2011-04-22 11:59:23.000000000 +0200 +++ gcc/testsuite/gcc.c-torture/execute/pr48717.c 2011-04-22 11:58:48.000000000 +0200 @@ -0,0 +1,26 @@ +/* PR tree-optimization/48717 */ + +extern void abort (void); + +int v = 1, w; + +unsigned short +foo (unsigned short x, unsigned short y) +{ + return x + y; +} + +void +bar (void) +{ + v = foo (~w, w); +} + +int +main () +{ + bar (); + if (v != (unsigned short) -1) + abort (); + return 0; +}