Patchwork Fix up operand_equal_p (PR tree-optimization/56448)

login
register
mail settings
Submitter Jakub Jelinek
Date Feb. 26, 2013, 6:46 a.m.
Message ID <20130226064627.GY1215@tucnak.zalov.cz>
Download mbox | patch
Permalink /patch/223118/
State New
Headers show

Comments

Jakub Jelinek - Feb. 26, 2013, 6:46 a.m.
Hi!

On the following testcase, VN during free keeps iterating forever, since
the unshare_expr call I've added to fix PR55935.  The bug is that
when we have
volatile int a[1];
and &a[0] expression, if we unshare_expr that expression, it doesn't compare
operand_equal_p (exp, unshare_expr (exp), OEP_PURE_SAME).  While when
looking at ADDR_EXPR we set OEP_CONSTANT_ADDRESS_OF flag, when
looking at the ARRAY_REF we see it has TREE_SIDE_EFFECTS set and return 0.
For ADDR_EXPR of ARRAY_REF or other reference it really doesn't matter
whether the ARRAY_REF itself has side-effects or not though, so we should
ignore the side-effects.

Fixed thusly, bootstrapped/regtested on x86_64-linux, ok for trunk?

2013-02-26  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/56448
	* fold-const.c (operand_equal_p) <case tcc_reference>: Don't look at
	TREE_SIDE_EFFECTS if flags contain OEP_CONSTANT_ADDRESS_OF.
	Clear OEP_CONSTANT_ADDRESS_OF from flags before recursing on second or
	later operands of the references, or even first operand for
	INDIRECT_REF, TARGET_MEM_REF or MEM_REF.

	* gcc.c-torture/compile/pr56448.c: New test.


	Jakub
