Patchwork Handle GIMPLE_ASSIGNs with different vuse in gimple_equal_p

login
register
mail settings
Submitter Tom de Vries
Date Nov. 9, 2013, 4:30 p.m.
Message ID <527E6308.6030104@mentor.com>
Download mbox | patch
Permalink /patch/290003/
State New
Headers show

Comments

Tom de Vries - Nov. 9, 2013, 4:30 p.m.
Richard,

Consider the test-case test.c:
...
int z;
int x;

void
f (int c, int d)
{
   if (c)
     z = 5;
   else
     {
       if (d)
	x = 4;
       z = 5;
     }
}
...

Atm, we don't tail-merge the 'z = 5' blocks, because gimple_equal_p returns 
false for the 'z = 5' statements. The relevant code is this:
...
       if (TREE_CODE (lhs1) != SSA_NAME
           && TREE_CODE (lhs2) != SSA_NAME)
         return (vn_valueize (gimple_vdef (s1))
                 == vn_valueize (gimple_vdef (s2)));
...
The vdefs of the 'z = 5' statements are different, because the incoming vuses 
are different.

This patch handles GIMPLE_ASSIGNs with different vuse in gimple_equal_p, by 
doing a structural comparison.

Bootstrapped and regtested on x86_64.

OK for trunk?

Thanks,
- Tom

2013-11-06  Tom de Vries  <tom@codesourcery.com>

	* tree-ssa-tail-merge.c (gimple_equal_p): Add test for structural
	equality for GIMPLE_ASSIGN.

	* gcc.dg/tail-merge-store.c: New test.
aldot - Nov. 10, 2013, 9:19 a.m.
On Sat, Nov 09, 2013 at 05:30:00PM +0100, Tom de Vries wrote:
>Richard,
>
>Consider the test-case test.c:
>...
>int z;
>int x;
>
>void
>f (int c, int d)
>{
>  if (c)
>    z = 5;
>  else
>    {
>      if (d)
>	x = 4;
>      z = 5;
>    }
>}
>...
>
>Atm, we don't tail-merge the 'z = 5' blocks, because gimple_equal_p
>returns false for the 'z = 5' statements. The relevant code is this:
>...
>      if (TREE_CODE (lhs1) != SSA_NAME
>          && TREE_CODE (lhs2) != SSA_NAME)
>        return (vn_valueize (gimple_vdef (s1))
>                == vn_valueize (gimple_vdef (s2)));
>...
>The vdefs of the 'z = 5' statements are different, because the
>incoming vuses are different.
>
>This patch handles GIMPLE_ASSIGNs with different vuse in
>gimple_equal_p, by doing a structural comparison.
>
>Bootstrapped and regtested on x86_64.
>
>OK for trunk?
>
>Thanks,
>- Tom
>
>2013-11-06  Tom de Vries  <tom@codesourcery.com>
>
>	* tree-ssa-tail-merge.c (gimple_equal_p): Add test for structural
>	equality for GIMPLE_ASSIGN.
>
>	* gcc.dg/tail-merge-store.c: New test.

>diff --git a/gcc/testsuite/gcc.dg/tail-merge-store.c b/gcc/testsuite/gcc.dg/tail-merge-store.c
>new file mode 100644
>index 0000000..1aefbdc
>--- /dev/null
>+++ b/gcc/testsuite/gcc.dg/tail-merge-store.c
>@@ -0,0 +1,22 @@
>+/* { dg-do compile } */
>+/* { dg-options "-O2 -ftree-tail-merge -fdump-tree-pre" } */
>+
>+int z;
>+int x;
>+
>+void
>+f (int c, int d)
>+{
>+  if (c)
>+    z = 5;
>+  else
>+    {
>+      if (d)
>+	x = 4;
>+      z = 5;
>+    }
>+}
>+
>+/* { dg-final { scan-tree-dump-times "duplicate of" 1 "pre"} } */
>+/* { dg-final { scan-tree-dump-times "z = 5" 1 "pre"} } */
>+/* { dg-final { cleanup-tree-dump "pre" } } */
>diff --git a/gcc/tree-ssa-tail-merge.c b/gcc/tree-ssa-tail-merge.c
>index 98b5882..43516a7 100644
>--- a/gcc/tree-ssa-tail-merge.c
>+++ b/gcc/tree-ssa-tail-merge.c
>@@ -1173,8 +1173,47 @@ gimple_equal_p (same_succ same_succ, gimple s1, gimple s2)
>       lhs2 = gimple_get_lhs (s2);
>       if (TREE_CODE (lhs1) != SSA_NAME
> 	  && TREE_CODE (lhs2) != SSA_NAME)
>-	return (vn_valueize (gimple_vdef (s1))
>-		== vn_valueize (gimple_vdef (s2)));
>+	{
>+	  /* If the vdef is the same, it's the same statement.  */
>+	  if (vn_valueize (gimple_vdef (s1))
>+	      == vn_valueize (gimple_vdef (s2)))
>+	    return true;
>+
>+	  /* If the vdef is not the same but the vuse is the same, it's not the
>+	     same stmt.  */
>+	  if (vn_valueize (gimple_vuse (s1))
>+	      == vn_valueize (gimple_vuse (s2)))
>+	    return false;
>+	  /* If the vdef is not the same and the vuse is not the same, it might be
>+	     same stmt.  */
>+
>+	  /* Test for structural equality.  */
>+	  if (gimple_assign_rhs_code (s1) != gimple_assign_rhs_code (s1)

typo, second one should be s2.
thanks,

>+	      || (gimple_assign_nontemporal_move_p (s1)
>+		  != gimple_assign_nontemporal_move_p (s2)))
>+	    return false;
>+
>+	  if (!operand_equal_p (lhs1, lhs2, 0))
>+	    return false;
>+
>+	  t1 = gimple_assign_rhs1 (s1);
>+	  t2 = gimple_assign_rhs1 (s2);
>+	  if (!gimple_operand_equal_value_p (t1, t2))
>+	    return false;
>+
>+	  t1 = gimple_assign_rhs2 (s1);
>+	  t2 = gimple_assign_rhs2 (s2);
>+	  if (!gimple_operand_equal_value_p (t1, t2))
>+	    return false;
>+
>+	  t1 = gimple_assign_rhs3 (s1);
>+	  t2 = gimple_assign_rhs3 (s2);
>+	  if (!gimple_operand_equal_value_p (t1, t2))
>+	    return false;
>+
>+	  /* Same structure.  */
>+	  return true;
>+	}
>       else if (TREE_CODE (lhs1) == SSA_NAME
> 	       && TREE_CODE (lhs2) == SSA_NAME)
> 	return vn_valueize (lhs1) == vn_valueize (lhs2);

