Message ID | 574AE27B.7060205@mentor.com |
---|---|
State | New |
Headers | show |
On Sun, 29 May 2016, Tom de Vries wrote: > Hi, > > this patch fixes graphite PR69067, a 6/7 regression. > > > I. > > Consider the following test-case, compiled with -O1 -floop-nest-optimize > -flto: > ... > int a1, c1, cr, kt; > int aa[2]; > > int > ce (void) > { > while (a1 < 1) { > int g8; > for (g8 = 0; g8 < 3; ++g8) > if (c1 != 0) > cr = aa[a1 * 2] = kt; > for (c1 = 0; c1 < 2; ++c1) > aa[c1] = cr; > ++a1; > } > return 0; > } > > int > main (void) > { > return ce (aa); > } > ... > > At graphite0, there's a loop with header bb4, which conditionally executes bb > 5: > ... > <bb 3>: > > <bb 4>: > # g8_39 = PHI <0(13), g8_19(3)> > # cr_lsm.1_4 = PHI <cr_lsm.1_21(13), cr_lsm.1_33(3)> > # cr_lsm.2_22 = PHI <cr_lsm.2_23(13), cr_lsm.2_11(3)> > if (c1_lsm.3_35 != 0) > goto <bb 5>; > else > goto <bb 6>; > > <bb 5>: > aa[_3] = 0; > > <bb 6>: > # cr_lsm.1_33 = PHI <cr_lsm.1_4(4), 0(5)> > # cr_lsm.2_11 = PHI <cr_lsm.2_22(4), 1(5)> > g8_19 = g8_39 + 1; > if (g8_19 <= 2) > goto <bb 3>; > else > goto <bb 7>; > ... > > > II. > > The graphite transformation moves the condition '(P_35 <= -1 || P_35 >= 1)' > out of the loop: > ... > [scheduler] original ast: > { > for (int c0 = 0; c0 <= 2; c0 += 1) { > S_4(c0); > if (P_35 <= -1 || P_35 >= 1) > S_5(c0); > S_6(c0); > } > S_7(); > for (int c0 = 0; c0 <= 1; c0 += 1) > S_8(c0); > } > > [scheduler] AST generated by isl: > { > if (P_35 <= -1) { > for (int c0 = 0; c0 <= 2; c0 += 1) > S_5(c0); > } else if (P_35 >= 1) > for (int c0 = 0; c0 <= 2; c0 += 1) > S_5(c0); > for (int c0 = 0; c0 <= 2; c0 += 1) { > S_4(c0); > S_6(c0); > } > S_7(); > for (int c0 = 0; c0 <= 1; c0 += 1) > S_8(c0); > } > ... > > When instantiating the ast back to gimple, we run into an assert: > ... > pr-graphite-4.c: In function ‘ce’: > pr-graphite-4.c:5:1: internal compiler error: in get_def_bb_for_const, at > graphite-isl-ast-to-gimple.c:1795 > ce() > ^ > ... > > > III. > > What happens is the following: in copy_cond_phi_args we try to copy the > arguments of phi in bb6 to the arguments of new_phi in bb 46 > ... > (gdb) call debug_gimple_stmt (phi) > cr_lsm.1_33 = PHI <cr_lsm.1_4(4), 0(5)> > (gdb) call debug_gimple_stmt (new_phi) > cr_lsm.1_62 = PHI <(28), (47)> > ... > > While handling the '0' phi argument in add_phi_arg_for_new_expr we trigger > this bit of code and call get_def_bb_for_const with bb.index == 46 and > old_bb.index == 5: > ... > /* If the corresponding def_bb could not be found the entry will > be NULL. */ > if (TREE_CODE (old_phi_args[i]) == INTEGER_CST) > def_pred[i] > = get_def_bb_for_const (new_bb, > gimple_phi_arg_edge (phi, i)->src); > ... > > Neither of the two copies of bb 5 dominates bb 46, so we run into the assert > at the end: > ... > /* Returns a basic block that could correspond to where a constant was > defined in the original code. In the original code OLD_BB had the > definition, we need to find which basic block out of the copies of > old_bb, in the new region, should a definition correspond to if it > has to reach BB. */ > > basic_block translate_isl_ast_to_gimple:: > get_def_bb_for_const (basic_block bb, basic_block old_bb) const > { > vec <basic_block> *bbs = region->copied_bb_map->get (old_bb); > > if (!bbs || bbs->is_empty ()) > return NULL; > > if (1 == bbs->length ()) > return (*bbs)[0]; > > int i; > basic_block b1 = NULL, b2; > FOR_EACH_VEC_ELT (*bbs, i, b2) > { > if (b2 == bb) > return bb; > > /* BB and B2 are in two unrelated if-clauses. */ > if (!dominated_by_p (CDI_DOMINATORS, bb, b2)) > continue; > > /* Compute the nearest dominator. */ > if (!b1 || dominated_by_p (CDI_DOMINATORS, b2, b1)) > b1 = b2; > } > > gcc_assert (b1); > return b1; > } > ... > > > IV. > > Attached patch fixes this by removing the assert. > > Bootstrapped and reg-tested on x86_64. > > OK for trunk, 6-branch? Ok. Thanks, Richard. > Thanks, > - Tom >
Remove assert in get_def_bb_for_const 2016-05-29 Tom de Vries <tom@codesourcery.com> PR tree-optimization/69067 * graphite-isl-ast-to-gimple.c (get_def_bb_for_const): Remove assert. * gcc.dg/graphite/pr69067.c: New test. --- gcc/graphite-isl-ast-to-gimple.c | 1 - gcc/testsuite/gcc.dg/graphite/pr69067.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/gcc/graphite-isl-ast-to-gimple.c b/gcc/graphite-isl-ast-to-gimple.c index 049a4c5..ff1d91f 100644 --- a/gcc/graphite-isl-ast-to-gimple.c +++ b/gcc/graphite-isl-ast-to-gimple.c @@ -1792,7 +1792,6 @@ get_def_bb_for_const (basic_block bb, basic_block old_bb) const b1 = b2; } - gcc_assert (b1); return b1; } diff --git a/gcc/testsuite/gcc.dg/graphite/pr69067.c b/gcc/testsuite/gcc.dg/graphite/pr69067.c new file mode 100644 index 0000000..d767381d --- /dev/null +++ b/gcc/testsuite/gcc.dg/graphite/pr69067.c @@ -0,0 +1,28 @@ +/* { dg-do link } */ +/* { dg-options " -O1 -floop-nest-optimize" } */ +/* { dg-additional-options "-flto" { target lto } } */ + +int a1, c1, cr, kt; +int aa[2]; + +int +ce (void) +{ + while (a1 < 1) + { + int g8; + for (g8 = 0; g8 < 3; ++g8) + if (c1 != 0) + cr = aa[a1 * 2] = kt; + for (c1 = 0; c1 < 2; ++c1) + aa[c1] = cr; + ++a1; + } + return 0; +} + +int +main (void) +{ + return ce (aa); +}