diff mbox

PATCH: [4.5 Regression] ICE with graphite enabled in cairo-1.8.10

Message ID 20101129192446.GA8629@intel.com
State New
Headers show

Commit Message

H.J. Lu Nov. 29, 2010, 7:24 p.m. UTC
Hi,

This patch fixes a 4.5 regression.  OK for 4.5?

Thanks.


H.J.
---
gcc/

2010-11-29  H.J. Lu  <hongjiu.lu@intel.com>

	Backport from mainline
    	PR middle-end/46651
	2010-07-22  Sebastian Pop  <sebastian.pop@amd.com>
    
	* graphite-sese-to-poly.c (rewrite_phi_out_of_ssa): Use
	SSA_NAME_DEF_STMT only on SSA_NAMEs.
    
gcc/testsuite/

2010-11-29  H.J. Lu  <hongjiu.lu@intel.com>

	Backport from mainline
	2010-07-22  Sebastian Pop  <sebastian.pop@amd.com>

    	PR middle-end/46651
	* gcc.dg/graphite/id-24.c: New.

Comments

Sebastian Pop Nov. 29, 2010, 9:49 p.m. UTC | #1
On Mon, Nov 29, 2010 at 13:24, H.J. Lu <hongjiu.lu@intel.com> wrote:
> Hi,
>
> This patch fixes a 4.5 regression.  OK for 4.5?
>

Ok for 4.5, thanks for fixing this.

Sebastian Pop
--
AMD / Open Source Compiler Engineering / GNU Tools
Richard Biener Nov. 30, 2010, 11:04 a.m. UTC | #2
On Mon, Nov 29, 2010 at 10:49 PM, Sebastian Pop <sebpop@gmail.com> wrote:
> On Mon, Nov 29, 2010 at 13:24, H.J. Lu <hongjiu.lu@intel.com> wrote:
>> Hi,
>>
>> This patch fixes a 4.5 regression.  OK for 4.5?
>>
>
> Ok for 4.5, thanks for fixing this.

Huh, the patch looks odd.  We now fall through to
insert_out_of_ssa_copy_on_edge.  Why not fix
insert_out_of_ssa_copy instead?

  stmt = SSA_NAME_DEF_STMT (var);
  if (gimple_code (stmt) == GIMPLE_PHI)

the def STMT isn't a PHI if it isn't an SSA name.

Richard.

> Sebastian Pop
> --
> AMD / Open Source Compiler Engineering / GNU Tools
>
Sebastian Pop Nov. 30, 2010, 5:02 p.m. UTC | #3
> Huh, the patch looks odd.  We now fall through to
> insert_out_of_ssa_copy_on_edge.  Why not fix
> insert_out_of_ssa_copy instead?
>
>  stmt = SSA_NAME_DEF_STMT (var);
>  if (gimple_code (stmt) == GIMPLE_PHI)
>
> the def STMT isn't a PHI if it isn't an SSA name.
>

I am sorry, I do not see how to fix insert_out_of_ssa_copy, could you
please be more specific?

Also please note that on trunk the code under the condition looks like
this: we insert the out of SSA copy just after the definition of arg,
so we have to have an SSA_NAME to get the SSA_NAME_DEF_STMT at this point

      if (TREE_CODE (arg) == SSA_NAME
	  && e->src == bb->loop_father->latch)
	insert_out_of_ssa_copy (scop, zero_dim_array, arg,
				SSA_NAME_DEF_STMT (arg));


/* Insert the assignment "RES := EXPR" just after AFTER_STMT.  */

static void
insert_out_of_ssa_copy (scop_p scop, tree res, tree expr, gimple after_stmt)
Richard Biener Dec. 1, 2010, 10:38 a.m. UTC | #4
On Tue, Nov 30, 2010 at 6:02 PM, Sebastian Pop <sebpop@gmail.com> wrote:
>> Huh, the patch looks odd.  We now fall through to
>> insert_out_of_ssa_copy_on_edge.  Why not fix
>> insert_out_of_ssa_copy instead?
>>
>>  stmt = SSA_NAME_DEF_STMT (var);
>>  if (gimple_code (stmt) == GIMPLE_PHI)
>>
>> the def STMT isn't a PHI if it isn't an SSA name.
>>
>
> I am sorry, I do not see how to fix insert_out_of_ssa_copy, could you
> please be more specific?
>
> Also please note that on trunk the code under the condition looks like
> this: we insert the out of SSA copy just after the definition of arg,
> so we have to have an SSA_NAME to get the SSA_NAME_DEF_STMT at this point
>
>      if (TREE_CODE (arg) == SSA_NAME
>          && e->src == bb->loop_father->latch)
>        insert_out_of_ssa_copy (scop, zero_dim_array, arg,
>                                SSA_NAME_DEF_STMT (arg));
>
>
> /* Insert the assignment "RES := EXPR" just after AFTER_STMT.  */
>
> static void
> insert_out_of_ssa_copy (scop_p scop, tree res, tree expr, gimple after_stmt)
>

The trunk looks different anyway it seems.  But yes, I didn't look
carefully enough, so ignore my comments.

Richard.
diff mbox

Patch

diff --git a/gcc/graphite-sese-to-poly.c b/gcc/graphite-sese-to-poly.c
index b12210b..b8f332a 100644
--- a/gcc/graphite-sese-to-poly.c
+++ b/gcc/graphite-sese-to-poly.c
@@ -2235,7 +2235,8 @@  rewrite_phi_out_of_ssa (gimple_stmt_iterator *psi)
 
       /* Avoid the insertion of code in the loop latch to please the
 	 pattern matching of the vectorizer.  */
-      if (e->src == bb->loop_father->latch)
+      if (TREE_CODE (arg) == SSA_NAME
+	  && e->src == bb->loop_father->latch)
  	insert_out_of_ssa_copy (zero_dim_array, arg);
       else
 	insert_out_of_ssa_copy_on_edge (e, zero_dim_array, arg);
diff --git a/gcc/testsuite/gcc.dg/graphite/id-24.c b/gcc/testsuite/gcc.dg/graphite/id-24.c
new file mode 100644
index 0000000..d466069
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/graphite/id-24.c
@@ -0,0 +1,22 @@ 
+/* gcc.dg/tree-ssa/loadpre23.c used to ICE with Graphite.  */
+
+struct {
+  int a;
+  int large[100];
+} x;
+
+int foo(int argc)
+{
+  int b;
+  int c;
+  int i;
+  int d, e;
+
+  for (i = 0; i < argc; i++)
+    {
+      e = x.a;
+      x.a = 9;
+    }
+  return d + e;
+}
+