| Message ID | alpine.LNX.2.00.1201091629520.4999@zhemvz.fhfr.qr |
|---|---|
| State | New |
| Headers | show |
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
Index: gcc/graphite-poly.h =================================================================== --- gcc/graphite-poly.h (revision 183013) +++ gcc/graphite-poly.h (working copy) @@ -656,7 +656,7 @@ psct_dynamic_dim (poly_bb_p pbb, graphit { graphite_dim_t result = 1 + 2 * level; - gcc_assert (result < pbb_nb_scattering_transform (pbb)); + gcc_assert (result <= pbb_nb_scattering_transform (pbb)); return result; } Index: gcc/testsuite/gcc.dg/graphite/pr50561.c =================================================================== --- gcc/testsuite/gcc.dg/graphite/pr50561.c (revision 0) +++ gcc/testsuite/gcc.dg/graphite/pr50561.c (revision 0) @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -floop-flatten -floop-strip-mine" } */ + +void f (unsigned *s) +{ + int n; + for (n = 0; n < 256; n++) + s[n] = 0; +}
We have two Graphite related release blockers for 4.7. The following patch fixes the first one. 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. Tobias, you are the only active(?) Graphite reviewer left. Ok? Thanks, Richard. 2012-01-09 Richard Guenther <rguenther@suse.de> PR tree-optimization/50561 * graphite-poly.h (psct_dynamic_dim): Fix assert. * gcc.dg/graphite/pr50561.c: New testcase.