Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FindOpenCL: CMake 3.1.0+ #104

Closed
ax3l opened this issue Jul 5, 2017 · 8 comments
Closed

FindOpenCL: CMake 3.1.0+ #104

ax3l opened this issue Jul 5, 2017 · 8 comments
Assignees

Comments

@ax3l
Copy link

ax3l commented Jul 5, 2017

Hi,

the current FindOpenCL.cmake module you ship is unfortunately not very flexible since it finds its *nix dependencies via try_compile rather then checking the lib and include paths.

Since CMake 3.1.0, an official FindOpenCL Module is shipped with CMake, so you can just raise your CMake requirement and remove your script as noted in your todo in its header.

This issue prevents that the OpenCL backends of gearshifft can be build in https://github.com/LLNL/spack/pull/4560

@tdd11235813
Copy link
Contributor

ok, thanks for reporting and the fix. The cmake 3.1.0 will be used in an upcoming PR as you recommended.

@tdd11235813
Copy link
Contributor

try out if cmake3.1 resolves the build problems.

@ax3l
Copy link
Author

ax3l commented Jul 12, 2017

@tdd11235813 thanks, the FindOpenCL works now as expected!

There is only an issue left with the FindclFFT.cmake script - can you please make sure it react properly on hints on the list of paths in the environment variable CMAKE_PREFIX_PATH instead (or on top) of the (deprecated) _ROOT environment vars?

@ax3l
Copy link
Author

ax3l commented Jul 12, 2017

Here is a module of ours that reacts on both the recommended CMAKE_PREFIX_PATH environment hints as well as a (deprecated) _ROOT environment hint: https://github.com/ComputationalRadiationPhysics/cmake-modules/blob/dev/FindSplash.cmake

(its not the most modern script but shows how to interact with find_path and find_library in order to do disturb the default behavior)

@tdd11235813
Copy link
Contributor

updated the cmake file with the file from clfft repository.
cmake on our HPC cluster Taurus now requires

cmake -DCLFFT_ROOT=$CLFFT_ROOT ..

@ax3l
Copy link
Author

ax3l commented Jul 13, 2017

Yes, but ideally a bash export of the form

export CMAKE_PREFIX_PATH=$CLFFT_ROOT:$CMAKE_PREFIX_PATH

cmake ..

should be sufficient, too.

I think you should not use the HINTS option in find_path but only PATHS / PATHS ENV for hints. Also, avoid adding hard-coded system paths such as /usr/local/. That allows commands such as find_library to find libs in a more flexible way since it does not alter its default search paths (such as figuring lib/ and lib64/ out for us by itself).

@tdd11235813
Copy link
Contributor

oh yes, you are right, CMAKE_PREFIX_PATH is the recommended way to point to the paths as it does not rely on lib-specific cmake variables. I provide an update. Thank you for your help again! :)
On a local system I use now:

export CMAKE_PREFIX_PATH=~/software/clFFT-cuda8.0-gcc5.4/:/opt/cuda:$CMAKE_PREFIX_PATH
cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_C_COMPILER=gcc-5 -DCMAKE_CXX_COMPILER=g++-5 ..

@ax3l
Copy link
Author

ax3l commented Jul 13, 2017

fantastic, that's it! ✨
works like a charm, awesome!

please ping we when you draft the next release so we can add the version's sha to spack :)

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

No branches or pull requests

2 participants