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;
+}