------- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-23 02:41 ------- Subject: [PR rtl-optimization/20532] plus(ashift,ashift) -> mult may overflow
In the sample testcase, if HOST_WIDE_INT is 32-bits wide, we ended up trying to simplify the two shifts into a single multiply. The shift by one was already represented as an add-to-itself. In combine, we turned the shifts by 31 substed into both operands of the plus into multiply by ((HOST_WIDE_INT)1 << 31), and then added the two coefficients, resulting zero. Oops. This patch arranges for us to represent coefficients as CONST_DOUBLEs when needed, avoiding overflows in all cases, since the two coefficients added are in the range [HOST_WIDE_INT_MIN,2*HOST_WIDE_INT_MAX+1]. In order to generate the optimal code that we generate on a 64-bit HOST_WIDE_INT host, I had to also get multiply simplification to apply to CONST_DOUBLE exact log2 constants, such as those produced after the patch. Bootstrapping on amd64-linux-gnu. Ok to install if it passes regtesting? Index: gcc/ChangeLog from Alexandre Oliva <[EMAIL PROTECTED]> PR rtl-optimization/20532 * simplify-rtx.c (simplify_binary_operation_1): Protect from overflow when adding coefficients for PLUS or MINUS. (simplify_binary_operation_1): Handle CONST_DOUBLE exact power of two as multiplier. Index: gcc/simplify-rtx.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/simplify-rtx.c,v retrieving revision 1.234 diff -u -p -r1.234 simplify-rtx.c --- gcc/simplify-rtx.c 21 Mar 2005 14:30:51 -0000 1.234 +++ gcc/simplify-rtx.c 23 Mar 2005 02:30:15 -0000 @@ -1257,44 +1257,67 @@ simplify_binary_operation_1 (enum rtx_co if (! FLOAT_MODE_P (mode)) { - HOST_WIDE_INT coeff0 = 1, coeff1 = 1; + HOST_WIDE_INT coeff0h = 0, coeff1h = 0; + unsigned HOST_WIDE_INT coeff0l = 1, coeff1l = 1; rtx lhs = op0, rhs = op1; if (GET_CODE (lhs) == NEG) - coeff0 = -1, lhs = XEXP (lhs, 0); + { + coeff0l = -1; + coeff0h = -1; + lhs = XEXP (lhs, 0); + } else if (GET_CODE (lhs) == MULT && GET_CODE (XEXP (lhs, 1)) == CONST_INT) - coeff0 = INTVAL (XEXP (lhs, 1)), lhs = XEXP (lhs, 0); + { + coeff0l = INTVAL (XEXP (lhs, 1)); + coeff0h = INTVAL (XEXP (lhs, 1)) < 0 ? -1 : 0; + lhs = XEXP (lhs, 0); + } else if (GET_CODE (lhs) == ASHIFT && GET_CODE (XEXP (lhs, 1)) == CONST_INT && INTVAL (XEXP (lhs, 1)) >= 0 && INTVAL (XEXP (lhs, 1)) < HOST_BITS_PER_WIDE_INT) { - coeff0 = ((HOST_WIDE_INT) 1) << INTVAL (XEXP (lhs, 1)); + coeff0l = ((HOST_WIDE_INT) 1) << INTVAL (XEXP (lhs, 1)); + coeff0h = 0; lhs = XEXP (lhs, 0); } if (GET_CODE (rhs) == NEG) - coeff1 = -1, rhs = XEXP (rhs, 0); + { + coeff1l = -1; + coeff1h = -1; + rhs = XEXP (rhs, 0); + } else if (GET_CODE (rhs) == MULT && GET_CODE (XEXP (rhs, 1)) == CONST_INT) { - coeff1 = INTVAL (XEXP (rhs, 1)), rhs = XEXP (rhs, 0); + coeff1l = INTVAL (XEXP (rhs, 1)); + coeff1h = INTVAL (XEXP (rhs, 1)) < 0 ? -1 : 0; + rhs = XEXP (rhs, 0); } else if (GET_CODE (rhs) == ASHIFT && GET_CODE (XEXP (rhs, 1)) == CONST_INT && INTVAL (XEXP (rhs, 1)) >= 0 && INTVAL (XEXP (rhs, 1)) < HOST_BITS_PER_WIDE_INT) { - coeff1 = ((HOST_WIDE_INT) 1) << INTVAL (XEXP (rhs, 1)); + coeff1l = ((HOST_WIDE_INT) 1) << INTVAL (XEXP (rhs, 1)); + coeff1h = 0; rhs = XEXP (rhs, 0); } if (rtx_equal_p (lhs, rhs)) { rtx orig = gen_rtx_PLUS (mode, op0, op1); - tem = simplify_gen_binary (MULT, mode, lhs, - GEN_INT (coeff0 + coeff1)); + rtx coeff; + unsigned HOST_WIDE_INT l; + HOST_WIDE_INT h; + + add_double (coeff0l, coeff0h, coeff1l, coeff1h, &l, &h); + coeff = immed_double_const (l, h, mode); + + tem = simplify_gen_binary (MULT, mode, lhs, coeff); return rtx_cost (tem, SET) <= rtx_cost (orig, SET) ? tem : 0; } @@ -1405,46 +1428,67 @@ simplify_binary_operation_1 (enum rtx_co if (! FLOAT_MODE_P (mode)) { - HOST_WIDE_INT coeff0 = 1, coeff1 = 1; + HOST_WIDE_INT coeff0h = 0, negcoeff1h = -1; + unsigned HOST_WIDE_INT coeff0l = 1, negcoeff1l = -1; rtx lhs = op0, rhs = op1; if (GET_CODE (lhs) == NEG) - coeff0 = -1, lhs = XEXP (lhs, 0); + { + coeff0l = -1; + coeff0h = -1; + lhs = XEXP (lhs, 0); + } else if (GET_CODE (lhs) == MULT && GET_CODE (XEXP (lhs, 1)) == CONST_INT) { - coeff0 = INTVAL (XEXP (lhs, 1)), lhs = XEXP (lhs, 0); + coeff0l = INTVAL (XEXP (lhs, 1)); + coeff0h = INTVAL (XEXP (lhs, 1)) < 0 ? -1 : 0; + lhs = XEXP (lhs, 0); } else if (GET_CODE (lhs) == ASHIFT && GET_CODE (XEXP (lhs, 1)) == CONST_INT && INTVAL (XEXP (lhs, 1)) >= 0 && INTVAL (XEXP (lhs, 1)) < HOST_BITS_PER_WIDE_INT) { - coeff0 = ((HOST_WIDE_INT) 1) << INTVAL (XEXP (lhs, 1)); + coeff0l = ((HOST_WIDE_INT) 1) << INTVAL (XEXP (lhs, 1)); + coeff0h = 0; lhs = XEXP (lhs, 0); } if (GET_CODE (rhs) == NEG) - coeff1 = - 1, rhs = XEXP (rhs, 0); + { + negcoeff1l = 1; + negcoeff1h = 0; + rhs = XEXP (rhs, 0); + } else if (GET_CODE (rhs) == MULT && GET_CODE (XEXP (rhs, 1)) == CONST_INT) { - coeff1 = INTVAL (XEXP (rhs, 1)), rhs = XEXP (rhs, 0); + negcoeff1l = -INTVAL (XEXP (rhs, 1)); + negcoeff1h = INTVAL (XEXP (rhs, 1)) <= 0 ? 0 : -1; + rhs = XEXP (rhs, 0); } else if (GET_CODE (rhs) == ASHIFT && GET_CODE (XEXP (rhs, 1)) == CONST_INT && INTVAL (XEXP (rhs, 1)) >= 0 && INTVAL (XEXP (rhs, 1)) < HOST_BITS_PER_WIDE_INT) { - coeff1 = ((HOST_WIDE_INT) 1) << INTVAL (XEXP (rhs, 1)); + negcoeff1l = ((HOST_WIDE_INT) 1) << INTVAL (XEXP (rhs, 1)); + negcoeff1h = -1; rhs = XEXP (rhs, 0); } if (rtx_equal_p (lhs, rhs)) { rtx orig = gen_rtx_MINUS (mode, op0, op1); - tem = simplify_gen_binary (MULT, mode, lhs, - GEN_INT (coeff0 - coeff1)); + rtx coeff; + unsigned HOST_WIDE_INT l; + HOST_WIDE_INT h; + + add_double (coeff0l, coeff0h, negcoeff1l, negcoeff1h, &l, &h); + coeff = immed_double_const (l, h, mode); + + tem = simplify_gen_binary (MULT, mode, lhs, coeff); return rtx_cost (tem, SET) <= rtx_cost (orig, SET) ? tem : 0; } @@ -1531,6 +1575,16 @@ simplify_binary_operation_1 (enum rtx_co || val != HOST_BITS_PER_WIDE_INT - 1)) return simplify_gen_binary (ASHIFT, mode, op0, GEN_INT (val)); + /* Likewise for multipliers wider than a word. */ + else if (GET_CODE (trueop1) == CONST_DOUBLE + && (GET_MODE (trueop1) == VOIDmode + || GET_MODE_CLASS (GET_MODE (trueop1)) == MODE_INT) + && GET_MODE (op0) == mode + && CONST_DOUBLE_LOW (trueop1) == 0 + && (val = exact_log2 (CONST_DOUBLE_HIGH (trueop1))) >= 0) + return simplify_gen_binary (ASHIFT, mode, op0, + GEN_INT (val + HOST_BITS_PER_WIDE_INT)); + /* x*2 is x+x and x*(-1) is -x */ if (GET_CODE (trueop1) == CONST_DOUBLE && GET_MODE_CLASS (GET_MODE (trueop1)) == MODE_FLOAT Index: gcc/testsuite/ChangeLog from Alexandre Oliva <[EMAIL PROTECTED]> PR rtl-optimization/20532 * gcc.target/i386/badshift.c: New. Index: gcc/testsuite/gcc.target/i386/badshift.c =================================================================== RCS file: gcc/testsuite/gcc.target/i386/badshift.c diff -N gcc/testsuite/gcc.target/i386/badshift.c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ gcc/testsuite/gcc.target/i386/badshift.c 23 Mar 2005 02:30:29 -0000 @@ -0,0 +1,28 @@ +/* PR rtl-optimization/20532 */ + +/* { dg-do run } */ +/* { dg-options "-m32 -march=i386 -O1" } */ + +/* We used to optimize the DImode shift-by-32 to zero because in combine + we turned: + + (v << 31) * (v << 31) + + into: + + (v * (((HOST_WIDE_ITN)1 << 31) + ((HOST_WIDE_INT)1 << 31))) + + With a 32-bit HOST_WIDE_INT, the coefficient overflowed to zero. */ + +unsigned long long int badshift(unsigned long long int v) +{ + return v << 31 << 1; +} + +extern void abort (); + +int main() { + if (badshift (1) == 0) + abort (); + return 0; +} -- Alexandre Oliva http://www.ic.unicamp.br/~oliva/ Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org} Free Software Evangelist [EMAIL PROTECTED], gnu.org} -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20532