[vsipl++] [patch] Fast convolution expression templates

[Don McCoy]

Jules Bergmann wrote:
> > +  static bool const ct_valid = Type_equal<T, std::complex<float> 
> >::value;
>
> [1] We should enforce that MatBlockT::value_type == complex<float>
>
What about CoeffsMatBlockT?  And isn't the type of MatBlockT at least 
captured somehow as part of fft::Fft_return_functor?

Come to think of it, what about VecBlockT in the previous expression?  
These are a little tricky -- I could stand to solidify my understanding 
a bit here.  :)


> > +
> > +  static bool rt_valid(DstBlock& dst, SrcBlock const& /*src*/)
> > +  {
> > +    return fconv_type::rt_valid_size(dst.size(2, 1));
>
> [2] Do we need to enforce any other run-time constaints?  Ext_data 
> access OK?
> Unit-stride?  etc.
>
> Or are those handled by Fastconv_base?
Probably both, and no.  The second is through Ext_cost or similar?

>
> We should definitely check FFT scaling (see ifdef'd out check in
> opt/expr/eval_fastconv).  IIRC that check was expensive for some
> reason, although I believe it shouldn't be.  If it proves to be
> expensive here, we can leave it out for the time being.
>
So do we need those checks in *all* evaluators then?  And on that note, 
do we want to add evaluators for the Fc_expr_tag as well (so it will 
work for non Cell/B.E. platforms)?


>
> > Index: benchmarks/fastconv.cpp
> > ===================================================================
>
> > -    double error = error_db(data, chk);
> > +    double error = error_db(LOCAL(data), LOCAL(chk));
>
> [3] The global version failed to compile right?  I think I've run
> across this too.  There is a bug in error_db and/or the reductions
> that I need to track down.  Your work around is better than mine, I
> just commented out the test altogether! :)
>
Yes.  At least I think so.  It is the same bug that happens when 
PARALLEL_FASTCONV is forced to 0, IIRC.


-- 
Don McCoy
don (at) CodeSourcery
(888) 776-0262 / (650) 331-3385, x712