Skip to content

Commit

Permalink
Disable HIP_PLATFORM auto-detect if already defined.
Browse files Browse the repository at this point in the history
This code appears to be old copy-pasta and is effectively just a no-op if -DHIP_PLATFORM=amd is passed explicitly. It has a number of nit-picky checks which will fail in this case for no real reason.

This is actually worse than that because it leaks global variables that establish a different default value/scoping vs the definitions in the inner hipamd/CMakeLists.txt. I opted to rewrite this section to scope the detection logic and keep it from leaking with respect to the inner build file. The detection logic is still not great but at least will not actively cause damage or make control flow hard to reason about. It seems like this is for legacy/compatibility so I opted to simply sequester it vs applying more diligence to it.

Also fixes a bug in hip-config.cmake where it enforces that hipcc exists even if not taking the install branch in the code above.

See: nod-ai/TheRock#21
  • Loading branch information
stellaraccident committed Jan 22, 2025
1 parent 1b9c177 commit 832a770
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 26 deletions.
43 changes: 27 additions & 16 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -40,40 +40,51 @@ endif()
#############
# Build steps
#############
if(CLR_BUILD_HIP)
if (UNIX)
set(HIPCC_EXECUTABLE "hipcc")
set(HIPCONFIG_EXECUTABLE "hipconfig")
else()
set(HIPCC_EXECUTABLE "hipcc.exe")
set(HIPCONFIG_EXECUTABLE "hipconfig.exe")
endif()

# Attempt to auto-detect HIP_PLATFORM by interrogating hipconfig. This is kept
# for compatibility: users are advised to pass HIP_PLATFORM explicitly.
# Sets the HIP_PLATFORM variable in the parent scope.
function(_hip_clr_auto_detect_hip_platform)
if (UNIX)
set(HIPCC_EXECUTABLE "hipcc")
set(HIPCONFIG_EXECUTABLE "hipconfig")
else()
set(HIPCC_EXECUTABLE "hipcc.exe")
set(HIPCONFIG_EXECUTABLE "hipconfig.exe")
endif()

# Set default HIPCC_BIN_DIR to /opt/rocm/bin
if(NOT DEFINED HIPCC_BIN_DIR AND UNIX)
set(HIPCC_BIN_DIR "/opt/rocm/bin" CACHE STRING "Default hipcc directory on linux.")
set(HIPCC_BIN_DIR "/opt/rocm/bin")
endif()
message(STATUS "HIPCC Binary Directory: ${HIPCC_BIN_DIR}")
message(STATUS "Auto-detect HIP_PLATFORM from HIPCC Binary Directory: ${HIPCC_BIN_DIR}")

if(NOT EXISTS ${HIPCC_BIN_DIR}/${HIPCONFIG_EXECUTABLE})
message(FATAL_ERROR "Please pass hipcc/build or hipcc/bin using -DHIPCC_BIN_DIR.")
endif()

message(STATUS "HIP Common Directory: ${HIP_COMMON_DIR}")
if(NOT DEFINED HIP_COMMON_DIR)
message(FATAL_ERROR "Please pass HIP using -DHIP_COMMON_DIR. HIP_COMMON_DIR is incorrect")
endif()
# Determine HIP_PLATFORM
if(NOT DEFINED HIP_PLATFORM)
if(NOT DEFINED ENV{HIP_PLATFORM})
execute_process(COMMAND ${HIPCC_BIN_DIR}/${HIPCONFIG_EXECUTABLE} --platform
OUTPUT_VARIABLE HIP_PLATFORM
OUTPUT_VARIABLE _detected_hip_platform
OUTPUT_STRIP_TRAILING_WHITESPACE)
set(HIP_PLATFORM "${_detected_hip_platform}" PARENT_SCOPE)
else()
set(HIP_PLATFORM $ENV{HIP_PLATFORM} CACHE STRING "HIP Platform")
set(HIP_PLATFORM "$ENV{HIP_PLATFORM}" PARENT_SCOPE)
endif()
endif()
endfunction()
if(CLR_BUILD_HIP)
message(STATUS "HIP Common Directory: ${HIP_COMMON_DIR}")
if(NOT DEFINED HIP_COMMON_DIR)
message(FATAL_ERROR "Please pass HIP using -DHIP_COMMON_DIR. HIP_COMMON_DIR is incorrect")
endif()
if(NOT DEFINED HIP_PLATFORM)
_hip_clr_auto_detect_hip_platform()
endif()
endif()

