Re: [PATCH][Graphite] Fix PR50561

[Tobias Grosser <grosser at fim dot uni-passau dot de>]

On 01/09/2012 04:31 PM, Richard Guenther wrote:
> We have two Graphite related release blockers for 4.7.  The
> following patch fixes the first one.

Hi Richard,

thanks a lot for working on this.

> The asserts in
>
> static inline ppl_dimension_type
> psct_dynamic_dim (poly_bb_p pbb, graphite_dim_t level)
> {
>    graphite_dim_t result = 1 + 2 * level;
>
>    gcc_assert (result<  pbb_nb_scattering_transform (pbb));
>    return result;
> }
>
> static inline ppl_dimension_type
> psct_static_dim (poly_bb_p pbb, graphite_dim_t level)
> {
>    graphite_dim_t result = 2 * level;
>
>    gcc_assert (result<  pbb_nb_scattering_transform (pbb));
>    return result;
> }
>
> are not consistent in case both a dynamic and static dimension should
> always exist at the same time.  Changing psct_dynamic_dim to use<=
> fixes the issue.

I don't think this is the right fix. The asserts seem to be correct.
The problem is actually already shown when you run cc1.

[CLooG] INFO: 2 dimensions (over 3) are scalar.
[CLooG] WARNING: not enough constraints for 1 scattering function(s).

This means, the schedule/scattering that we defined was not sufficient
enough to define the execution order. It is underspecified. The result 
is that CLooG generates the following code:

for (c2=-250;c2<=1535;c2++) {
  for (i=max(0,ceild(51*c2,307))
       i<=min(255,floord(51*c2+12800,307));i++) {
    S1(i);
  }
}

c2 is a scattering variable, but 'i' is actually a original loop 
dimension. I believe graphite does not expect this dimension to show up.
Somehow the scattering that is defined is incorrect. I suspect the loop 
flattening has some problems with the inequalities introduced by the 
strip-mining.

I will look into this.

> Tobias, you are the only active(?) Graphite reviewer left.

I am, though i have currently not a lot of time to write patches myself.

I am still looking into finishing Sebastians patches. Afterwards it 
should be simple to use the new polyhedral optimizer as it was recently 
added to isl.

Tobi