Richard Guenther - Feb. 26, 2013, 9:47 a.m.
On Tue, Feb 26, 2013 at 7:46 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> On the following testcase, VN during free keeps iterating forever, since
> the unshare_expr call I've added to fix PR55935.  The bug is that
> when we have
> volatile int a[1];
> and &a[0] expression, if we unshare_expr that expression, it doesn't compare
> operand_equal_p (exp, unshare_expr (exp), OEP_PURE_SAME).  While when
> looking at ADDR_EXPR we set OEP_CONSTANT_ADDRESS_OF flag, when
> looking at the ARRAY_REF we see it has TREE_SIDE_EFFECTS set and return 0.
> For ADDR_EXPR of ARRAY_REF or other reference it really doesn't matter
> whether the ARRAY_REF itself has side-effects or not though, so we should
> ignore the side-effects.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2013-02-26  Jakub Jelinek  <jakub@redhat.com>
>
>         PR tree-optimization/56448
>         * fold-const.c (operand_equal_p) <case tcc_reference>: Don't look at
>         TREE_SIDE_EFFECTS if flags contain OEP_CONSTANT_ADDRESS_OF.
>         Clear OEP_CONSTANT_ADDRESS_OF from flags before recursing on second or
>         later operands of the references, or even first operand for
>         INDIRECT_REF, TARGET_MEM_REF or MEM_REF.
>
>         * gcc.c-torture/compile/pr56448.c: New test.
>
> --- gcc/fold-const.c.jj 2013-02-08 16:05:34.000000000 +0100
> +++ gcc/fold-const.c    2013-02-25 23:25:46.676092462 +0100
> @@ -2542,19 +2542,25 @@ operand_equal_p (const_tree arg0, const_
>
>      case tcc_reference:
>        /* If either of the pointer (or reference) expressions we are
> -        dereferencing contain a side effect, these cannot be equal.  */
> -      if (TREE_SIDE_EFFECTS (arg0)
> -         || TREE_SIDE_EFFECTS (arg1))
> +        dereferencing contain a side effect, these cannot be equal,
> +        but their addresses can be.  */
> +      if ((flags & OEP_CONSTANT_ADDRESS_OF) == 0
> +         && (TREE_SIDE_EFFECTS (arg0)
> +             || TREE_SIDE_EFFECTS (arg1)))
>         return 0;
>
>        switch (TREE_CODE (arg0))
>         {
>         case INDIRECT_REF:
> +         flags &= ~OEP_CONSTANT_ADDRESS_OF;
> +         return OP_SAME (0);
> +
>         case REALPART_EXPR:
>         case IMAGPART_EXPR:
>           return OP_SAME (0);
>
>         case TARGET_MEM_REF:
> +         flags &= ~OEP_CONSTANT_ADDRESS_OF;
>           /* Require equal extra operands and then fall through to MEM_REF
>              handling of the two common operands.  */
>           if (!OP_SAME_WITH_NULL (2)
> @@ -2563,6 +2569,7 @@ operand_equal_p (const_tree arg0, const_
>             return 0;
>           /* Fallthru.  */
>         case MEM_REF:
> +         flags &= ~OEP_CONSTANT_ADDRESS_OF;
>           /* Require equal access sizes, and similar pointer types.
>              We can have incomplete types for array references of
>              variable-sized arrays from the Fortran frontent
> @@ -2581,22 +2588,28 @@ operand_equal_p (const_tree arg0, const_
>           /* Operands 2 and 3 may be null.
>              Compare the array index by value if it is constant first as we
>              may have different types but same value here.  */
> -         return (OP_SAME (0)
> -                 && (tree_int_cst_equal (TREE_OPERAND (arg0, 1),
> -                                         TREE_OPERAND (arg1, 1))
> -                     || OP_SAME (1))
> +         if (!OP_SAME (0))
> +           return 0;
> +         flags &= ~OEP_CONSTANT_ADDRESS_OF;
> +         return ((tree_int_cst_equal (TREE_OPERAND (arg0, 1),
> +                                      TREE_OPERAND (arg1, 1))
> +                  || OP_SAME (1))
>                   && OP_SAME_WITH_NULL (2)
>                   && OP_SAME_WITH_NULL (3));
>
>         case COMPONENT_REF:
>           /* Handle operand 2 the same as for ARRAY_REF.  Operand 0
>              may be NULL when we're called to compare MEM_EXPRs.  */
> -         return OP_SAME_WITH_NULL (0)
> -                && OP_SAME (1)
> -                && OP_SAME_WITH_NULL (2);
> +         if (!OP_SAME_WITH_NULL (0))
> +           return 0;
> +         flags &= ~OEP_CONSTANT_ADDRESS_OF;
> +         return OP_SAME (1) && OP_SAME_WITH_NULL (2);
>
>         case BIT_FIELD_REF:
> -         return OP_SAME (0) && OP_SAME (1) && OP_SAME (2);
> +         if (!OP_SAME (0))
> +           return 0;
> +         flags &= ~OEP_CONSTANT_ADDRESS_OF;
> +         return OP_SAME (1) && OP_SAME (2);
>
>         default:
>           return 0;
> --- gcc/testsuite/gcc.c-torture/compile/pr56448.c.jj    2013-02-25 23:31:55.266922056 +0100
> +++ gcc/testsuite/gcc.c-torture/compile/pr56448.c       2013-02-25 23:29:55.000000000 +0100
> @@ -0,0 +1,14 @@
> +/* PR tree-optimization/56448 */
> +
> +volatile int a[1];
> +int b;
> +
> +void
> +foo ()
> +{
> +  for (;;)
> +    {
> +      int *c[3][6] = { 0, 0, 0, &b, 0, 0, 0, 0, &b, 0, 0, 0, 0, 0, 0, 0, &b, (int *) &a[0] };
> +      b = *c[2][5];
> +    }
> +}
>
>         Jakub

Patch

--- gcc/fold-const.c.jj	2013-02-08 16:05:34.000000000 +0100
+++ gcc/fold-const.c	2013-02-25 23:25:46.676092462 +0100
@@ -2542,19 +2542,25 @@  operand_equal_p (const_tree arg0, const_
 
     case tcc_reference:
       /* If either of the pointer (or reference) expressions we are
-	 dereferencing contain a side effect, these cannot be equal.  */
-      if (TREE_SIDE_EFFECTS (arg0)
-	  || TREE_SIDE_EFFECTS (arg1))
+	 dereferencing contain a side effect, these cannot be equal,
+	 but their addresses can be.  */
+      if ((flags & OEP_CONSTANT_ADDRESS_OF) == 0
+	  && (TREE_SIDE_EFFECTS (arg0)
+	      || TREE_SIDE_EFFECTS (arg1)))
 	return 0;
 
       switch (TREE_CODE (arg0))
 	{
 	case INDIRECT_REF:
+	  flags &= ~OEP_CONSTANT_ADDRESS_OF;
+	  return OP_SAME (0);
+
 	case REALPART_EXPR:
 	case IMAGPART_EXPR:
 	  return OP_SAME (0);
 
 	case TARGET_MEM_REF:
+	  flags &= ~OEP_CONSTANT_ADDRESS_OF;
 	  /* Require equal extra operands and then fall through to MEM_REF
 	     handling of the two common operands.  */
 	  if (!OP_SAME_WITH_NULL (2)
@@ -2563,6 +2569,7 @@  operand_equal_p (const_tree arg0, const_
 	    return 0;
 	  /* Fallthru.  */
 	case MEM_REF:
+	  flags &= ~OEP_CONSTANT_ADDRESS_OF;
 	  /* Require equal access sizes, and similar pointer types.
 	     We can have incomplete types for array references of
 	     variable-sized arrays from the Fortran frontent
@@ -2581,22 +2588,28 @@  operand_equal_p (const_tree arg0, const_
 	  /* Operands 2 and 3 may be null.
 	     Compare the array index by value if it is constant first as we
 	     may have different types but same value here.  */
-	  return (OP_SAME (0)
-		  && (tree_int_cst_equal (TREE_OPERAND (arg0, 1),
-					  TREE_OPERAND (arg1, 1))
-		      || OP_SAME (1))
+	  if (!OP_SAME (0))
+	    return 0;
+	  flags &= ~OEP_CONSTANT_ADDRESS_OF;
+	  return ((tree_int_cst_equal (TREE_OPERAND (arg0, 1),
+				       TREE_OPERAND (arg1, 1))
+		   || OP_SAME (1))
 		  && OP_SAME_WITH_NULL (2)
 		  && OP_SAME_WITH_NULL (3));
 
 	case COMPONENT_REF:
 	  /* Handle operand 2 the same as for ARRAY_REF.  Operand 0
 	     may be NULL when we're called to compare MEM_EXPRs.  */
-	  return OP_SAME_WITH_NULL (0)
-		 && OP_SAME (1)
-		 && OP_SAME_WITH_NULL (2);
+	  if (!OP_SAME_WITH_NULL (0))
+	    return 0;
+	  flags &= ~OEP_CONSTANT_ADDRESS_OF;
+	  return OP_SAME (1) && OP_SAME_WITH_NULL (2);
 
 	case BIT_FIELD_REF:
-	  return OP_SAME (0) && OP_SAME (1) && OP_SAME (2);
+	  if (!OP_SAME (0))
+	    return 0;
+	  flags &= ~OEP_CONSTANT_ADDRESS_OF;
+	  return OP_SAME (1) && OP_SAME (2);
 
 	default:
 	  return 0;
--- gcc/testsuite/gcc.c-torture/compile/pr56448.c.jj	2013-02-25 23:31:55.266922056 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr56448.c	2013-02-25 23:29:55.000000000 +0100
@@ -0,0 +1,14 @@ 
+/* PR tree-optimization/56448 */
+
+volatile int a[1];
+int b;
+
+void
+foo ()
+{
+  for (;;)
+    {
+      int *c[3][6] = { 0, 0, 0, &b, 0, 0, 0, 0, &b, 0, 0, 0, 0, 0, 0, 0, &b, (int *) &a[0] };
+      b = *c[2][5];
+    }
+}