if(CLR_BUILD_HIP)
option(BUILD_SHARED_LIBS "Build the shared library" ON)
if (NOT BUILD_SHARED_LIBS)
Expand Down
5 changes: 4 additions & 1 deletion hipamd/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ if(MSVC)
set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} /DEBUG:FULL")
endif()

set(HIPCC_BIN_DIR "" CACHE STRING "HIPCC and HIPCONFIG binary directories")
# Set default HIPCC_BIN_DIR to /opt/rocm/bin
set(HIPCC_BIN_DIR "/opt/rocm/bin" CACHE STRING "HIPCC and HIPCONFIG binary directories")

if(__HIP_ENABLE_PCH)
set(_pchStatus 1)
Expand Down Expand Up @@ -393,9 +394,11 @@ endif()
install(FILES ${PROJECT_BINARY_DIR}/include/hip/hip_version.h
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/hip)

set(HIP_INSTALLS_HIPCC OFF)
if (NOT ${HIPCC_BIN_DIR} STREQUAL "")
file(TO_CMAKE_PATH "${HIPCC_BIN_DIR}" HIPCC_BIN_DIR)
if(EXISTS ${HIPCC_BIN_DIR})
set(HIP_INSTALLS_HIPCC ON)
install(PROGRAMS ${HIPCC_BIN_DIR}/${HIPCC_EXECUTABLE} DESTINATION bin)
install(PROGRAMS ${HIPCC_BIN_DIR}/${HIPCONFIG_EXECUTABLE} DESTINATION bin)
install(PROGRAMS ${HIPCC_BIN_DIR}/hipcc.pl DESTINATION bin)
Expand Down
21 changes: 12 additions & 9 deletions hipamd/hip-config.cmake.in
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,14 @@ set_and_check( hip_INCLUDE_DIR "@PACKAGE_INCLUDE_INSTALL_DIR@" )
set_and_check( hip_INCLUDE_DIRS "${hip_INCLUDE_DIR}" )
set_and_check( hip_LIB_INSTALL_DIR "@PACKAGE_LIB_INSTALL_DIR@" )
set_and_check( hip_BIN_INSTALL_DIR "@PACKAGE_BIN_INSTALL_DIR@" )
if (WIN32)
set_and_check(hip_HIPCC_EXECUTABLE "${hip_BIN_INSTALL_DIR}/hipcc.exe")
set_and_check(hip_HIPCONFIG_EXECUTABLE "${hip_BIN_INSTALL_DIR}/hipconfig.exe")
else()
set_and_check(hip_HIPCC_EXECUTABLE "${hip_BIN_INSTALL_DIR}/hipcc")
set_and_check(hip_HIPCONFIG_EXECUTABLE "${hip_BIN_INSTALL_DIR}/hipconfig")
if("@HIP_INSTALLS_HIPCC@")
if (WIN32)
set_and_check(hip_HIPCC_EXECUTABLE "${hip_BIN_INSTALL_DIR}/hipcc.exe")
set_and_check(hip_HIPCONFIG_EXECUTABLE "${hip_BIN_INSTALL_DIR}/hipconfig.exe")
else()
set_and_check(hip_HIPCC_EXECUTABLE "${hip_BIN_INSTALL_DIR}/hipcc")
set_and_check(hip_HIPCONFIG_EXECUTABLE "${hip_BIN_INSTALL_DIR}/hipconfig")
endif()
endif()

if(NOT DEFINED HIP_PLATFORM)
Expand Down Expand Up @@ -140,6 +142,7 @@ set(HIP_LIB_INSTALL_DIR ${hip_LIB_INSTALL_DIR})
set(HIP_BIN_INSTALL_DIR ${hip_BIN_INSTALL_DIR})
set(HIP_LIBRARIES ${hip_LIBRARIES})
set(HIP_LIBRARY ${hip_LIBRARY})
set(HIP_HIPCC_EXECUTABLE ${hip_HIPCC_EXECUTABLE})
set(HIP_HIPCONFIG_EXECUTABLE ${hip_HIPCONFIG_EXECUTABLE})

if("@@HIP_INSTALLS_HIPCC@@")
set(HIP_HIPCC_EXECUTABLE ${hip_HIPCC_EXECUTABLE})
set(HIP_HIPCONFIG_EXECUTABLE ${hip_HIPCONFIG_EXECUTABLE})
endif()

0 comments on commit 832a770

Please sign in to comment.