[PR65637,3/5] Fix gcc_assert in expand_omp_for_static_chunk

[ Was: Re: [PR65637][PATCH][1/3] Fix gcc_assert in 
expand_omp_for_static_chunk ]

On 15/04/15 15:15, Tom de Vries wrote:
> On 15-04-15 15:10, Tom de Vries wrote:
>> Hi,
>>
>> This patch series fixes PR65637.
>>

<SNIP>

> This patch fixes a segfault in an gcc_assert in expand_omp_for_static_chunk
> while compiling autopar/pr46099.c.
>
> When compiling f1 from autopar/pr46099.c using
> expand_omp_for_static_chunk, we
> redirect the edge (trip_update_bb -> fin_bb) to point to iter_part_bb:
> ...
>        redirect_edge_and_branch (single_succ_edge (trip_update_bb),
> iter_part_bb);
> ...
>
> And fin_bb is an empty block without any phis, so during the redirect we
> don't
> store any entries in the edge_var_map:
> ...
> (gdb) call debug_bb (fin_bb)
> ;; basic block 18, loop depth 0, count 0, freq 0, maybe hot
> ;;  prev block 21, next block 16, flags: (NEW)
> ;;  pred:       21 [100.0%]  (FALLTHRU)
> ;;              19 (FALSE_VALUE)
> ;;  succ:       16 [100.0%]  (FALLTHRU)
> ...
>
> Consequently, head will be NULL.
> ...
>        vec<edge_var_map> *head = redirect_edge_var_map_vector (re);
> ...
>
> And because head is NULL, this assert causes a segfault:
> ...
>    gcc_assert (gsi_end_p (psi) && i == head->length ());
> ...
>
> This patch fixes that, by handling the case that head is NULL in the
> assert.
>

This updated patch includes a test-case.

OK for trunk?

Thanks,
- Tom

On Mon, Aug 31, 2015 at 01:55:40PM +0200, Tom de Vries wrote:
> Fix gcc_assert in expand_omp_for_static_chunk
> 
> 2015-08-31  Tom de Vries  <tom@codesourcery.com>
> 
> 	PR tree-optimization/65637
> 	* omp-low.c (expand_omp_for_static_chunk): Fix gcc_assert for the case
> 	that head is NULL.
> 
> 	* gcc.dg/autopar/pr46099-chunk-size.c: New test.
> ---
>  gcc/ChangeLog                                     |  6 +++
>  gcc/omp-low.c                                     |  2 +-
>  gcc/testsuite/gcc.dg/autopar/pr46099-chunk-size.c | 47 +++++++++++++++++++++++
>  3 files changed, 54 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.dg/autopar/pr46099-chunk-size.c
> 
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index a0123b1..5a273ba 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,9 @@
> +2015-05-18  Tom de Vries  <tom@codesourcery.com>
> +
> +	PR tree-optimization/65637
> +	* omp-low.c (expand_omp_for_static_chunk): Fix gcc_assert for the case
> +	that head is NULL.
> +
>  2015-08-31  Tom de Vries  <tom@codesourcery.com>
>  
>  	* tree-ssa-loop-manip.c (find_uses_to_rename_use)
> diff --git a/gcc/omp-low.c b/gcc/omp-low.c
> index c3dfc51..4e732ae 100644
> --- a/gcc/omp-low.c
> +++ b/gcc/omp-low.c
> @@ -7326,7 +7326,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) && (head == NULL || i == head->length ()));
>        redirect_edge_var_map_clear (re);
>        while (1)
>  	{

Ok.

> diff --git a/gcc/testsuite/gcc.dg/autopar/pr46099-chunk-size.c b/gcc/testsuite/gcc.dg/autopar/pr46099-chunk-size.c
> new file mode 100644
> index 0000000..709841a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/autopar/pr46099-chunk-size.c

I'd name the testcase just pr46099-2.c.

> @@ -0,0 +1,47 @@
> +/* PR tree-optimization/46099.  */
> +/* { dg-do compile } */
> +/* { dg-options "-ftree-parallelize-loops=2 -fcompare-debug -O --param parloops-chunk-size=100" } */

But more importantly, if you haven't changed anything in the testcase
beyond dg-options, just
#include "pr46099.c"
here rather than duplicating the whole testcase.  Ok with that change.

	Jakub