diff mbox

Fix {and,or}_comparisons_1 (PR tree-optimization/49073)

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

Commit Message

Jakub Jelinek May 20, 2011, 1:12 p.m. UTC 
Hi!

As discussed in the PR, these routines happily look through PHI nodes
into previous loop iteration.  f is initialized in the previous
loop iteration with f_10 = d_6 == 3; and in the current iteration
there is f_2 != 0 && d_6 == 4 test.  As we look through
# f_2 = PHI <0(2), f_10(6)> PHI, this is evaluated as
0 != 0 && d_6 == 4 (which is correctly false) and
(d_6 == 3) != 0 && d_6 == 4 (this one is false due to d_6 being
from one iteration in one case and next one in another case).
The following patch fixes it, bootstrapped/regtested on x86_64-linux and
i686-linux, ok for trunk/4.6?

2011-05-20  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/49073
	* gimple-fold.c (and_comparisons_1, or_comparisons_1): Return
	NULL if PHI argument is SSA_NAME, whose def_stmt is dominated
	by the PHI.
	* tree-ssa-ifcombine.c (tree_ssa_ifcombine): Calculate dominators.

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


	Jakub

Comments

Richard Biener May 20, 2011, 2:14 p.m. UTC | #1 
On Fri, May 20, 2011 at 3:12 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> As discussed in the PR, these routines happily look through PHI nodes
> into previous loop iteration.  f is initialized in the previous
> loop iteration with f_10 = d_6 == 3; and in the current iteration
> there is f_2 != 0 && d_6 == 4 test.  As we look through
> # f_2 = PHI <0(2), f_10(6)> PHI, this is evaluated as
> 0 != 0 && d_6 == 4 (which is correctly false) and
> (d_6 == 3) != 0 && d_6 == 4 (this one is false due to d_6 being
> from one iteration in one case and next one in another case).
> The following patch fixes it, bootstrapped/regtested on x86_64-linux and
> i686-linux, ok for trunk/4.6?

Ok.

Thanks,
Richard.

