On 12-11-14 11:00, Tom de Vries wrote:
Jakub,this patch fixes a gcc_assert in expand_omp_for_static_chunk. The assert follows a loop with composite loop condition: ... vec<edge_var_map> *head = redirect_edge_var_map_vector (re); ene = single_succ_edge (entry_bb); psi = gsi_start_phis (fin_bb); for (i = 0; !gsi_end_p (psi) && head->iterate (i, &vm); gsi_next (&psi), ++i) ... AFAIU, the intention of the assert is that it tries to check that both: - all phis have been handled (gsi_end_p (psi)), and - all elements of head have been used (head->length () == i). In other words, that we have stopped iterating because both loop conditions are false. The current assert checks that *not* all phis have been handled: ... gcc_assert (!gsi_end_p (psi) && i == head->length ()); ... Looking back in the history, it seems we started out with the 'all phis handled' semantics, but I suspect that that got lost due to a typo: ... 79acaae1 2007-09-07 gcc_assert (!phi && !args); 75a70cf95 2008-07-28 gcc_assert (!gsi_end_p (psi) && i == VEC_length (edge_var_map, head)); f1f41a6c 2012-11-18 gcc_assert (!gsi_end_p (psi) && i == head->length ()); ... Now, if the current assert is incorrect, why didn't it trigger? The assert is in ssa-handling code in expand_omp_for_static_chunk. Ssa-handling code in omp-low.c is only triggered by pass_parallelize_loops, and that pass doesn't specify a chunk size on the GIMPLE_OMP_FOR it constructs, so that will only call expand_omp_for_static_nochunk. I managed to trigger this assert in my oacc kernels directive patch set (on top of the gomp-4_0-branch), which constructs an oacc for loop in pass_parallelize_loops, and then this code in gomp-4_0-branch has the effect that we trigger expand_omp_for_static_chunk: ... //TODO /* For OpenACC loops, force a chunk size of one, as this avoids the default scheduling where several subsequent iterations are being executed by the same thread. */ if (gimple_omp_for_kind (for_stmt) == GF_OMP_FOR_KIND_OACC_LOOP) { gcc_assert (fd->chunk_size == NULL_TREE); fd->chunk_size = build_int_cst (TREE_TYPE (fd->loop.v), 1); } ... So, AFAIU, this assert (and associated ssa-handling code in expand_omp_for_static_chunk) is dead on trunk, but I'm excercising the code currently in my patch series, so I'd prefer to fix it rather than remove it. Bootstrapped and reg-tested on x86_64, on top of trunk, gomp-4_0-branch and internal oacc dev branch. OK for trunk?
Ping. Thanks, - Tom
0001-Fix-gcc_assert-in-expand_omp_for_static_chunk.patch 2014-11-12 Tom de Vries<[email protected]> * omp-low.c (expand_omp_for_static_chunk): Fix assert. --- gcc/omp-low.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/omp-low.c b/gcc/omp-low.c index b59d069..5210de1 100644 --- a/gcc/omp-low.c +++ b/gcc/omp-low.c @@ -6775,7 +6775,7 @@ expand_omp_for_static_chunk (struct omp_region *region, locus = redirect_edge_var_map_location (vm); add_phi_arg (nphi, redirect_edge_var_map_def (vm), re, locus); } - gcc_assert (!gsi_end_p (psi) && i == head->length ()); + gcc_assert (gsi_end_p (psi) && i == head->length ()); redirect_edge_var_map_clear (re); while (1) { -- 1.9.1
