diff mbox

Fix (X & C1) | C2 folding (PR tree-optimization/52286)

Message ID 20120217135006.GM18768@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Feb. 17, 2012, 1:50 p.m. UTC
Hi!

The following testcase is miscompiled (by CCP, or, if -fno-tree-ccp,
by VRP), because the (X & C1) | C2 folding changes the C1 constant
into an integer constant with most significant bit set, but not
sign-extended into the double_int.  Fixed by using double_int_to_tree
which extends it properly, instead of build_int_cst_wide.
Instead of creating a double_int from the hi3/lo3 pair I've changed
the code to actually work on double_ints.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2012-02-17  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/52286
	* fold-const.c (fold_binary_loc): For (X & C1) | C2
	optimization use double_int_to_tree instead of build_int_cst_wide,
	rewrite to use double_int vars.

	* gcc.c-torture/execute/pr52286.c: New test.


	Jakub

Comments

Richard Biener Feb. 20, 2012, 10 a.m. UTC | #1
On Fri, Feb 17, 2012 at 2:50 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> The following testcase is miscompiled (by CCP, or, if -fno-tree-ccp,
> by VRP), because the (X & C1) | C2 folding changes the C1 constant
> into an integer constant with most significant bit set, but not
> sign-extended into the double_int.  Fixed by using double_int_to_tree
> which extends it properly, instead of build_int_cst_wide.
> Instead of creating a double_int from the hi3/lo3 pair I've changed
> the code to actually work on double_ints.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2012-02-17  Jakub Jelinek  <jakub@redhat.com>
>
>        PR tree-optimization/52286
>        * fold-const.c (fold_binary_loc): For (X & C1) | C2
>        optimization use double_int_to_tree instead of build_int_cst_wide,
>        rewrite to use double_int vars.
>
>        * gcc.c-torture/execute/pr52286.c: New test.
>
> --- gcc/fold-const.c.jj 2012-02-16 20:04:34.000000000 +0100
> +++ gcc/fold-const.c    2012-02-17 12:05:46.559700551 +0100
> @@ -10959,66 +10959,50 @@ fold_binary_loc (location_t loc,
>          && TREE_CODE (arg1) == INTEGER_CST
>          && TREE_CODE (TREE_OPERAND (arg0, 1)) == INTEGER_CST)
>        {
> -         unsigned HOST_WIDE_INT hi1, lo1, hi2, lo2, hi3, lo3, mlo, mhi;
> +         double_int c1, c2, c3, msk;
>          int width = TYPE_PRECISION (type), w;
> -         hi1 = TREE_INT_CST_HIGH (TREE_OPERAND (arg0, 1));
> -         lo1 = TREE_INT_CST_LOW (TREE_OPERAND (arg0, 1));
> -         hi2 = TREE_INT_CST_HIGH (arg1);
> -         lo2 = TREE_INT_CST_LOW (arg1);
> +         c1 = tree_to_double_int (TREE_OPERAND (arg0, 1));
> +         c2 = tree_to_double_int (arg1);
>
>          /* If (C1&C2) == C1, then (X&C1)|C2 becomes (X,C2).  */
> -         if ((hi1 & hi2) == hi1 && (lo1 & lo2) == lo1)
> +         if (double_int_equal_p (double_int_and (c1, c2), c1))
>            return omit_one_operand_loc (loc, type, arg1,
> -                                    TREE_OPERAND (arg0, 0));
> +                                        TREE_OPERAND (arg0, 0));
>
> -         if (width > HOST_BITS_PER_WIDE_INT)
> -           {
> -             mhi = (unsigned HOST_WIDE_INT) -1
> -                   >> (2 * HOST_BITS_PER_WIDE_INT - width);
> -             mlo = -1;
> -           }
> -         else
> -           {
> -             mhi = 0;
> -             mlo = (unsigned HOST_WIDE_INT) -1
> -                   >> (HOST_BITS_PER_WIDE_INT - width);
> -           }
> +         msk = double_int_mask (width);
>
>          /* If (C1|C2) == ~0 then (X&C1)|C2 becomes X|C2.  */
> -         if ((~(hi1 | hi2) & mhi) == 0 && (~(lo1 | lo2) & mlo) == 0)
> +         if (double_int_zero_p (double_int_and_not (msk,
> +                                                    double_int_ior (c1, c2))))
>            return fold_build2_loc (loc, BIT_IOR_EXPR, type,
> -                               TREE_OPERAND (arg0, 0), arg1);
> +                                   TREE_OPERAND (arg0, 0), arg1);
>
>          /* Minimize the number of bits set in C1, i.e. C1 := C1 & ~C2,
>             unless (C1 & ~C2) | (C2 & C3) for some C3 is a mask of some
>             mode which allows further optimizations.  */
> -         hi1 &= mhi;
> -         lo1 &= mlo;
> -         hi2 &= mhi;
> -         lo2 &= mlo;
> -         hi3 = hi1 & ~hi2;
> -         lo3 = lo1 & ~lo2;
> +         c1 = double_int_and (c1, msk);
> +         c2 = double_int_and (c2, msk);
> +         c3 = double_int_and_not (c1, c2);
>          for (w = BITS_PER_UNIT;
>               w <= width && w <= HOST_BITS_PER_WIDE_INT;
>               w <<= 1)
>            {
>              unsigned HOST_WIDE_INT mask
>                = (unsigned HOST_WIDE_INT) -1 >> (HOST_BITS_PER_WIDE_INT - w);
> -             if (((lo1 | lo2) & mask) == mask
> -                 && (lo1 & ~mask) == 0 && hi1 == 0)
> +             if (((c1.low | c2.low) & mask) == mask
> +                 && (c1.low & ~mask) == 0 && c1.high == 0)
>                {
> -                 hi3 = 0;
> -                 lo3 = mask;
> +                 c3 = uhwi_to_double_int (mask);
>                  break;
>                }
>            }
> -         if (hi3 != hi1 || lo3 != lo1)
> +         if (!double_int_equal_p (c3, c1))
>            return fold_build2_loc (loc, BIT_IOR_EXPR, type,
> -                               fold_build2_loc (loc, BIT_AND_EXPR, type,
> -                                            TREE_OPERAND (arg0, 0),
> -                                            build_int_cst_wide (type,
> -                                                                lo3, hi3)),
> -                               arg1);
> +                                   fold_build2_loc (loc, BIT_AND_EXPR, type,
> +                                                    TREE_OPERAND (arg0, 0),
> +                                                    double_int_to_tree (type,
> +                                                                        c3)),
> +                                   arg1);
>        }
>
>       /* (X & Y) | Y is (X, Y).  */
> --- gcc/testsuite/gcc.c-torture/execute/pr52286.c.jj    2012-02-17 12:09:41.131621700 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/pr52286.c       2012-02-17 12:09:22.000000000 +0100
> @@ -0,0 +1,14 @@
> +/* PR tree-optimization/52286 */
> +
> +extern void abort (void);
> +
> +int
> +main ()
> +{
> +  int a, b;
> +  asm ("" : "=r" (a) : "0" (0));
> +  b = (~a | 1) & -2038094497;
> +  if (b >= 0)
> +    abort ();
> +  return 0;
> +}
>
>        Jakub
diff mbox