> 2011-05-20  Jakub Jelinek  <jakub@redhat.com>
>
>        PR tree-optimization/49073
>        * gimple-fold.c (and_comparisons_1, or_comparisons_1): Return
>        NULL if PHI argument is SSA_NAME, whose def_stmt is dominated
>        by the PHI.
>        * tree-ssa-ifcombine.c (tree_ssa_ifcombine): Calculate dominators.
>
>        * gcc.c-torture/execute/pr49073.c: New test.
>
> --- gcc/gimple-fold.c.jj        2011-05-11 19:39:04.000000000 +0200
> +++ gcc/gimple-fold.c   2011-05-20 11:05:11.000000000 +0200
> @@ -1,5 +1,5 @@
>  /* Statement simplification on GIMPLE.
> -   Copyright (C) 2010 Free Software Foundation, Inc.
> +   Copyright (C) 2010, 2011 Free Software Foundation, Inc.
>    Split out from tree-ssa-ccp.c.
>
>  This file is part of GCC.
> @@ -2278,8 +2278,19 @@ and_comparisons_1 (enum tree_code code1,
>                    }
>                  else if (TREE_CODE (arg) == SSA_NAME)
>                    {
> -                     tree temp = and_var_with_comparison (arg, invert,
> -                                                          code2, op2a, op2b);
> +                     tree temp;
> +                     gimple def_stmt = SSA_NAME_DEF_STMT (arg);
> +                     /* In simple cases we can look through PHI nodes,
> +                        but we have to be careful with loops.
> +                        See PR49073.  */
> +                     if (! dom_info_available_p (CDI_DOMINATORS)
> +                         || gimple_bb (def_stmt) == gimple_bb (stmt)
> +                         || dominated_by_p (CDI_DOMINATORS,
> +                                            gimple_bb (def_stmt),
> +                                            gimple_bb (stmt)))
> +                       return NULL_TREE;
> +                     temp = and_var_with_comparison (arg, invert, code2,
> +                                                     op2a, op2b);
>                      if (!temp)
>                        return NULL_TREE;
>                      else if (!result)
> @@ -2728,8 +2739,19 @@ or_comparisons_1 (enum tree_code code1,
>                    }
>                  else if (TREE_CODE (arg) == SSA_NAME)
>                    {
> -                     tree temp = or_var_with_comparison (arg, invert,
> -                                                         code2, op2a, op2b);
> +                     tree temp;
> +                     gimple def_stmt = SSA_NAME_DEF_STMT (arg);
> +                     /* In simple cases we can look through PHI nodes,
> +                        but we have to be careful with loops.
> +                        See PR49073.  */
> +                     if (! dom_info_available_p (CDI_DOMINATORS)
> +                         || gimple_bb (def_stmt) == gimple_bb (stmt)
> +                         || dominated_by_p (CDI_DOMINATORS,
> +                                            gimple_bb (def_stmt),
> +                                            gimple_bb (stmt)))
> +                       return NULL_TREE;
> +                     temp = or_var_with_comparison (arg, invert, code2,
> +                                                    op2a, op2b);
>                      if (!temp)
>                        return NULL_TREE;
>                      else if (!result)
> --- gcc/tree-ssa-ifcombine.c.jj 2011-05-20 08:14:08.000000000 +0200
> +++ gcc/tree-ssa-ifcombine.c    2011-05-20 11:37:24.000000000 +0200
> @@ -1,5 +1,5 @@
>  /* Combining of if-expressions on trees.
> -   Copyright (C) 2007, 2008, 2009, 2010 Free Software Foundation, Inc.
> +   Copyright (C) 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
>    Contributed by Richard Guenther <rguenther@suse.de>
>
>  This file is part of GCC.
> @@ -625,6 +625,7 @@ tree_ssa_ifcombine (void)
>   int i;
>
>   bbs = blocks_in_phiopt_order ();
> +  calculate_dominance_info (CDI_DOMINATORS);
>
>   for (i = 0; i < n_basic_blocks - NUM_FIXED_BLOCKS; ++i)
>     {
> --- gcc/testsuite/gcc.c-torture/execute/pr49073.c.jj    2011-05-20 11:38:58.000000000 +0200
> +++ gcc/testsuite/gcc.c-torture/execute/pr49073.c       2011-05-20 11:37:40.000000000 +0200
> @@ -0,0 +1,26 @@
> +/* PR tree-optimization/49073 */
> +
> +extern void abort (void);
> +int a[] = { 1, 2, 3, 4, 5, 6, 7 }, c;
> +
> +int
> +main ()
> +{
> +  int d = 1, i = 1;
> +  _Bool f = 0;
> +  do
> +    {
> +      d = a[i];
> +      if (f && d == 4)
> +       {
> +         ++c;
> +         break;
> +       }
> +      i++;
> +      f = (d == 3);
> +    }
> +  while (d < 7);
> +  if (c != 1)
> +    abort ();
> +  return 0;
> +}
>
>        Jakub
>
diff mbox

Patch

--- gcc/gimple-fold.c.jj	2011-05-11 19:39:04.000000000 +0200
+++ gcc/gimple-fold.c	2011-05-20 11:05:11.000000000 +0200
@@ -1,5 +1,5 @@ 
 /* Statement simplification on GIMPLE.
-   Copyright (C) 2010 Free Software Foundation, Inc.
+   Copyright (C) 2010, 2011 Free Software Foundation, Inc.
    Split out from tree-ssa-ccp.c.
 
 This file is part of GCC.
@@ -2278,8 +2278,19 @@  and_comparisons_1 (enum tree_code code1,
 		    }
 		  else if (TREE_CODE (arg) == SSA_NAME)
 		    {
-		      tree temp = and_var_with_comparison (arg, invert,
-							   code2, op2a, op2b);
+		      tree temp;
+		      gimple def_stmt = SSA_NAME_DEF_STMT (arg);
+		      /* In simple cases we can look through PHI nodes,
+			 but we have to be careful with loops.
+			 See PR49073.  */
+		      if (! dom_info_available_p (CDI_DOMINATORS)
+			  || gimple_bb (def_stmt) == gimple_bb (stmt)
+			  || dominated_by_p (CDI_DOMINATORS,
+					     gimple_bb (def_stmt),
+					     gimple_bb (stmt)))
+			return NULL_TREE;
+		      temp = and_var_with_comparison (arg, invert, code2,
+						      op2a, op2b);
 		      if (!temp)
 			return NULL_TREE;
 		      else if (!result)
@@ -2728,8 +2739,19 @@  or_comparisons_1 (enum tree_code code1, 
 		    }
 		  else if (TREE_CODE (arg) == SSA_NAME)
 		    {
-		      tree temp = or_var_with_comparison (arg, invert,
-							  code2, op2a, op2b);
+		      tree temp;
+		      gimple def_stmt = SSA_NAME_DEF_STMT (arg);
+		      /* In simple cases we can look through PHI nodes,
+			 but we have to be careful with loops.
+			 See PR49073.  */
+		      if (! dom_info_available_p (CDI_DOMINATORS)
+			  || gimple_bb (def_stmt) == gimple_bb (stmt)
+			  || dominated_by_p (CDI_DOMINATORS,
+					     gimple_bb (def_stmt),
+					     gimple_bb (stmt)))
+			return NULL_TREE;
+		      temp = or_var_with_comparison (arg, invert, code2,
+						     op2a, op2b);
 		      if (!temp)
 			return NULL_TREE;
 		      else if (!result)
--- gcc/tree-ssa-ifcombine.c.jj	2011-05-20 08:14:08.000000000 +0200
+++ gcc/tree-ssa-ifcombine.c	2011-05-20 11:37:24.000000000 +0200
@@ -1,5 +1,5 @@ 
 /* Combining of if-expressions on trees.
-   Copyright (C) 2007, 2008, 2009, 2010 Free Software Foundation, Inc.
+   Copyright (C) 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
    Contributed by Richard Guenther <rguenther@suse.de>
 
 This file is part of GCC.
@@ -625,6 +625,7 @@  tree_ssa_ifcombine (void)
   int i;
 
   bbs = blocks_in_phiopt_order ();
+  calculate_dominance_info (CDI_DOMINATORS);
 
   for (i = 0; i < n_basic_blocks - NUM_FIXED_BLOCKS; ++i)
     {
--- gcc/testsuite/gcc.c-torture/execute/pr49073.c.jj	2011-05-20 11:38:58.000000000 +0200
+++ gcc/testsuite/gcc.c-torture/execute/pr49073.c	2011-05-20 11:37:40.000000000 +0200
@@ -0,0 +1,26 @@ 
+/* PR tree-optimization/49073 */
+
+extern void abort (void);
+int a[] = { 1, 2, 3, 4, 5, 6, 7 }, c;
+
+int
+main ()
+{
+  int d = 1, i = 1;
+  _Bool f = 0;
+  do
+    {
+      d = a[i];
+      if (f && d == 4)
+	{
+	  ++c;
+	  break;
+	}
+      i++;
+      f = (d == 3);
+    }
+  while (d < 7);
+  if (c != 1)
+    abort ();
+  return 0;
+}