Skip to content

cmake: Set SIMD flags on appropriate source files only#322

Merged
matteo-frigo merged 1 commit intoFFTW:masterfrom
ryanvolz:cmake-simd-runtime-optional
Apr 7, 2023
Merged

cmake: Set SIMD flags on appropriate source files only#322
matteo-frigo merged 1 commit intoFFTW:masterfrom
ryanvolz:cmake-simd-runtime-optional

Conversation

@ryanvolz
Copy link
Copy Markdown
Contributor

@ryanvolz ryanvolz commented Apr 6, 2023

This addresses a shortcoming observed with the Windows FFTW package on conda-forge, which uses CMake to build with MSVC. Those packages are built with SSE2 and AVX instructions enabled, and contrary to the desired behavior, users with processors that don't have AVX support have been reporting that the library fails to load. I finally tracked this down to the fact that when using CMake the entire library is built with the enabled SIMD flags, rather than just the specific SIMD files as with configure/make. Thus the compiler was inserting SIMD instructions elsewhere in the library, not gated by the check for whether the runtime CPU supported them.

This PR modifies the CMake build so that it applies the SIMD flags only on the appropriate source files as with configure/make. Patching this fix into the conda-forge package allowed me to use the library on a machine without AVX support while the package still had it enabled.

Doing this matches the behavior of the configure script and makefiles.

Previously, enabling a set of SIMD instructions with the CMake build
would build the whole library with that flag enabled, allowing the
compiler to optimize by using that set of instructions anywhere. That
would then require that any SIMD instructions enabled at build time
would have to be present at run time. This change enables the intended
behavior of using SIMD instructions only when they are detected at run
time.
@matteo-frigo matteo-frigo merged commit 11d93a8 into FFTW:master Apr 7, 2023
@ryanvolz ryanvolz deleted the cmake-simd-runtime-optional branch April 7, 2023 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants