Segfaults in avx2 methods about concrete

[aPere3]

Hi @vthib ,

Can you provide a pointer that explicitly states that fftw_malloc returns arrays aligned on 16 bits ?

I tried to find one and could not.

[vthib]

FFTW allocates using this routine: https://github.com/FFTW/fftw3/blob/master/kernel/kalloc.c#L71

It uses MIN_ALIGNMENT, defined here: https://github.com/FFTW/fftw3/blob/master/kernel/ifftw.h#L133

So it seems to use 32 bytes alignment if a certain number of conditions is met. On my distrib (arch), fftw is compiled with --enable-avx, which makes this 32 bytes alignment used afaict. But in rust-fftw, this is probably not the case.

Anyway, this seems quite hard to enforce, unless a patch is done in rust-fftw to enable avx if fftw is compiled, + an assert is added. Even with this solution it seems quite brittle.

Imho, the best solution would be to drop the use of AlignedVec from fftw, and use a custom new type, using #[repr(C, align(32)].

This also raises an interesting issue, if we want the best possible performance, fftw should be compiled with aggressive options, which does not seem to be the case

[vthib]

I can do a PR with:

• a in-house AlignedVec where we control the alignment (forced on 32 bytes), that is a drop-in replacement for the fftw AlignedVec everywhere
• asserts in the avx method to ensure alignment is correct.

Would that be ok for you?

[aPere3]

Hey @vthib,

We are currently forking the rust_fftw wrapper, so I would prefer us to arrange for that in this new wrapper.

[vthib]

There would be a big benefit in doing it in concrete, it would decouple concrete from fftw. Right now we have functions that depends on AlignedVec for optimizations, but that isn't related to fft at all. Having AlignedVec in concrete would remove this dependency, making it much easier to use another fft library, for benchmarking or targeting wasm for example.
The Rust implem for AlignedVec is actually quite clean as well

[aPere3]

The concrete-fftw crate now forces the alignment of the AlignedVec structure to 32 bytes, which solves the segfault. We may consider adding an AlignedVecto concrete when we refactor it to support rustfft.

Closing.