* Re: asking your advice about bug
[not found] ` <2dfaa7b0-4a9e-443e-8307-394083fe9777@googlegroups.com>
@ 2014-02-24 9:17 ` Tobias Grosser
[not found] ` <13a29a46-3826-436e-9f6c-81fa9e9edbd6@googlegroups.com>
2014-03-06 13:28 ` Tobias Grosser
1 sibling, 1 reply; 6+ messages in thread
From: Tobias Grosser @ 2014-02-24 9:17 UTC (permalink / raw)
To: Roman Gareev, gcc-graphite; +Cc: GCC Mailing List
On 02/17/2014 06:50 PM, Roman Gareev wrote:
>
>
> Hi Tobias,
>
>
> thanks for the answer!
Hi Roman,
sorry for missing this mail.
>
> 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)
>
Interesting analysis.
> 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.
Yes, this would be a 'solution'. However, I am in fact surprised that
those variables are NULL at all. Do you have an idea why this is the
case? Understanding this would help to understand if the patch you
propose is actually the right solution or if it is just hiding a
previous bug.
Cheers,
Tobias
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: asking your advice about bug
[not found] ` <2dfaa7b0-4a9e-443e-8307-394083fe9777@googlegroups.com>
2014-02-24 9:17 ` Tobias Grosser
@ 2014-03-06 13:28 ` Tobias Grosser
1 sibling, 0 replies; 6+ messages in thread
From: Tobias Grosser @ 2014-03-06 13:28 UTC (permalink / raw)
To: Roman Gareev, gcc-graphite; +Cc: GCC Mailing List
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
^ permalink raw reply [flat|nested] 6+ messages in thread