ROCm: Migrate to native CMake HIP support

CMakeLists.txt

@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 3.18 FATAL_ERROR)
+cmake_minimum_required(VERSION 3.21 FATAL_ERROR)
 
 project(gloo CXX C)
 
@@ -43,6 +43,7 @@ endif()
 option(USE_CUDA "Build with CUDA support" OFF)
 option(GLOO_USE_CUDA_TOOLKIT "Build CUDA with FindCUDATookit.cmake and enable_language(CUDA)" OFF)
 option(GLOO_USE_TORCH_DTYPES "Build CUDA kernels with pytorch dtypes" 0)
+option(USE_ROCM "Build with ROCm support" OFF)
 
 if(MSVC)
   message(STATUS "MSVC detected")

cmake/Dependencies.cmake

@@ -140,35 +140,29 @@ if(USE_ROCM)
   include(cmake/Hip.cmake)
   if(HAVE_HIP)
     include(cmake/Hipify.cmake)
-    list(APPEND HIP_CXX_FLAGS -D__HIP_PLATFORM_HCC__=1)
-    list(APPEND HIP_CXX_FLAGS -DCUDA_HAS_FP16=1)
-    list(APPEND HIP_CXX_FLAGS -D__HIP_NO_HALF_OPERATORS__=1)
-    list(APPEND HIP_CXX_FLAGS -D__HIP_NO_HALF_CONVERSIONS__=1)
-    list(APPEND HIP_CXX_FLAGS -DHIP_VERSION=${HIP_VERSION_MAJOR})
+
+    # Set include directories for HIP targets
+    set(GLOO_HIP_INCLUDE
+      ${hip_INCLUDE_DIRS}
+      $<BUILD_INTERFACE:${HIPIFY_OUTPUT_ROOT_DIR}>
+      $<INSTALL_INTERFACE:include>
+    )
+
+    # Set compile flags for HIP targets
+    set(GLOO_HIP_FLAGS
+      -DCUDA_HAS_FP16=1
+      -D__HIP_NO_HALF_OPERATORS__=1
+      -D__HIP_NO_HALF_CONVERSIONS__=1
+      -DHIP_VERSION=${hip_VERSION_MAJOR}
+      -DUSE_MIOPEN
+    )
     if(NOT WIN32)
-      list(APPEND HIP_CXX_FLAGS -fPIC)
-      list(APPEND HIP_CXX_FLAGS -Wno-shift-count-negative)
-      list(APPEND HIP_CXX_FLAGS -Wno-shift-count-overflow)
-      list(APPEND HIP_CXX_FLAGS -Wno-duplicate-decl-specifier)
+      list(APPEND GLOO_HIP_FLAGS
+        -Wno-shift-count-negative
+        -Wno-shift-count-overflow
+        -Wno-duplicate-decl-specifier
+      )
     endif()
-    list(APPEND HIP_CXX_FLAGS -DUSE_MIOPEN)
-
-    set(HIP_CLANG_FLAGS ${HIP_CXX_FLAGS})
-    # Ask hcc to generate device code during compilation so we can use
-    # host linker to link.
-    list(APPEND HIP_CLANG_FLAGS -fno-gpu-rdc)
-    list(APPEND HIP_CLANG_FLAGS -Wno-defaulted-function-deleted)
-    foreach(gloo_rocm_arch ${GLOO_ROCM_ARCH})
-      list(APPEND HIP_CLANG_FLAGS --offload-arch=${gloo_rocm_arch})
-    endforeach()
-
-    set(GLOO_HIP_INCLUDE ${hip_INCLUDE_DIRS} $<BUILD_INTERFACE:${HIPIFY_OUTPUT_ROOT_DIR}> $<INSTALL_INTERFACE:include> ${GLOO_HIP_INCLUDE})
-
-    # This is needed for library added by hip_add_library (same for hip_add_executable)
-    hip_include_directories(${GLOO_HIP_INCLUDE})
-
-    set(gloo_hip_DEPENDENCY_LIBS ${GLOO_HIP_HCC_LIBRARIES})
-
   else()
     message(WARNING "Not compiling with HIP support. Suppress this warning with -DUSE_ROCM=OFF.")
     set(USE_ROCM OFF)

