Skip to content

Commit ad0a9af

Browse files
committed
ROCm: use native CMake HIP support
CMake's native HIP support has become the recommended way to add HIP code into a project (see [here](https://rocm.docs.amd.com/en/docs-6.0.0/conceptual/cmake-packages.html#using-hip-in-cmake)). This PR makes the following changes: 1. The environment variable `HIPCXX` or CMake option `CMAKE_HIP_COMPILER` should be used to specify the HIP compiler. Notably this shouldn't be `hipcc`, but ROCm's clang, which usually resides in `$ROCM_PATH/llvm/bin/clang`. Previously this was control by `CMAKE_C_COMPILER` and `CMAKE_CXX_COMPILER`. 2. CMake option `CMAKE_HIP_ARCHITECTURES` is used to control the GPU architectures to build for. Previously this was controled by `GPU_TARGETS`. The most important part about this PR is that this separation of HIP compiler and the C/C++ compiler allows user to choose a different C/C++ compiler if desired, compared to the current situation where when building for ROCm, everything must be built with ROCm's clang. Makefile is unchanged. Please let me know if we want to be consistent on variables' naming because Makefile still uses `GPU_TARGETS` to control architectures to build for, but I feel like setting `CMAKE_HIP_ARCHITECTURES` is a bit awkward when you're calling `make`.
1 parent c2101a2 commit ad0a9af

File tree

2 files changed

+16
-18
lines changed

2 files changed

+16
-18
lines changed

.devops/nix/package.nix

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -207,18 +207,18 @@ effectiveStdenv.mkDerivation (
207207
)
208208
]
209209
++ optionals useRocm [
210-
(cmakeFeature "CMAKE_C_COMPILER" "hipcc")
211-
(cmakeFeature "CMAKE_CXX_COMPILER" "hipcc")
212-
213-
# Build all targets supported by rocBLAS. When updating search for TARGET_LIST_ROCM
214-
# in https://github.com/ROCmSoftwarePlatform/rocBLAS/blob/develop/CMakeLists.txt
215-
# and select the line that matches the current nixpkgs version of rocBLAS.
216-
# Should likely use `rocmPackages.clr.gpuTargets`.
217-
"-DAMDGPU_TARGETS=gfx803;gfx900;gfx906:xnack-;gfx908:xnack-;gfx90a:xnack+;gfx90a:xnack-;gfx940;gfx941;gfx942;gfx1010;gfx1012;gfx1030;gfx1100;gfx1101;gfx1102"
210+
(cmakeFeature "CMAKE_HIP_COMPILER" "${rocmPackages.llvm.clang}/bin/clang")
211+
(cmakeFeature "CMAKE_HIP_ARCHITECTURES" (builtins.concatStringsSep ";" rocmPackages.clr.gpuTargets))
218212
]
219213
++ optionals useMetalKit [ (lib.cmakeFeature "CMAKE_C_FLAGS" "-D__ARM_FEATURE_DOTPROD=1") ]
220214
++ optionals useBlas [ (lib.cmakeFeature "LLAMA_BLAS_VENDOR" "OpenBLAS") ];
221215

216+
# Environment variables needed for ROCm
217+
env = optionals useRocm {
218+
ROCM_PATH = "${rocmPackages.clr}";
219+
HIP_DEVICE_LIB_PATH = "${rocmPackages.rocm-device-libs}/amdgcn/bitcode";
220+
};
221+
222222
# TODO(SomeoneSerge): It's better to add proper install targets at the CMake level,
223223
# if they haven't been added yet.
224224
postInstall = ''

CMakeLists.txt

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -486,16 +486,14 @@ if (LLAMA_VULKAN)
486486
endif()
487487

488488
if (LLAMA_HIPBLAS)
489-
list(APPEND CMAKE_PREFIX_PATH /opt/rocm)
490-
491-
if (NOT ${CMAKE_C_COMPILER_ID} MATCHES "Clang")
492-
message(WARNING "Only LLVM is supported for HIP, hint: CC=/opt/rocm/llvm/bin/clang")
493-
endif()
494-
495-
if (NOT ${CMAKE_CXX_COMPILER_ID} MATCHES "Clang")
496-
message(WARNING "Only LLVM is supported for HIP, hint: CXX=/opt/rocm/llvm/bin/clang++")
489+
if ($ENV{ROCM_PATH})
490+
set(ROCM_PATH $ENV{ROCM_PAT})
491+
else()
492+
set(ROCM_PATH /opt/rocm)
497493
endif()
494+
list(APPEND CMAKE_PREFIX_PATH ${ROCM_PATH})
498495

496+
enable_language(HIP)
499497
find_package(hip REQUIRED)
500498
find_package(hipblas REQUIRED)
501499
find_package(rocblas REQUIRED)
@@ -523,13 +521,13 @@ if (LLAMA_HIPBLAS)
523521
add_compile_definitions(GGML_CUDA_MMV_Y=${LLAMA_CUDA_MMV_Y})
524522
add_compile_definitions(K_QUANTS_PER_ITERATION=${LLAMA_CUDA_KQUANTS_ITER})
525523

526-
set_source_files_properties(ggml-cuda.cu PROPERTIES LANGUAGE CXX)
524+
set_source_files_properties(ggml-cuda.cu PROPERTIES LANGUAGE HIP)
527525

528526
if (LLAMA_STATIC)
529527
message(FATAL_ERROR "Static linking not supported for HIP/ROCm")
530528
endif()
531529

532-
set(LLAMA_EXTRA_LIBS ${LLAMA_EXTRA_LIBS} hip::device PUBLIC hip::host roc::rocblas roc::hipblas)
530+
set(LLAMA_EXTRA_LIBS ${LLAMA_EXTRA_LIBS} PUBLIC hip::host roc::rocblas roc::hipblas)
533531
endif()
534532

535533
if (LLAMA_SYCL)

0 commit comments

Comments
 (0)