ROCm / hipamd

34 stars 37 forks source link

Inconsistent CMake module names #39

Open pvelesko opened 2 years ago

pvelesko commented 2 years ago

Documentation https://rocmdocs.amd.com/en/latest/Installation_Guide/Using-CMake-with-AMD-ROCm.html states to use find_package(hip) but HIP provides FindHIP.cmake, not Findhip.cmake as seen here: https://github.com/ROCm-Developer-Tools/HIP/blob/develop/cmake/FindHIP.cmake

resulting in the following error:

[cmake] CMake Error at CMakeLists.txt:8 (find_package):
[cmake]   By not providing "Findhip.cmake" in CMAKE_MODULE_PATH this project has
[cmake]   asked CMake to find a package configuration file provided by "hip", but
[cmake]   CMake did not find one.
pvelesko commented 1 year ago

HIP provides the MODULE way of satisfying find_package whereas HIPAMD provides the CONFIG way. The CONFIG method is the newer one while MODULE is the legacy way of using CMake. CONFIG method provides hip::host and hip::device targets which are documented in rocmdocs.amd.com/en/latest/Installation_Guide/Using-CMake-with-AMD-ROCm.html

MODULE path implements hip_add_executable macros which can be used for linking HIP applications but from my understanding this has been deprecated in HIP.

pvelesko commented 1 year ago

I think the documentation should specify find_package(hip CONFIG REQUIRED) to make it less confusing or the MODULE approach should be removed/renamed

AngryLoki commented 6 months ago

the MODULE approach should be removed/renamed

Note that some projects like pytorch depend on FindHIP.cmake and automatically hipify CMakeLists.txt with simple replacements like CUDA_ADD_EXECUTABLE -> HIP_ADD_EXECUTABLE, find_package(CUDA ... -> find_package(HIP ..., and even though FindCUDA is deprecated since CMake 3.10, many projects still use CUDA_ADD_EXECUTABLE and HIP_ADD_EXECUTABLE. So deprecation is fine, but not complete removal.