Message ID | 20180912075951.GD8250@tucnak |
---|---|
State | New |
Headers | show |
Series | Fix fold-const (X & C) ? C : 0 optimization (PR middle-end/87248) | expand |
On Wed, 12 Sep 2018, Jakub Jelinek wrote: > Hi! > > The following testcase is miscompiled, because it optimizes > (x & -128) ? -128 : 0 to (x & -128) when it shouldn't. The problem is if > the type of the COND_EXPR has smaller precision than the BIT_AND_EXPR in the > test, we verify that integer_pow2p (arg1), which is true in this case (-128 > in signed char is a power of two), and then operand_equal_p between that > value and the constant in BIT_AND_EXPR's second argument (operand_equal_p > compares only the value, and -128 == -128). The following patch verifies > also that the BIT_AND_EXPR's second operand is a power of two. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and > release branches? OK. Richard. > > 2018-09-12 Jakub Jelinek <jakub@redhat.com> > > PR middle-end/87248 > * fold-const.c (fold_ternary_loc) <case COND_EXPR>: Verify also that > BIT_AND_EXPR's second operand is a power of two. Formatting fix. > > * c-c++-common/torture/pr87248.c: New test. > > --- gcc/fold-const.c.jj 2018-09-06 09:41:59.000000000 +0200 > +++ gcc/fold-const.c 2018-09-08 00:12:28.332418784 +0200 > @@ -11607,10 +11607,16 @@ fold_ternary_loc (location_t loc, enum t > && integer_pow2p (arg1) > && TREE_CODE (TREE_OPERAND (arg0, 0)) == BIT_AND_EXPR > && operand_equal_p (TREE_OPERAND (TREE_OPERAND (arg0, 0), 1), > - arg1, OEP_ONLY_CONST)) > + arg1, OEP_ONLY_CONST) > + /* operand_equal_p compares just value, not precision, so e.g. > + arg1 could be 8-bit -128 and be power of two, but BIT_AND_EXPR > + second operand 32-bit -128, which is not a power of two (or vice > + versa. */ > + && integer_pow2p (TREE_OPERAND (TREE_OPERAND (arg0, 0), 1))) > return pedantic_non_lvalue_loc (loc, > - fold_convert_loc (loc, type, > - TREE_OPERAND (arg0, 0))); > + fold_convert_loc (loc, type, > + TREE_OPERAND (arg0, > + 0))); > > /* Disable the transformations below for vectors, since > fold_binary_op_with_conditional_arg may undo them immediately, > --- gcc/testsuite/c-c++-common/torture/pr87248.c.jj 2018-09-08 01:13:43.431334239 +0200 > +++ gcc/testsuite/c-c++-common/torture/pr87248.c 2018-09-08 01:14:55.446593520 +0200 > @@ -0,0 +1,36 @@ > +/* PR middle-end/87248 */ > +/* { dg-do run } */ > + > +void > +foo (signed char *p, int q) > +{ > + *p = q & (-__SCHAR_MAX__ - 1) ? (-__SCHAR_MAX__ - 1) : 0; > +} > + > +int > +bar (long long x) > +{ > + return x & (-__INT_MAX__ - 1) ? (-__INT_MAX__ - 1) : 0; > +} > + > +int > +main () > +{ > +#if __INT_MAX__ > 4 * __SCHAR_MAX__ > + signed char a[4]; > + foo (a, __SCHAR_MAX__ + 1U); > + foo (a + 1, 2 * (__SCHAR_MAX__ + 1U)); > + foo (a + 2, -__INT_MAX__ - 1); > + foo (a + 3, (__SCHAR_MAX__ + 1U) / 2); > + if (a[0] != (-__SCHAR_MAX__ - 1) || a[1] != a[0] || a[2] != a[0] || a[3] != 0) > + __builtin_abort (); > +#endif > +#if __LONG_LONG_MAX__ > 4 * __INT_MAX__ > + if (bar (__INT_MAX__ + 1LL) != (-__INT_MAX__ - 1) > + || bar (2 * (__INT_MAX__ + 1LL)) != (-__INT_MAX__ - 1) > + || bar (-__LONG_LONG_MAX__ - 1) != (-__INT_MAX__ - 1) > + || bar ((__INT_MAX__ + 1LL) / 2) != 0) > + __builtin_abort (); > +#endif > + return 0; > +} > > Jakub > >
--- gcc/fold-const.c.jj 2018-09-06 09:41:59.000000000 +0200 +++ gcc/fold-const.c 2018-09-08 00:12:28.332418784 +0200 @@ -11607,10 +11607,16 @@ fold_ternary_loc (location_t loc, enum t && integer_pow2p (arg1) && TREE_CODE (TREE_OPERAND (arg0, 0)) == BIT_AND_EXPR && operand_equal_p (TREE_OPERAND (TREE_OPERAND (arg0, 0), 1), - arg1, OEP_ONLY_CONST)) + arg1, OEP_ONLY_CONST) + /* operand_equal_p compares just value, not precision, so e.g. + arg1 could be 8-bit -128 and be power of two, but BIT_AND_EXPR + second operand 32-bit -128, which is not a power of two (or vice + versa. */ + && integer_pow2p (TREE_OPERAND (TREE_OPERAND (arg0, 0), 1))) return pedantic_non_lvalue_loc (loc, - fold_convert_loc (loc, type, - TREE_OPERAND (arg0, 0))); + fold_convert_loc (loc, type, + TREE_OPERAND (arg0, + 0))); /* Disable the transformations below for vectors, since fold_binary_op_with_conditional_arg may undo them immediately, --- gcc/testsuite/c-c++-common/torture/pr87248.c.jj 2018-09-08 01:13:43.431334239 +0200 +++ gcc/testsuite/c-c++-common/torture/pr87248.c 2018-09-08 01:14:55.446593520 +0200 @@ -0,0 +1,36 @@ +/* PR middle-end/87248 */ +/* { dg-do run } */ + +void +foo (signed char *p, int q) +{ + *p = q & (-__SCHAR_MAX__ - 1) ? (-__SCHAR_MAX__ - 1) : 0; +} + +int +bar (long long x) +{ + return x & (-__INT_MAX__ - 1) ? (-__INT_MAX__ - 1) : 0; +} + +int +main () +{ +#if __INT_MAX__ > 4 * __SCHAR_MAX__ + signed char a[4]; + foo (a, __SCHAR_MAX__ + 1U); + foo (a + 1, 2 * (__SCHAR_MAX__ + 1U)); + foo (a + 2, -__INT_MAX__ - 1); + foo (a + 3, (__SCHAR_MAX__ + 1U) / 2); + if (a[0] != (-__SCHAR_MAX__ - 1) || a[1] != a[0] || a[2] != a[0] || a[3] != 0) + __builtin_abort (); +#endif +#if __LONG_LONG_MAX__ > 4 * __INT_MAX__ + if (bar (__INT_MAX__ + 1LL) != (-__INT_MAX__ - 1) + || bar (2 * (__INT_MAX__ + 1LL)) != (-__INT_MAX__ - 1) + || bar (-__LONG_LONG_MAX__ - 1) != (-__INT_MAX__ - 1) + || bar ((__INT_MAX__ + 1LL) / 2) != 0) + __builtin_abort (); +#endif + return 0; +}