cmake/Hip.cmake

@@ -1,53 +1,90 @@
 set(HAVE_HIP FALSE)
 
-IF(NOT DEFINED ENV{ROCM_PATH})
-  SET(ROCM_PATH /opt/rocm)
-ELSE()
-  SET(ROCM_PATH $ENV{ROCM_PATH})
-ENDIF()
+# ROCm path configuration (still needed for finding hip package)
+if(NOT DEFINED ENV{ROCM_PATH})
+  set(ROCM_PATH /opt/rocm)
+else()
+  set(ROCM_PATH $ENV{ROCM_PATH})
+endif()
 
-IF(NOT DEFINED ENV{GLOO_ROCM_ARCH})
-  SET(GLOO_ROCM_ARCH gfx906;gfx908;gfx90a)
-ELSE()
-  SET(GLOO_ROCM_ARCH $ENV{GLOO_ROCM_ARCH})
-ENDIF()
+# Architecture handling with the following priority:
+# 1. GLOO_ROCM_ARCH (env or cache) takes priority
+# 2. Fall back to CMAKE_HIP_ARCHITECTURES if set
+# 3. Use defaults if neither is set
+set(GLOO_ROCM_ARCH_DEFAULT "gfx906;gfx908;gfx90a")
 
-# Add HIP to the CMAKE Module Path
-set(CMAKE_MODULE_PATH ${ROCM_PATH}/lib/cmake/hip ${CMAKE_MODULE_PATH})
+if(DEFINED ENV{GLOO_ROCM_ARCH})
+  set(GLOO_ROCM_ARCH $ENV{GLOO_ROCM_ARCH} CACHE STRING "HIP architectures for Gloo")
+elseif(NOT DEFINED GLOO_ROCM_ARCH)
+  if(DEFINED CMAKE_HIP_ARCHITECTURES)
+    set(GLOO_ROCM_ARCH ${CMAKE_HIP_ARCHITECTURES} CACHE STRING "HIP architectures for Gloo")
+  else()
+    set(GLOO_ROCM_ARCH ${GLOO_ROCM_ARCH_DEFAULT} CACHE STRING "HIP architectures for Gloo")
+  endif()
+endif()
 
-# Disable Asserts In Code (Can't use asserts on HIP stack.)
-ADD_DEFINITIONS(-DNDEBUG)
+# Warn if CMAKE_HIP_ARCHITECTURES differs from GLOO_ROCM_ARCH
+if(DEFINED CMAKE_HIP_ARCHITECTURES AND NOT "${CMAKE_HIP_ARCHITECTURES}" STREQUAL "${GLOO_ROCM_ARCH}")
+  message(WARNING "CMAKE_HIP_ARCHITECTURES (${CMAKE_HIP_ARCHITECTURES}) differs from GLOO_ROCM_ARCH (${GLOO_ROCM_ARCH}). Using GLOO_ROCM_ARCH for Gloo targets.")
+endif()
 
-# Find the HIP Package
-find_package(HIP 1.0)
+# Enable HIP language
+enable_language(HIP)
 
-IF(HIP_FOUND)
-  set(HAVE_HIP TRUE)
-
-  set(hip_library_name amdhip64)
-  message("HIP library name: ${hip_library_name}")
-
-  set(CMAKE_HCC_FLAGS_DEBUG ${CMAKE_CXX_FLAGS_DEBUG})
-  set(CMAKE_HCC_FLAGS_RELEASE ${CMAKE_CXX_FLAGS_RELEASE})
-  FIND_LIBRARY(GLOO_HIP_HCC_LIBRARIES ${hip_library_name} HINTS ${ROCM_PATH}/lib)
+# Find HIP package for hip::host target
+find_package(hip REQUIRED)
 
-ENDIF()
+if(hip_FOUND)
+  set(HAVE_HIP TRUE)
+  message(STATUS "Found HIP: ${hip_VERSION}")
+  message(STATUS "GLOO_ROCM_ARCH: ${GLOO_ROCM_ARCH}")
+endif()
 
 ################################################################################
+# Helper function for HIP libraries using CMake native support
 function(gloo_hip_add_library target)
   set(sources ${ARGN})
