asking your advice about bug

[Tobias Grosser]

On 02/17/2014 06:50 PM, Roman Gareev wrote:
>
>
> Hi Tobias,
>
>
>   thanks for the answer!
>
>
>   I think that the segfault is being caused by NULL arguments being passedto compute_deps
> by loop_level_carries_dependences.*This is **causing **an*  *assignment of**
> NULL values to the following parameters of **compute_deps:* must_raw_no_source,
> may_raw_no_source, must_war_no_source, may_war_no_source,
> must_waw_no_source, may_waw_no_source. They are being passed to subtract_commutative_associative_deps
> and dereferenced in the following statements:
>
>
>   *must_raw_no_source = isl_union_map_subtract (*must_raw_no_source,
> x_must_raw_no_source);
>
>
>   *may_raw_no_source = isl_union_map_subtract (*may_raw_no_source,
> x_may_raw_no_source);
>
>
>   *must_war_no_source = isl_union_map_subtract (*must_war_no_source,
> x_must_war_no_source);
>
>
>   *may_war_no_source = isl_union_map_subtract (*may_war_no_source,
> x_may_war_no_source);
>
>
>   *must_waw_no_source = isl_union_map_subtract (*must_waw_no_source,
> x_must_waw_no_source);
>
>
>   *may_waw_no_source = isl_union_map_subtract (*may_waw_no_source,
> x_may_waw_no_source);
>
>
>   This is the reason of segfault. (All functions mentioned above are located
> in gcc/graphite-dependences.c)
>
>
>   I think that this can be solved by the addition to
> subtract_commutative_associative_deps of NULL checking of the following
> variables: must_raw_no_source, may_raw_no_source, must_war_no_source,
> may_war_no_source, must_waw_no_source, may_waw_no_source. I've implemented
> this in the patch, which can be found below.
>
>
>   Tested x86_64-unknown-linux-gnu, applying to revisions 189156, 207802
> (svn://gcc.gnu.org/svn/gcc/trunk) and 207802
> (svn://gcc.gnu.org/svn/gcc/branches/ibm/gcc-4_8-branch)
>
>
>   Thanks for your answers and advice, Sven!
>
>
>   --
>
> Roman Gareev
>
> -- --- You received this message because you are subscribed to the
> Google Groups "GCC GRAPHITE" group. To unsubscribe from this group and
> stop receiving emails from it, send an email to
> gcc-graphite+unsubscribe@googlegroups.com. For more options, visit
> https://groups.google.com/groups/opt_out.
>
>
> patch
>
>
> diff --git a/gcc/graphite-dependences.c b/gcc/graphite-dependences.c
> index b0f8680..002e3d1 100644
> --- a/gcc/graphite-dependences.c
> +++ b/gcc/graphite-dependences.c
> @@ -424,24 +424,83 @@ subtract_commutative_associative_deps (scop_p scop,
>   					  &x_may_waw_no_source);
>   	gcc_assert (res == 0);
>
> -	*must_raw = isl_union_map_subtract (*must_raw, x_must_raw);
> -	*may_raw = isl_union_map_subtract (*may_raw, x_may_raw);
> -	*must_raw_no_source = isl_union_map_subtract (*must_raw_no_source,
> -						      x_must_raw_no_source);
> -	*may_raw_no_source = isl_union_map_subtract (*may_raw_no_source,
> -						     x_may_raw_no_source);
> -	*must_war = isl_union_map_subtract (*must_war, x_must_war);
> -	*may_war = isl_union_map_subtract (*may_war, x_may_war);
> -	*must_war_no_source = isl_union_map_subtract (*must_war_no_source,
> -						      x_must_war_no_source);
> -	*may_war_no_source = isl_union_map_subtract (*may_war_no_source,
> -						     x_may_war_no_source);
> -	*must_waw = isl_union_map_subtract (*must_waw, x_must_waw);
> -	*may_waw = isl_union_map_subtract (*may_waw, x_may_waw);
> -	*must_waw_no_source = isl_union_map_subtract (*must_waw_no_source,
> -						      x_must_waw_no_source);
> -	*may_waw_no_source = isl_union_map_subtract (*may_waw_no_source,
> -						     x_may_waw_no_source);
> +	if (must_raw)
> +	  *must_raw = isl_union_map_subtract (*must_raw, x_must_raw);
> +	else
> +	  isl_union_map_free (x_must_raw);
> +
> +	if (may_raw)
> +	  *may_raw = isl_union_map_subtract (*may_raw, x_may_raw);
> +	else
> +	  isl_union_map_free (x_may_raw);

In my understanding, it is sufficient to guard the no_source statements, no?

> +
> +	if (must_raw_no_source)
> +	  {
> +	    *must_raw_no_source = isl_union_map_subtract (*must_raw_no_source,
> +						          x_must_raw_no_source);
> +	  }
> +	else
> +	  isl_union_map_free (x_must_raw_no_source);

Could you remove the '{' '}' around the first statement?

> +
> +	if (may_raw_no_source)
> +	  {
> +	    *may_raw_no_source = isl_union_map_subtract (*may_raw_no_source,
> +						         x_may_raw_no_source);
> +	  }
> +	else
> +	  isl_union_map_free (x_may_raw_no_source);

Could you remove the '{' '}' around the first statement?

> +	if (must_war)
> +	  *must_war = isl_union_map_subtract (*must_war, x_must_war);
> +	else
> +	  isl_union_map_free (x_must_war);
> +
> +	if (may_war)
> +	  *may_war = isl_union_map_subtract (*may_war, x_may_war);
> +	else
> +	  isl_union_map_free (x_may_war);

Those do not seem to be necessary, no?

> +
> +	if (must_war_no_source)
> +	  {
> +	    *must_war_no_source = isl_union_map_subtract (*must_war_no_source,
> +						          x_must_war_no_source);
> +	  }
> +	else
> +	  isl_union_map_free (x_must_war_no_source);
> +
> +	if (may_war_no_source)
> +	  {
> +	    *may_war_no_source = isl_union_map_subtract (*may_war_no_source,
> +						         x_may_war_no_source);
> +	  }
> +	else
> +	  isl_union_map_free (x_may_war_no_source);

Could you remove the '{' '}' around the first statement?

> +
> +	if (must_waw)
> +	  *must_waw = isl_union_map_subtract (*must_waw, x_must_waw);
> +	else
> +	  isl_union_map_free (x_must_waw);
> +
> +	if (may_waw)
> +	  *may_waw = isl_union_map_subtract (*may_waw, x_may_waw);
> +	else
> +	  isl_union_map_free (x_may_waw);

Those do not seem to be necessary, no?

> +
> +	if (must_waw_no_source)
> +	  {
> +	    *must_waw_no_source = isl_union_map_subtract (*must_waw_no_source,
> +						          x_must_waw_no_source);
> +	  }
> +	else
> +	  isl_union_map_free (x_must_waw_no_source);
> +
> +	if (may_waw_no_source)
> +	  {
> +	    *may_waw_no_source = isl_union_map_subtract (*may_waw_no_source,
> +						         x_may_waw_no_source);
> +	  }
> +	else
> +	  isl_union_map_free (x_may_waw_no_source);

Could you remove the '{' '}' around the first statement?

Otherwise, the patch looks good.

To commit this patch, could you please provide a ChangeLog entry as well 
as a commit message that _briefly_ describes the findings mentioned in 
you last email and submit this patch to gcc-patches?

Thanks again,
Tobias