Skip to content

Commit ca4ff9c

Browse files
committed
ROCm: use native CMake HIP support
Supercedes ggml-org#4024 and ggml-org#4813. 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`. 3. Updated the Nix recipe to account for these new changes. 4. The GPU targets to build against in the Nix recipe is now consistent with the supported GPU targets in nixpkgs. The most important part about this PR is the separation of the HIP compiler and the C/C++ compiler. This allows users to choose a different C/C++ compiler if desired, compared to the current situation where when building for ROCm support, everything must be compiled 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`.~~ Makefile used `GPU_TARGETS` but the README says to use `AMDGPU_TARGETS`. For consistency with CMake, all usage of `GPU_TARGETS` in Makefile has been updated to `AMDGPU_TARGETS`. Thanks to the suggestion of @jin-eld, to maintain backwards compatibility (and not break too many downstream users' builds), if `CMAKE_CXX_COMPILER` ends with `hipcc`, then we still compile using the original behavior and emit a warning that recommends switching to the new HIP support. Similarly, if `AMDGPU_TARGETS` is set but `CMAKE_HIP_ARCHITECTURES` is not, then we forward `AMDGPU_TARGETS` to `CMAKE_HIP_ARCHITECTURES` to ease the transition to the new HIP support. Signed-off-by: Gavin Zhao <[email protected]>
1 parent 4e9a7f7 commit ca4ff9c

File tree

5 files changed

+78
-21
lines changed

5 files changed

+78
-21
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 = ''

.github/workflows/build.yml

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,33 @@ jobs:
167167
cmake -DLLAMA_VULKAN=ON ..
168168
cmake --build . --config Release -j $(nproc)
169169
170+
ubuntu-22-cmake-hip:
171+
runs-on: ubuntu-22.04
172+
container: rocm/dev-ubuntu-22.04:6.0.2-complete
173+
174+
steps:
175+
- name: Clone
176+
id: checkout
177+
uses: actions/checkout@v3
178+
179+
- name: Dependencies
180+
id: depends
181+
run: |
182+
sudo apt-get update
183+
sudo apt-get install -y build-essential git cmake rocblas-dev hipblas-dev
184+
185+
- name: Build with native CMake HIP support
186+
id: cmake_build
187+
run: |
188+
cmake -B build -S . -DLLAMA_HIPBLAS=ON -DCMAKE_HIP_COMPILER="$(hipconfig -l)/clang"
189+
cmake --build build --config Release -j $(nproc)
190+
191+
- name: Build with legacy HIP support
192+
id: cmake_build_legacy_hip
193+
run: |
194+
cmake -B build2 -S . -DLLAMA_HIPBLAS=ON -DCMAKE_C_COMPILER=hipcc -DCMAKE_CXX_COMPILER=hipcc
195+
cmake --build build2 --config Release -j $(nproc)
196+
170197
ubuntu-22-cmake-sycl:
171198
runs-on: ubuntu-22.04
172199

CMakeLists.txt

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -497,16 +497,26 @@ if (LLAMA_VULKAN)
497497
endif()
498498

499499
if (LLAMA_HIPBLAS)
500-
list(APPEND CMAKE_PREFIX_PATH /opt/rocm)
501-
502-
if (NOT ${CMAKE_C_COMPILER_ID} MATCHES "Clang")
503-
message(WARNING "Only LLVM is supported for HIP, hint: CC=/opt/rocm/llvm/bin/clang")
500+
if ($ENV{ROCM_PATH})
501+
set(ROCM_PATH $ENV{ROCM_PATH})
502+
else()
503+
set(ROCM_PATH /opt/rocm)
504504
endif()
505+
list(APPEND CMAKE_PREFIX_PATH ${ROCM_PATH})
505506

506-
if (NOT ${CMAKE_CXX_COMPILER_ID} MATCHES "Clang")
507-
message(WARNING "Only LLVM is supported for HIP, hint: CXX=/opt/rocm/llvm/bin/clang++")
508-
endif()
507+
string(REGEX MATCH "hipcc$" CXX_IS_HIPCC "${CMAKE_CXX_COMPILER}")
509508

509+
if (CXX_IS_HIPCC)
510+
message(WARNING "Setting hipcc as the C++ compiler is legacy behavior."
511+
" Prefer setting the HIP compiler directly. See README for details.")
512+
else()
513+
# Forward AMDGPU_TARGETS to CMAKE_HIP_ARCHITECTURES.
514+
if (AMDGPU_TARGETS AND NOT CMAKE_HIP_ARCHITECTURES)
515+
set(CMAKE_HIP_ARCHITECTURES ${AMDGPU_ARGETS})
516+
endif()
517+
cmake_minimum_required(VERSION 3.21)
518+
enable_language(HIP)
519+
endif()
510520
find_package(hip REQUIRED)
511521
find_package(hipblas REQUIRED)
512522
find_package(rocblas REQUIRED)
@@ -534,13 +544,18 @@ if (LLAMA_HIPBLAS)
534544
add_compile_definitions(GGML_CUDA_MMV_Y=${LLAMA_CUDA_MMV_Y})
535545
add_compile_definitions(K_QUANTS_PER_ITERATION=${LLAMA_CUDA_KQUANTS_ITER})
536546