Patch

--- gcc/fold-const.c.jj	2012-02-16 20:04:34.000000000 +0100
+++ gcc/fold-const.c	2012-02-17 12:05:46.559700551 +0100
@@ -10959,66 +10959,50 @@  fold_binary_loc (location_t loc,
 	  && TREE_CODE (arg1) == INTEGER_CST
 	  && TREE_CODE (TREE_OPERAND (arg0, 1)) == INTEGER_CST)
 	{
-	  unsigned HOST_WIDE_INT hi1, lo1, hi2, lo2, hi3, lo3, mlo, mhi;
+	  double_int c1, c2, c3, msk;
 	  int width = TYPE_PRECISION (type), w;
-	  hi1 = TREE_INT_CST_HIGH (TREE_OPERAND (arg0, 1));
-	  lo1 = TREE_INT_CST_LOW (TREE_OPERAND (arg0, 1));
-	  hi2 = TREE_INT_CST_HIGH (arg1);
-	  lo2 = TREE_INT_CST_LOW (arg1);
+	  c1 = tree_to_double_int (TREE_OPERAND (arg0, 1));
+	  c2 = tree_to_double_int (arg1);
 
 	  /* If (C1&C2) == C1, then (X&C1)|C2 becomes (X,C2).  */
-	  if ((hi1 & hi2) == hi1 && (lo1 & lo2) == lo1)
+	  if (double_int_equal_p (double_int_and (c1, c2), c1))
 	    return omit_one_operand_loc (loc, type, arg1,
-				     TREE_OPERAND (arg0, 0));
+					 TREE_OPERAND (arg0, 0));
 
