[vsipl++] patch: support sal-fft

[Stefan Seefeld]

Jules Bergmann wrote:

>> +#elif defined(VSIP_IMPL_SAL_FFT)
>> +    // In some contexts, SAL destroys the input data itself, and 
>> sometimes
>> +    // we have to modify it to 'pack' data into the format SAL expects
>> +    // (see SAL Tutorial for details).
>> +    // Therefor, we always copy the input.
>> +    static const bool  must_copy = true;
> 
> 
> For complex-to-complex FFTs, there are non-clobbering variants of some 
> of the functions (fft_coptx instead of fft_copx) that we should use 
> instead of copying the data.  For real-to-complex and complex-to-real it 
> still might be necessary to copy.

I agree. However, I believe the required logic will be much simpler after
our fft code is modularized, i.e. backends become slightly better encapsulated.
For example, SAL has its own memory allocator that we might want to use to
allocate temporaries used during fft...

Also, the packing / unpacking issue only exists on the complex side of a
real fft, so copying is only needed for the real inverse FFTs, but not
forward.


>> +  assert(0 && "Sorry, operation not yet supported for this type !");
>> +  // TBD
> 
> 
> This shouldn't make it out into a release, but to be on the safe size, 
> you should really say:
> 
>     VSIP_IMPL_THROW(vsip::impl::unimplemented("..."));

I agree, though, with a dispatcher in place, this code would be unreachable.

>> +  FFT_setup setup = reinterpret_cast<FFT_setup>(self.plan_);
>> +  float *out_ = reinterpret_cast<float*>(out);
> 
> 
> We reserve the "_" suffix for member variables.  Perhaps you could call 
> the parameter "arg_out" and then use "out" for the cast?

Fine.

Thanks for the review !

		Stefan