| Message ID | 574C475A.3040304@mentor.com |
|---|---|
| State | New |
| Headers | show |
On Mon, 30 May 2016, Tom de Vries wrote: > 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? Ok with the now redundant 2nd check removed @@ -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; ^^^^^^^^^^ Thanks, Richard. > Thanks, > - Tom > > >
Handle 3-arg phi in copy_bb_and_scalar_dependences 2016-05-30 Tom de Vries <tom@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; +}