Fix CMake: FFTW Threads #55
Conversation
ax3l
force-pushed
the
fix-fftw-threads
branch
10 times, most recently
from
ed89d3e to
89ce120
Compare
| if (FFTW_OMP_LIBRARIES) | ||
| list(APPEND FFTW_OMP_LIBRARIES "${FFTW_LIBRARIES}") | ||
| list(APPEND FFTW_LIBRARIES "${FFTW_OMP_LIBRARIES}") |
There was a problem hiding this comment.
That was a bug for OMP: swapped the arguments and assigned to an unused variable.
Trying to fix build issues on Linux in Conda-Forge in which pthreads are over-linked and ill-resolved from FFTW.
|
@mkstoyanov this one is ready for review now :) |
| OPTIONAL "fftw3_threads" "fftw3f_threads") | ||
| heffte_find_fftw_libraries( | ||
| PREFIX ${FFTW_ROOT} | ||
| VAR FFTW_OMP_LIBRARIES | ||
| REQUIRED "" | ||
| OPTIONAL "fftw3_omp" "fftw3f_omp") | ||
|
|
||
| if (FFTW_THREADS_LIBRARIES) |
There was a problem hiding this comment.
The current way of listing all libraries will favor searching for OpenMP over Threads. I think we should preserve this behavior, e.g., search for OpenMP first and then look for threads only if the OpenMP libraries were not found.
The main reason is that scientific code using OpenMP is far more common than Threads.
There was a problem hiding this comment.
Makes sense to me, will update tomorrow
| if (FFTW_THREADS_LIBRARIES) | ||
| list(APPEND FFTW_LIBRARIES "${FFTW_THREADS_LIBRARIES}") | ||
| set(THREADS_PREFER_PTHREAD_FLAG TRUE) | ||
| if(WIN32) # Windows has an internal pthreads lib we can use |
There was a problem hiding this comment.
heFFTe doesn't support windows, I don't even know how one will start with MPI on Windows.
| if (@Heffte_ENABLE_FFTW@) | ||
| if (NOT "@FFTW_THREADS_LIBRARIES@" STREQUAL "") | ||
| set(THREADS_PREFER_PTHREAD_FLAG TRUE) | ||
| if(WIN32) # Windows has an internal pthreads lib we can use |
There was a problem hiding this comment.
Same, no need to consider windows.
|
@ax3l I'm currently on vacation, so I'm responding with a delay. I don't like changing the default behavior on system with both threads and openmp, which should be an easy fix. I also don't like superfluous code for features that aren't supported anyway. But other than that, this looks OK. |
|
Effectively done in #65 |
Fix a build issues on Linux in Conda-Forge in which pthreads showed unresolved library paths from FFTW threads variants: conda-forge/staged-recipes#26633
Similar logic as in #47, but for (p)threads.