-	  if (width > HOST_BITS_PER_WIDE_INT)
-	    {
-	      mhi = (unsigned HOST_WIDE_INT) -1
-		    >> (2 * HOST_BITS_PER_WIDE_INT - width);
-	      mlo = -1;
-	    }
-	  else
-	    {
-	      mhi = 0;
-	      mlo = (unsigned HOST_WIDE_INT) -1
-		    >> (HOST_BITS_PER_WIDE_INT - width);
-	    }
+	  msk = double_int_mask (width);
 
 	  /* If (C1|C2) == ~0 then (X&C1)|C2 becomes X|C2.  */
-	  if ((~(hi1 | hi2) & mhi) == 0 && (~(lo1 | lo2) & mlo) == 0)
+	  if (double_int_zero_p (double_int_and_not (msk,
+						     double_int_ior (c1, c2))))
 	    return fold_build2_loc (loc, BIT_IOR_EXPR, type,
-				TREE_OPERAND (arg0, 0), arg1);
+				    TREE_OPERAND (arg0, 0), arg1);
 
 	  /* Minimize the number of bits set in C1, i.e. C1 := C1 & ~C2,
 	     unless (C1 & ~C2) | (C2 & C3) for some C3 is a mask of some
 	     mode which allows further optimizations.  */
-	  hi1 &= mhi;
-	  lo1 &= mlo;
-	  hi2 &= mhi;
-	  lo2 &= mlo;
-	  hi3 = hi1 & ~hi2;
-	  lo3 = lo1 & ~lo2;
+	  c1 = double_int_and (c1, msk);
+	  c2 = double_int_and (c2, msk);
+	  c3 = double_int_and_not (c1, c2);
 	  for (w = BITS_PER_UNIT;
 	       w <= width && w <= HOST_BITS_PER_WIDE_INT;
 	       w <<= 1)
 	    {
 	      unsigned HOST_WIDE_INT mask
 		= (unsigned HOST_WIDE_INT) -1 >> (HOST_BITS_PER_WIDE_INT - w);
-	      if (((lo1 | lo2) & mask) == mask
-		  && (lo1 & ~mask) == 0 && hi1 == 0)
+	      if (((c1.low | c2.low) & mask) == mask
+		  && (c1.low & ~mask) == 0 && c1.high == 0)
 		{
-		  hi3 = 0;
-		  lo3 = mask;
+		  c3 = uhwi_to_double_int (mask);
 		  break;
 		}
 	    }
-	  if (hi3 != hi1 || lo3 != lo1)
+	  if (!double_int_equal_p (c3, c1))
 	    return fold_build2_loc (loc, BIT_IOR_EXPR, type,
-				fold_build2_loc (loc, BIT_AND_EXPR, type,
-					     TREE_OPERAND (arg0, 0),
-					     build_int_cst_wide (type,
-								 lo3, hi3)),
-				arg1);
+				    fold_build2_loc (loc, BIT_AND_EXPR, type,
+						     TREE_OPERAND (arg0, 0),
+						     double_int_to_tree (type,
+									 c3)),
+				    arg1);
 	}
 
       /* (X & Y) | Y is (X, Y).  */
--- gcc/testsuite/gcc.c-torture/execute/pr52286.c.jj	2012-02-17 12:09:41.131621700 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr52286.c	2012-02-17 12:09:22.000000000 +0100
@@ -0,0 +1,14 @@ 
+/* PR tree-optimization/52286 */
+
+extern void abort (void);
+
+int
+main ()
+{
+  int a, b;
+  asm ("" : "=r" (a) : "0" (0));
+  b = (~a | 1) & -2038094497;
+  if (b >= 0)
+    abort ();
+  return 0;
+}