Patch

diff --git a/gcc/testsuite/gcc.dg/tail-merge-store.c b/gcc/testsuite/gcc.dg/tail-merge-store.c
new file mode 100644
index 0000000..1aefbdc
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tail-merge-store.c
@@ -0,0 +1,22 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -ftree-tail-merge -fdump-tree-pre" } */
+
+int z;
+int x;
+
+void
+f (int c, int d)
+{
+  if (c)
+    z = 5;
+  else
+    {
+      if (d)
+	x = 4;
+      z = 5;
+    }
+}
+
+/* { dg-final { scan-tree-dump-times "duplicate of" 1 "pre"} } */
+/* { dg-final { scan-tree-dump-times "z = 5" 1 "pre"} } */
+/* { dg-final { cleanup-tree-dump "pre" } } */
diff --git a/gcc/tree-ssa-tail-merge.c b/gcc/tree-ssa-tail-merge.c
index 98b5882..43516a7 100644
--- a/gcc/tree-ssa-tail-merge.c
+++ b/gcc/tree-ssa-tail-merge.c
@@ -1173,8 +1173,47 @@  gimple_equal_p (same_succ same_succ, gimple s1, gimple s2)
       lhs2 = gimple_get_lhs (s2);
       if (TREE_CODE (lhs1) != SSA_NAME
 	  && TREE_CODE (lhs2) != SSA_NAME)
-	return (vn_valueize (gimple_vdef (s1))
-		== vn_valueize (gimple_vdef (s2)));
+	{
+	  /* If the vdef is the same, it's the same statement.  */
+	  if (vn_valueize (gimple_vdef (s1))
+	      == vn_valueize (gimple_vdef (s2)))
+	    return true;
+
+	  /* If the vdef is not the same but the vuse is the same, it's not the
+	     same stmt.  */
+	  if (vn_valueize (gimple_vuse (s1))
+	      == vn_valueize (gimple_vuse (s2)))
+	    return false;
+	  /* If the vdef is not the same and the vuse is not the same, it might be
+	     same stmt.  */
+
+	  /* Test for structural equality.  */
+	  if (gimple_assign_rhs_code (s1) != gimple_assign_rhs_code (s1)
+	      || (gimple_assign_nontemporal_move_p (s1)
+		  != gimple_assign_nontemporal_move_p (s2)))
+	    return false;
+
+	  if (!operand_equal_p (lhs1, lhs2, 0))
+	    return false;
+
+	  t1 = gimple_assign_rhs1 (s1);
+	  t2 = gimple_assign_rhs1 (s2);
+	  if (!gimple_operand_equal_value_p (t1, t2))
+	    return false;
+
+	  t1 = gimple_assign_rhs2 (s1);
+	  t2 = gimple_assign_rhs2 (s2);
+	  if (!gimple_operand_equal_value_p (t1, t2))
+	    return false;
+
+	  t1 = gimple_assign_rhs3 (s1);
+	  t2 = gimple_assign_rhs3 (s2);
+	  if (!gimple_operand_equal_value_p (t1, t2))
+	    return false;
+
+	  /* Same structure.  */
+	  return true;
+	}
       else if (TREE_CODE (lhs1) == SSA_NAME
 	       && TREE_CODE (lhs2) == SSA_NAME)
 	return vn_valueize (lhs1) == vn_valueize (lhs2);