------- 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

Reply via email to