Re: [patch] Fix PR43464: update loop closed SSA form once copy prop is done

[Sebastian Pop <sebpop at gmail dot com>]

On Thu, Apr 1, 2010 at 09:02, Sebastian Pop <sebpop@gmail.com> wrote:
> On Thu, Apr 1, 2010 at 08:46, Sebastian Pop <sebpop@gmail.com> wrote:
>> Hi,
>>
>> On Thu, Apr 1, 2010 at 05:28, Michael Matz <matz@suse.de> wrote:
>>>> Michael, do you see other places in graphite-sese-to-poly.c that
>>>> may still have similar problems?
>>>
>>> No, that's the one I meant. ?1) Are you sure BB->loop structure is current
>>> when that function is called, so that loop_exit_edge_p works?
>>
>> yes.
>>
>> I found another problem a few lines below that point: in
>> rewrite_close_phi_out_of_ssa we assume that the phi node has
>> only one argument:
>>
>> ?tree arg = gimple_phi_arg_def (phi, 0);
>>
>> I am working on correcting this.
>
> Actually there is no problem with all these assumptions in Graphite,
> as we transform the loop closed ssa form in a canonical form:
>
> /* Converts the current loop closed SSA form to a canonical form
> ? expected by the Graphite code generation.
>
> ? The loop closed SSA form has the following invariant: a variable
> ? defined in a loop that is used outside the loop appears only in the
> ? phi nodes in the destination of the loop exit. ?These phi nodes are
> ? called close phi nodes.
>
> ? The canonical loop closed SSA form contains the extra invariants:
>
> ? - when the loop contains only one exit, the close phi nodes contain
> ? only one argument. ?That implies that the basic block that contains
> ? the close phi nodes has only one predecessor, that is a basic block
> ? in the loop.
>
> ? - the basic block containing the close phi nodes does not contain
> ? other statements.
> */
>
> static void
> canonicalize_loop_closed_ssa_form (void)
>

I will commit the attached patch to the Graphite branch for testing.

Sebastian

From 87f9a2f0d8f5fda668cb0a21a0aef2236f1cbf14 Mon Sep 17 00:00:00 2001
From: Sebastian Pop <sebpop@gmail.com>
Date: Thu, 1 Apr 2010 13:43:43 -0500
Subject: [PATCH] Add extra checks for places assuming one argument loop close SSA form.

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

	* graphite-sese-to-poly.c (rewrite_close_phi_out_of_ssa): Assert that
	gimple_phi_num_args of the loop close SSA phi node is equal to 1.
	(detect_commutative_reduction): Same.
---
 gcc/ChangeLog.graphite      |    6 ++++++
 gcc/graphite-sese-to-poly.c |   10 ++++++++++
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/gcc/ChangeLog.graphite b/gcc/ChangeLog.graphite
index 5af8f36..ce27561 100644
--- a/gcc/ChangeLog.graphite
+++ b/gcc/ChangeLog.graphite
@@ -1,3 +1,9 @@
+2010-04-01  Sebastian Pop  <sebastian.pop@amd.com>
+
+	* graphite-sese-to-poly.c (rewrite_close_phi_out_of_ssa): Assert that
+	gimple_phi_num_args of the loop close SSA phi node is equal to 1.
+	(detect_commutative_reduction): Same.
+
 2010-03-31  Sebastian Pop  <sebastian.pop@amd.com>
 
 	* graphite-sese-to-poly.c (scalar_close_phi_node_p): Loop
diff --git a/gcc/graphite-sese-to-poly.c b/gcc/graphite-sese-to-poly.c
index bb241da..01c166e 100644
--- a/gcc/graphite-sese-to-poly.c
+++ b/gcc/graphite-sese-to-poly.c
@@ -2212,6 +2212,11 @@ rewrite_close_phi_out_of_ssa (gimple_stmt_iterator *psi)
   gimple stmt = gimple_build_assign (res, zero_dim_array);
   tree arg = gimple_phi_arg_def (phi, 0);
 
+  /* Note that loop close phi nodes should have a single argument
+     because we translated the representation into a canonical form
+     before Graphite: see canonicalize_loop_closed_ssa_form.  */
+  gcc_assert (gimple_phi_num_args (phi) == 1);
+
   if (TREE_CODE (arg) == SSA_NAME
       && !SSA_NAME_IS_DEFAULT_DEF (arg))
     insert_out_of_ssa_copy (zero_dim_array, arg);
@@ -2679,6 +2684,11 @@ detect_commutative_reduction (gimple stmt, VEC (gimple, heap) **in,
       if (TREE_CODE (arg) != SSA_NAME)
 	return NULL;
 
+      /* Note that loop close phi nodes should have a single argument
+	 because we translated the representation into a canonical form
+	 before Graphite: see canonicalize_loop_closed_ssa_form.  */
+      gcc_assert (gimple_phi_num_args (stmt) == 1);
+
       def = SSA_NAME_DEF_STMT (arg);
       loop_phi = detect_commutative_reduction (def, in, out);
 
-- 
1.6.3.3