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

Check for HIP versions in min/max range #2196

Merged
merged 3 commits into from
Nov 29, 2023

Conversation

StewMH
Copy link
Contributor

@StewMH StewMH commented Nov 27, 2023

With the current logic, cmake checks first for HIP 5.5, then for 5.0:

CMake Warning at cmake/alpakaCommon.cmake:506 (find_package):
  By not providing "Findhip.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "hip", but
  CMake did not find one.

  Could not find a package configuration file provided by "hip" (requested
  version 5.5) with any of the following names:

    hipConfig.cmake
    hip-config.cmake

  Add the installation prefix of "hip" to CMAKE_PREFIX_PATH or set "hip_DIR"
  to a directory containing one of the above files.  If "hip" provides a
  separate development package or SDK, be sure it has been installed.
Call Stack (most recent call first):
  CMakeLists.txt:94 (include)


-- Could not find HIP v5.5. Now searching for HIP v5.0
CMake Warning at cmake/alpakaCommon.cmake:509 (find_package):
  By not providing "Findhip.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "hip", but
  CMake did not find one.

  Could not find a package configuration file provided by "hip" (requested
  version 5.0) with any of the following names:

    hipConfig.cmake
    hip-config.cmake

  Add the installation prefix of "hip" to CMAKE_PREFIX_PATH or set "hip_DIR"
  to a directory containing one of the above files.  If "hip" provides a
  separate development package or SDK, be sure it has been installed.
Call Stack (most recent call first):
  CMakeLists.txt:94 (include)


CMake Error at cmake/alpakaCommon.cmake:514 (message):
  Optional alpaka dependency HIP could not be found!
Call Stack (most recent call first):
  CMakeLists.txt:94 (include)

I think the behaviour should give an inclusive range but I don't immediately have a way to test this (only having a somewhat broken HIP 5.1 installation).

message(STATUS "Could not find HIP v${_alpaka_HIP_MAX_VER}. Now searching for HIP v${_alpaka_HIP_MIN_VER}")
find_package(hip "${_alpaka_HIP_MIN_VER}")
endif()
find_package(hip "${_alpaka_HIP_MIN_VER}...<${_alpaka_HIP_MAX_VER}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, this feature has been available since CMake 3.19 and we forgot about it after we required a newer version of CMake.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

< should be removed else the max version will be excluded. see https://cmake.org/cmake/help/latest/command/find_package.htm

cmake/alpakaCommon.cmake Show resolved Hide resolved
@psychocoderHPC psychocoderHPC added Type:Refactoring Type:Install installation & packaging labels Nov 27, 2023
Copy link
Member

@SimeonEhrig SimeonEhrig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could verify it with ROCm 5.5 and ROCm 5.7 on our dev system.

cmake/alpakaCommon.cmake Show resolved Hide resolved
cmake/alpakaCommon.cmake Outdated Show resolved Hide resolved
Co-authored-by: Simeon Ehrig <[email protected]>
Copy link
Member

@SimeonEhrig SimeonEhrig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked it again locally. For unknown reason the warning with HIP 5.5 and different CMake versions appears only on my dev system. The CI is fine and does not trow a warning. Also the documentation says, that the upper version should be included. Therefor I will merge it :-)

@psychocoderHPC psychocoderHPC merged commit 4d1a221 into alpaka-group:develop Nov 29, 2023
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type:Install installation & packaging Type:Refactoring
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants