| Message ID | 20150212074040.GU1746@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 12 Feb 2015, Jakub Jelinek wrote: > Hi! > > We pass the values after STRIP_NOPS to *ROTATE_EXPR build, which is > undesirable, as the testcase shows, the operand could very well end up being > a pointer rather than integer. Ugh, indeed tree_nop_conversion_p allows conversion to/from pointer types... (I wonder if tree_nop_conversion_p should simply map to useless_type_conversion_p ... for example tree_nop_conversion_p happily drops conversion between pointers to different address-spaces). Of course useless_type_conversion_p preserves sign-changes as well. > This patch fixes it by passing the original trees instead. > Alternatively we could fold_convert the STRIP_NOPS result to the original > type, but I'd be afraid that in the common case that would just create more > GC garbage. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Ok. Thanks, Richard. > 2015-02-12 Jakub Jelinek <jakub@redhat.com> > > PR tree-optimization/65014 > * fold-const.c (fold_binary_loc): When creating {L,R}ROTATE_EXPR, > use original second operand of arg0 or arg1 instead of > that adjusted by STRIP_NOPS. > > * gcc.c-torture/compile/pr65014.c: New test. > > --- gcc/fold-const.c.jj 2015-01-22 19:30:59.000000000 +0100 > +++ gcc/fold-const.c 2015-02-11 15:06:52.628078961 +0100 > @@ -10261,7 +10261,9 @@ fold_binary_loc (location_t loc, > tem = build2_loc (loc, LROTATE_EXPR, > TREE_TYPE (TREE_OPERAND (arg0, 0)), > TREE_OPERAND (arg0, 0), > - code0 == LSHIFT_EXPR ? tree01 : tree11); > + code0 == LSHIFT_EXPR > + ? TREE_OPERAND (arg0, 1) > + : TREE_OPERAND (arg1, 1)); > return fold_convert_loc (loc, type, tem); > } > else if (code11 == MINUS_EXPR) > @@ -10283,7 +10285,8 @@ fold_binary_loc (location_t loc, > ? LROTATE_EXPR > : RROTATE_EXPR), > TREE_TYPE (TREE_OPERAND (arg0, 0)), > - TREE_OPERAND (arg0, 0), tree01)); > + TREE_OPERAND (arg0, 0), > + TREE_OPERAND (arg0, 1))); > } > else if (code01 == MINUS_EXPR) > { > @@ -10304,7 +10307,7 @@ fold_binary_loc (location_t loc, > ? LROTATE_EXPR > : RROTATE_EXPR), > TREE_TYPE (TREE_OPERAND (arg0, 0)), > - TREE_OPERAND (arg0, 0), tree11)); > + TREE_OPERAND (arg0, 0), TREE_OPERAND (arg1, 1))); > } > } > } > --- gcc/testsuite/gcc.c-torture/compile/pr65014.c.jj 2015-02-11 15:21:19.175681614 +0100 > +++ gcc/testsuite/gcc.c-torture/compile/pr65014.c 2015-02-11 15:22:18.040703653 +0100 > @@ -0,0 +1,10 @@ > +/* PR tree-optimization/65014 */ > +/* { dg-do compile { target int32plus } } */ > + > +extern int x; > + > +unsigned > +foo (unsigned int y) > +{ > + return (y << ((__INTPTR_TYPE__) &x)) | (y >> (32 - ((__INTPTR_TYPE__) &x))); > +} > > Jakub > >
--- gcc/fold-const.c.jj 2015-01-22 19:30:59.000000000 +0100 +++ gcc/fold-const.c 2015-02-11 15:06:52.628078961 +0100 @@ -10261,7 +10261,9 @@ fold_binary_loc (location_t loc, tem = build2_loc (loc, LROTATE_EXPR, TREE_TYPE (TREE_OPERAND (arg0, 0)), TREE_OPERAND (arg0, 0), - code0 == LSHIFT_EXPR ? tree01 : tree11); + code0 == LSHIFT_EXPR + ? TREE_OPERAND (arg0, 1) + : TREE_OPERAND (arg1, 1)); return fold_convert_loc (loc, type, tem); } else if (code11 == MINUS_EXPR) @@ -10283,7 +10285,8 @@ fold_binary_loc (location_t loc, ? LROTATE_EXPR : RROTATE_EXPR), TREE_TYPE (TREE_OPERAND (arg0, 0)), - TREE_OPERAND (arg0, 0), tree01)); + TREE_OPERAND (arg0, 0), + TREE_OPERAND (arg0, 1))); } else if (code01 == MINUS_EXPR) { @@ -10304,7 +10307,7 @@ fold_binary_loc (location_t loc, ? LROTATE_EXPR : RROTATE_EXPR), TREE_TYPE (TREE_OPERAND (arg0, 0)), - TREE_OPERAND (arg0, 0), tree11)); + TREE_OPERAND (arg0, 0), TREE_OPERAND (arg1, 1))); } } } --- gcc/testsuite/gcc.c-torture/compile/pr65014.c.jj 2015-02-11 15:21:19.175681614 +0100 +++ gcc/testsuite/gcc.c-torture/compile/pr65014.c 2015-02-11 15:22:18.040703653 +0100 @@ -0,0 +1,10 @@ +/* PR tree-optimization/65014 */ +/* { dg-do compile { target int32plus } } */ + +extern int x; + +unsigned +foo (unsigned int y) +{ + return (y << ((__INTPTR_TYPE__) &x)) | (y >> (32 - ((__INTPTR_TYPE__) &x))); +}
Hi! We pass the values after STRIP_NOPS to *ROTATE_EXPR build, which is undesirable, as the testcase shows, the operand could very well end up being a pointer rather than integer. This patch fixes it by passing the original trees instead. Alternatively we could fold_convert the STRIP_NOPS result to the original type, but I'd be afraid that in the common case that would just create more GC garbage. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2015-02-12 Jakub Jelinek <jakub@redhat.com> PR tree-optimization/65014 * fold-const.c (fold_binary_loc): When creating {L,R}ROTATE_EXPR, use original second operand of arg0 or arg1 instead of that adjusted by STRIP_NOPS. * gcc.c-torture/compile/pr65014.c: New test. Jakub