Re: [graphite] Use params for the size of loop blocking tiles

[Albert Cohen <Albert dot Cohen at inria dot fr>]

Sebastian Pop wrote:
> On Sat, Feb 21, 2009 at 04:25, Richard Guenther <rguenther@suse.de> wrote:
> > -  /* TODO: - Calculate the stride size automatically.  */
> > -  int stride_size = 64;
> > +  int stride_size = PARAM_VALUE (PARAM_LOOP_BLOCK_TILE_SIZE);
> >
> > So - why is it now 51 and it was 64?
>
> 51 is more magic than 64.
> 51 is better to avoid cache padding problems and it avoids
> to fill up the cache.  Would a simpler patch like the one
> attached be ok for trunk?
>
> >  And why is the TODO removed?
>
> It shouldn't have been removed.
>
> > I think stride size should be computed based on CPU cache
> > parameters and the number of streams.
>
> Agreed.
>
> > Thus, I don't think a user-controllable param makes much sense here.
>
> Ok.  So we don't want programmers to play with this.

Indeed. Instead of a parameter, and before an integrated default cache 
model is provided, it would be interesting to instrument the decision 
through the ICI. I CC Grigori and Zbigniew so they can start playing 
with it in feedback-directed optimization.

> At least we do want to play with it a little bit, so in the graphite
> branch it makes sense to have this parameter.  I committed a similar
> patch to the branch.

In the short term, such a parameter can indeed help a bit to show that 
Graphite can improve performance!

In the long term, the tile size will result from a much more complex 
computation that depends on the degree of thread-level data parallelism 
exposed among threads, the cache hierarchy, the vectorizer... and some 
features from the SCoP. There is no good solution to this, but many 
people can start working on it now.

Albert