-  set_source_files_properties(${sources} PROPERTIES HIP_SOURCE_PROPERTY_FORMAT 1)
-  hip_add_library(${target} ${sources} ${GLOO_STATIC_OR_SHARED})
+
+  # Mark non-.hip files as HIP language
+  foreach(source ${sources})
+    get_filename_component(ext ${source} LAST_EXT)
+    if(NOT "${ext}" STREQUAL ".hip")
+      set_source_files_properties(${source} PROPERTIES LANGUAGE HIP)
+    endif()
+  endforeach()
+
+  add_library(${target} ${GLOO_STATIC_OR_SHARED} ${sources})
   target_include_directories(${target} PUBLIC ${GLOO_HIP_INCLUDE})
-  target_compile_options(${target} PUBLIC ${HIP_CXX_FLAGS})
-  target_link_libraries(${target} ${gloo_hip_DEPENDENCY_LIBS})
+  target_link_libraries(${target} PRIVATE hip::host ${gloo_hip_DEPENDENCY_LIBS})
+  target_compile_options(${target} PRIVATE ${GLOO_HIP_FLAGS})
+
+  # Set target-specific properties
+  set_target_properties(${target} PROPERTIES
+    HIP_ARCHITECTURES "${GLOO_ROCM_ARCH}"
+    POSITION_INDEPENDENT_CODE ON
+  )
 endfunction()
 
+# Helper function for HIP executables using CMake native support
 function(gloo_hip_add_executable target)
   set(sources ${ARGN})
-  set_source_files_properties(${sources} PROPERTIES HIP_SOURCE_PROPERTY_FORMAT 1)
-  hip_add_executable(${target} ${sources})
+
+  # Mark non-.hip files as HIP language
+  foreach(source ${sources})
+    get_filename_component(ext ${source} LAST_EXT)
+    if(NOT "${ext}" STREQUAL ".hip")
+      set_source_files_properties(${source} PROPERTIES LANGUAGE HIP)
+    endif()
+  endforeach()
+
+  add_executable(${target} ${sources})
   target_include_directories(${target} PUBLIC ${GLOO_HIP_INCLUDE})
-  target_compile_options(${target} PUBLIC ${HIP_CXX_FLAGS})
-  target_link_libraries(${target} ${gloo_hip_DEPENDENCY_LIBS})
+  target_link_libraries(${target} PRIVATE hip::host ${gloo_hip_DEPENDENCY_LIBS})
+  target_compile_options(${target} PRIVATE ${GLOO_HIP_FLAGS})
+
+  # Set target-specific properties
+  set_target_properties(${target} PROPERTIES
+    HIP_ARCHITECTURES "${GLOO_ROCM_ARCH}"
+    POSITION_INDEPENDENT_CODE ON
+  )
 endfunction()

gloo/CMakeLists.txt

@@ -155,7 +155,7 @@ if(USE_CUDA)
 endif()
 if(USE_ROCM)
   gloo_hip_add_library(gloo_hip ${GLOO_HIP_SRCS})
-  target_link_libraries(gloo_hip gloo)
+  target_link_libraries(gloo_hip PUBLIC gloo)
 endif()
 if(USE_LIBUV)
   target_link_libraries(gloo PRIVATE uv_a)

gloo/benchmark/CMakeLists.txt

@@ -34,7 +34,7 @@ if(USE_ROCM)
     )
 
   gloo_hip_add_executable(benchmark_hip ${GLOO_BENCHMARK_HIP_SRCS})
-  target_link_libraries(benchmark_hip gloo_hip)
+  target_link_libraries(benchmark_hip PRIVATE gloo_hip)
 
   if(GLOO_INSTALL)
     install(TARGETS benchmark_hip DESTINATION ${CMAKE_INSTALL_PREFIX}/bin)

gloo/test/CMakeLists.txt

@@ -79,5 +79,5 @@ if(USE_ROCM)
     )
 
   gloo_hip_add_executable(gloo_test_hip ${GLOO_TEST_HIP_SRCS})
-  target_link_libraries(gloo_test_hip gloo_hip gtest OpenSSL::SSL OpenSSL::Crypto)
+  target_link_libraries(gloo_test_hip PRIVATE gloo_hip gtest OpenSSL::SSL OpenSSL::Crypto)
 endif()