Fix PR46036: if-conversion problem

Fix PR46036: if-conversion problem

[Sebastian Pop]

[-- Attachment #1: Type: text/plain, Size: 104 bytes --]

Hi,

Here is a fix for PR46036 bootstrapped and tested on amd64-linux.
Ok for trunk?

Thanks,
Sebastian

[-- Attachment #2: 0001-Fix-PR46036.patch --]
[-- Type: text/x-patch, Size: 2611 bytes --]

From ee9d3095a33df9aa6956c9a7d8782d9bb3fad85e Mon Sep 17 00:00:00 2001
From: Sebastian Pop <sebpop@gmail.com>
Date: Fri, 5 Nov 2010 16:57:00 -0500
Subject: [PATCH] Fix PR46036.

2010-11-04  Sebastian Pop  <sebastian.pop@amd.com>

	PR tree-optimization/46036
	* tree-if-conv.c (predicate_bbs): Call unshare_expr before
	add_to_dst_predicate_list.

	* gfortran.dg/lto/pr46036_0.f90: New.
---
 gcc/ChangeLog                               |    6 ++++++
 gcc/testsuite/ChangeLog                     |    5 +++++
 gcc/testsuite/gfortran.dg/lto/pr46036_0.f90 |   14 ++++++++++++++
 gcc/tree-if-conv.c                          |    2 +-
 4 files changed, 26 insertions(+), 1 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/lto/pr46036_0.f90

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index b0618ac..d0b178a 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2010-11-04  Sebastian Pop  <sebastian.pop@amd.com>
+
+	PR tree-optimization/46036
+	* tree-if-conv.c (predicate_bbs): Call unshare_expr before
+	add_to_dst_predicate_list.
+
 2010-11-03  Uros Bizjak  <ubizjak@gmail.com>
 
 	PR middle-end/46292
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 89a0535..f296193 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2010-11-04  Sebastian Pop  <sebastian.pop@amd.com>
+
+	PR tree-optimization/46036
+	* gfortran.dg/lto/pr46036_0.f90: New.
+
 2010-11-04  Jason Merrill  <jason@redhat.com>
 
 	* g++.dg/cpp0x/constexpr-access.C: New.
diff --git a/gcc/testsuite/gfortran.dg/lto/pr46036_0.f90 b/gcc/testsuite/gfortran.dg/lto/pr46036_0.f90
new file mode 100644
index 0000000..558c7ed
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/lto/pr46036_0.f90
@@ -0,0 +1,14 @@
+! { dg-lto-do link }
+! { dg-lto-options {{ -O -flto -ftree-vectorize }} }
+
+function no_of_edges(self) result(res)
+  integer(kind=kind(1)) :: edge_bit_string
+  integer(kind=kind(1)) :: res
+  integer(kind=kind(1)) :: e
+  do e = 0, 11
+     if (.not. btest(edge_bit_string,e)) cycle
+     res = res + 1
+  end do
+end function no_of_edges
+
+end program
diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c
index 2c334d9..5ee0a45 100644
--- a/gcc/tree-if-conv.c
+++ b/gcc/tree-if-conv.c
@@ -942,7 +942,7 @@ predicate_bbs (loop_p loop)
 						     &true_edge, &false_edge);
 
 		/* If C is true, then TRUE_EDGE is taken.  */
-		add_to_dst_predicate_list (loop, true_edge, cond, c);
+		add_to_dst_predicate_list (loop, true_edge, cond, unshare_expr (c));
 
 		/* If C is false, then FALSE_EDGE is taken.  */
 		c2 = invert_truthvalue_loc (loc, unshare_expr (c));
-- 
1.7.0.4

Re: Fix PR46036: if-conversion problem

[Sebastian Pop]

[-- Attachment #1: Type: text/plain, Size: 507 bytes --]

Hi,

While working on PR46036, I remarked that there are some OR
expressions that do not fold correctly into "true".  The attached
patch does improve the situation a bit, and there is still a missing
case a bit more difficult to handle where parse_predicate returns
two SSA_NAMEs, then we would need to fold "a || b".

The attached patch passed bootstrap and test on amd64-linux.
Would this kind of patch be ok for trunk?  Should I send out a
patch addressing the remaining missing case?

Thanks,
Sebastian

[-- Attachment #2: 0001-improve-parse_predicate.patch --]
[-- Type: text/x-patch, Size: 3938 bytes --]

From 554bc66748971f79e81dfb89536db7fd4b80d452 Mon Sep 17 00:00:00 2001
From: Sebastian Pop <sebpop@gmail.com>
Date: Thu, 4 Nov 2010 17:13:09 -0500
Subject: [PATCH 1/2] improve parse_predicate

---
 gcc/gimple-fold.c  |    5 +----
 gcc/gimple.h       |    3 ++-
 gcc/tree-if-conv.c |   28 +++++++++++++++++++++-------
 3 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index e561a63..63725c3 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -1864,9 +1864,6 @@ static tree
 or_comparisons_1 (enum tree_code code1, tree op1a, tree op1b,
 		  enum tree_code code2, tree op2a, tree op2b);
 static tree
-or_var_with_comparison (tree var, bool invert,
-			enum tree_code code2, tree op2a, tree op2b);
-static tree
 or_var_with_comparison_1 (gimple stmt, 
 			  enum tree_code code2, tree op2a, tree op2b);
 
@@ -2323,7 +2320,7 @@ maybe_fold_and_comparisons (enum tree_code code1, tree op1a, tree op1b,
    If INVERT is true, invert the value of VAR before doing the OR.
    Return NULL_EXPR if we can't simplify this to a single expression.  */
 
-static tree
+tree
 or_var_with_comparison (tree var, bool invert,
 			enum tree_code code2, tree op2a, tree op2b)
 {
diff --git a/gcc/gimple.h b/gcc/gimple.h
index 150b836..623cd51 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -4879,9 +4879,10 @@ tree maybe_fold_stmt_addition (location_t, tree, tree, tree);
 tree get_symbol_constant_value (tree);
 tree canonicalize_constructor_val (tree);
 bool may_propagate_address_into_dereference (tree, tree);
-extern tree maybe_fold_and_comparisons (enum tree_code, tree, tree, 
+extern tree maybe_fold_and_comparisons (enum tree_code, tree, tree,
 					enum tree_code, tree, tree);
 extern tree maybe_fold_or_comparisons (enum tree_code, tree, tree,
 				       enum tree_code, tree, tree);
+extern tree or_var_with_comparison (tree, bool, enum tree_code, tree, tree);
 
 #endif  /* GCC_GIMPLE_H */
diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c
index 642dbda..2c334d9 100644
--- a/gcc/tree-if-conv.c
+++ b/gcc/tree-if-conv.c
@@ -273,14 +273,16 @@ parse_predicate (tree cond, tree *op0, tree *op1)
   if (TREE_CODE (cond) == SSA_NAME
       && is_gimple_assign (s = SSA_NAME_DEF_STMT (cond)))
     {
-      if (TREE_CODE_CLASS (gimple_assign_rhs_code (s)) == tcc_comparison)
+      enum tree_code outer_code = gimple_assign_rhs_code (s);
+
+      if (TREE_CODE_CLASS (outer_code) == tcc_comparison)
 	{
 	  *op0 = gimple_assign_rhs1 (s);
 	  *op1 = gimple_assign_rhs2 (s);
-	  return gimple_assign_rhs_code (s);
+	  return outer_code;
 	}
 
-      else if (gimple_assign_rhs_code (s) == TRUTH_NOT_EXPR)
+      if (outer_code == TRUTH_NOT_EXPR)
 	{
 	  tree op = gimple_assign_rhs1 (s);
 	  tree type = TREE_TYPE (op);
@@ -290,6 +292,9 @@ parse_predicate (tree cond, tree *op0, tree *op1)
 	    : invert_tree_comparison (code, HONOR_NANS (TYPE_MODE (type)));
 	}
 
+      if (gimple_assign_cast_p (s))
+	return parse_predicate (gimple_assign_rhs1 (s), op0, op1);
+
       return ERROR_MARK;
     }
 
@@ -308,15 +313,24 @@ parse_predicate (tree cond, tree *op0, tree *op1)
 static tree
 fold_or_predicates (location_t loc, tree c1, tree c2)
 {
-  tree op1a, op1b, op2a, op2b;
+  tree op1a, op1b, op2a, op2b, t;
   enum tree_code code1 = parse_predicate (c1, &op1a, &op1b);
   enum tree_code code2 = parse_predicate (c2, &op2a, &op2b);
 
   if (code1 != ERROR_MARK && code2 != ERROR_MARK)
     {
-      tree t = maybe_fold_or_comparisons (code1, op1a, op1b,
-					  code2, op2a, op2b);
-      if (t)
+      if ((t = maybe_fold_or_comparisons (code1, op1a, op1b,
+					  code2, op2a, op2b)))
+	return t;
+    }
+  else if (code2 != ERROR_MARK)
+    {
+      if ((t = or_var_with_comparison (c1, false, code2, op2a, op2b)))
+	return t;
+    }
+  else if (code1 != ERROR_MARK)
+    {
+      if ((t = or_var_with_comparison (c2, false, code1, op1a, op1b)))
 	return t;
     }
 
-- 
1.7.0.4

Re: Fix PR46036: if-conversion problem

[Richard Guenther]

On Mon, 8 Nov 2010, Sebastian Pop wrote:

> Hi,
> 
> Here is a fix for PR46036 bootstrapped and tested on amd64-linux.
> Ok for trunk?

Ok.

Thanks,
Richard.

Re: Fix PR46036: if-conversion problem

[Richard Guenther]

On Mon, 8 Nov 2010, Sebastian Pop wrote:

> Hi,
> 
> While working on PR46036, I remarked that there are some OR
> expressions that do not fold correctly into "true".  The attached
> patch does improve the situation a bit, and there is still a missing
> case a bit more difficult to handle where parse_predicate returns
> two SSA_NAMEs, then we would need to fold "a || b".
> 
> The attached patch passed bootstrap and test on amd64-linux.
> Would this kind of patch be ok for trunk?  Should I send out a
> patch addressing the remaining missing case?

Ok, yes please.

Thanks,
Richard.