[vsipl++] [patch] Support for Cell FFT's up to 4K points.

[Jules Bergmann]

Don McCoy wrote:
> Stefan Seefeld wrote:
>>> -  Fft_impl(Domain<1> const &dom)
>>> +  Fft_impl(Domain<1> const &dom, rtype scale) 
>>> VSIP_THROW((std::bad_alloc))
>>>     
>>
>> Since any throw specifier other than 'throw ()' will lead to worse 
>> code, I
>> think we should not use it if we can, in particular in non-public-API 
>> code.
>>
>>   
> The reason for the above is the the alloc below.  I guess I'm not sure 
> what the right thing to do here is.

It is OK to leave the VSIP_THROW specifier off.


> 
>> Don't enclose any function doing actual work into assert(), as that 
>> will be removed
>> when compiling with -DNDEBUG. Also, let's be careful (and explicit) 
>> with possible
>> return values: some values may be impossible with correct code (and 
>> thus should lead
>> to an abort(), while others may not, and thus should result in an 
>> exception. So, it
>> would be best to explicitely list (named) return values, and possibly 
>> even add a comment
>> that explains what values we check for and what not. (Who knows, may 
>> be ALF's own API
>> evolves, so we have to carefully make adjustments...)
>>
>>   
> Yes, that's true.  I was trying to get more informative messages out of 
> assert(), but that can better be done by naming the return value 
> something informative.  Is that what you meant?

I think Stefan meant that some ALF return codes could be because of 
design errors in our own code (such as passing a null pointer), which 
should be assertion failures, while others could be because of external 
conditions outside of our control (DMA timed out or something), which 
should potentially be exceptions.



>>   
> fft_1d_r2_inv() does the scaling and reordering needed after calling 
> fft_1d_r2() (that is called directly for the forward cases).  It is very 
> little additional code, and necessary to have for fast convolution in 
> order to avoid reloading the kernel.
> 
> That doesn't stop us from putting the individual ones in separate 
> kernels if we choose, but I don't think it will make much of a difference.

I agree.  Forward and inverse do share the bulk of their code, so the 
code size overhead should be minimal.  And applications that do forward 
FFT usually do inverse FFT, so putting them in the same kernel may be a 
net win in some cases.


-- 
Jules Bergmann
CodeSourcery
jules at codesourcery.com
(650) 331-3385 x705