On 30/05/16 12:56, Richard Biener wrote:
On Mon, 30 May 2016, Tom de Vries wrote:

>On 30/05/16 11:46, Richard Biener wrote:
> > >This patch fixes the assert conservatively by aborting graphite code
> > > >generation when encountering a phi with more than two arguments in
> > > >copy_bb_and_scalar_dependences.
> > > >
> > > >Bootstrapped and reg-tested on x86_64.
> > > >
> > > >OK for trunk, 6 branch?
>
> >Did you check if simply returning false from bb_contains_loop_phi_nodes
> >instead of asserting works?  The caller has a 'else' that is supposed
> >to handle condition PHIs.  After all it already handles one predecessor
> >specially ...  Thus
> >
> >    if (EDGE_COUNT (bb->preds) != 2)
> >      return false;
> >
> >should work here.
>
>Unfortunately, that doesn't work. We run into another assert in
>copy_cond_phi_nodes:
>...
>   /* Cond phi nodes should have exactly two arguments.  */
>   gcc_assert (2 == EDGE_COUNT (bb->preds));
>...
Hah.  So can we still do my suggested change and bail out conservatively
in Cond PHI node handling instead?  Because the PHI node is clearly
_not_  a loop header PHI and the cond phi assert is also a latent bug.


Agreed. Updated as attached.

OK for trunk, 6 branch?

Thanks,
- Tom


Handle 3-arg phi in copy_bb_and_scalar_dependences

2016-05-30  Tom de Vries  <t...@codesourcery.com>

	PR tree-optimization/69068
	* graphite-isl-ast-to-gimple.c (copy_bb_and_scalar_dependences): Handle
	phis with more than two args.

	* gcc.dg/graphite/pr69068.c: New test.

---
 gcc/graphite-isl-ast-to-gimple.c        | 10 ++++++----
 gcc/testsuite/gcc.dg/graphite/pr69068.c | 14 ++++++++++++++
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/gcc/graphite-isl-ast-to-gimple.c b/gcc/graphite-isl-ast-to-gimple.c
index ff1d91f..b4ee9a9 100644
--- a/gcc/graphite-isl-ast-to-gimple.c
+++ b/gcc/graphite-isl-ast-to-gimple.c
@@ -1075,7 +1075,8 @@ bb_contains_loop_close_phi_nodes (basic_block bb)
 static bool
 bb_contains_loop_phi_nodes (basic_block bb)
 {
-  gcc_assert (EDGE_COUNT (bb->preds) <= 2);
+  if (EDGE_COUNT (bb->preds) != 2)
+    return false;
 
   if (bb->preds->length () == 1)
     return false;
@@ -2480,13 +2481,14 @@ copy_cond_phi_nodes (basic_block bb, basic_block new_bb, vec<tree> iv_map)
 
   gcc_assert (!bb_contains_loop_close_phi_nodes (bb));
 
+  /* TODO: Handle cond phi nodes with more than 2 predecessors.  */
+  if (EDGE_COUNT (bb->preds) != 2)
+    return false;
+
   if (dump_file)
     fprintf (dump_file, "[codegen] copying cond phi nodes in bb_%d.\n",
 	     new_bb->index);
 
-  /* Cond phi nodes should have exactly two arguments.  */
-  gcc_assert (2 == EDGE_COUNT (bb->preds));
-
   for (gphi_iterator psi = gsi_start_phis (bb); !gsi_end_p (psi);
        gsi_next (&psi))
     {
diff --git a/gcc/testsuite/gcc.dg/graphite/pr69068.c b/gcc/testsuite/gcc.dg/graphite/pr69068.c
new file mode 100644
index 0000000..0abea06
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/graphite/pr69068.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-O1 -fgraphite-identity" } */
+
+int qo;
+int zh[2];
+
+void
+td (void)
+{
+  int ly, en;
+  for (ly = 0; ly < 2; ++ly)
+    for (en = 0; en < 2; ++en)
+      zh[en] = ((qo == 0) || (((qo * 2) != 0))) ? 1 : -1;
+}

Reply via email to