537-
set_source_files_properties(ggml-cuda.cu PROPERTIES LANGUAGE CXX)
547+
if (CXX_IS_HIPCC)
548+
set_source_files_properties(ggml-cuda.cu PROPERTIES LANGUAGE CXX)
549+
set(LLAMA_EXTRA_LIBS ${LLAMA_EXTRA_LIBS} hip::device)
550+
else()
551+
set_source_files_properties(ggml-cuda.cu PROPERTIES LANGUAGE HIP)
552+
endif()
538553

539554
if (LLAMA_STATIC)
540555
message(FATAL_ERROR "Static linking not supported for HIP/ROCm")
541556
endif()
542557

543-
set(LLAMA_EXTRA_LIBS ${LLAMA_EXTRA_LIBS} hip::device PUBLIC hip::host roc::rocblas roc::hipblas)
558+
set(LLAMA_EXTRA_LIBS ${LLAMA_EXTRA_LIBS} PUBLIC hip::host roc::rocblas roc::hipblas)
544559
endif()
545560

546561
if (LLAMA_SYCL)

Makefile

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -512,10 +512,10 @@ ifdef LLAMA_HIPBLAS
512512

513513
ifeq ($(wildcard /opt/rocm),)
514514
ROCM_PATH ?= /usr
515-
GPU_TARGETS ?= $(shell $(shell which amdgpu-arch))
515+
AMDGPU_TARGETS ?= $(shell $(shell which amdgpu-arch))
516516
else
517517
ROCM_PATH ?= /opt/rocm
518-
GPU_TARGETS ?= $(shell $(ROCM_PATH)/llvm/bin/amdgpu-arch)
518+
AMDGPU_TARGETS ?= $(shell $(ROCM_PATH)/llvm/bin/amdgpu-arch)
519519
endif
520520
HIPCC ?= $(CCACHE) $(ROCM_PATH)/bin/hipcc
521521
LLAMA_CUDA_DMMV_X ?= 32
@@ -527,7 +527,7 @@ ifdef LLAMA_HIP_UMA
527527
endif # LLAMA_HIP_UMA
528528
MK_LDFLAGS += -L$(ROCM_PATH)/lib -Wl,-rpath=$(ROCM_PATH)/lib
529529
MK_LDFLAGS += -lhipblas -lamdhip64 -lrocblas
530-
HIPFLAGS += $(addprefix --offload-arch=,$(GPU_TARGETS))
530+
HIPFLAGS += $(addprefix --offload-arch=,$(AMDGPU_TARGETS))
531531
HIPFLAGS += -DGGML_CUDA_DMMV_X=$(LLAMA_CUDA_DMMV_X)
532532
HIPFLAGS += -DGGML_CUDA_MMV_Y=$(LLAMA_CUDA_MMV_Y)
533533
HIPFLAGS += -DK_QUANTS_PER_ITERATION=$(LLAMA_CUDA_KQUANTS_ITER)

README.md

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -488,13 +488,28 @@ Building the program with BLAS support may lead to some performance improvements
488488
```
489489
- Using `CMake` for Linux (assuming a gfx1030-compatible AMD GPU):
490490
```bash
491-
CC=/opt/rocm/llvm/bin/clang CXX=/opt/rocm/llvm/bin/clang++ \
491+
HIPCXX="$(hipconfig -l)/clang" HIP_PATH="$(hipconfig -R)" \
492492
cmake -H. -Bbuild -DLLAMA_HIPBLAS=ON -DAMDGPU_TARGETS=gfx1030 -DCMAKE_BUILD_TYPE=Release \
493493
&& cmake --build build -- -j 16
494494
```
495495
On Linux it is also possible to use unified memory architecture (UMA) to share main memory between the CPU and integrated GPU by setting `-DLLAMA_HIP_UMA=ON"`.
496496
However, this hurts performance for non-integrated GPUs (but enables working with integrated GPUs).
497497
498+
Note that if you get the following error:
499+
```
500+
clang: error: cannot find ROCm device library; provide its path via '--rocm-path' or '--rocm-device-lib-path', or pass '-nogpulib' to build without ROCm device library
501+
```
502+
Try searching for a directory under `HIP_PATH` that contains the file
503+
`oclc_abi_version_400.bc`. Then, add the following to the start of the
504+
command: `HIP_DEVICE_LIB_PATH=<directory-you-just-found>`, so something
505+
like:
506+
```bash
507+
HIPCXX="$(hipconfig -l)/clang" HIP_PATH="$(hipconfig -p)" \
508+
HIP_DEVICE_LIB_PATH=<directory-you-just-found> \
509+
cmake -H. -Bbuild -DLLAMA_HIPBLAS=ON -DAMDGPU_TARGETS=gfx1030 -DCMAKE_BUILD_TYPE=Release \
510+
&& cmake --build build -- -j 16
511+
```
512+
498513
- Using `make` (example for target gfx1030, build with 16 CPU threads):
499514
```bash
500515
make -j16 LLAMA_HIPBLAS=1 LLAMA_HIP_UMA=1 AMDGPU_TARGETS=gxf1030

0 commit comments

Comments
 (0)