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  <ja...@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
> 
> 

-- 
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Jennifer Guild,
Dilip Upmanyu, Graham Norton HRB 21284 (AG Nuernberg)

Reply via email to