llama.cpp ROCm: use native CMake HIP support

Supercedes #4024 and #4813.

CMake's native HIP support has become the recommended way to add HIP code into a project (see here). This PR makes the following changes:

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.
CMake option CMAKE_HIP_ARCHITECTURES is used to control the GPU architectures to build for. Previously this was controled by GPU_TARGETS.
Updated the Nix recipe to account for these new changes.
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.

Mar 09 '24 17:03 GZGavinZhao

Since this requires cmake 3.21, I think it would be good to add a cmake_minimum_required(VERSION 3.21) to the HIP section, similar to how the CUDA section requires version 3.17.

Mar 09 '24 19:03 slaren

@slaren Good point, adjusted.

Mar 11 '24 01:03 GZGavinZhao

@slaren responding to the test request. Since I learned that the only way this works on Fedora is to pass CC=/usr/bin/hipcc CXX=/usr/bin/hipcc I am now a bit confused on how or what to test, since according to the description I should not be setting HIPCXX to hipcc and at the same time I do not have a custom /opt/ROCm llvm installation and hipcc is supposedly somehow handling that (still trying to figure out how Fedora does it exactly).

Not setting anything for CC and CXX fails right away:

-- Check for working CXX compiler: /usr/bin/c++
-- Check for working CXX compiler: /usr/bin/c++ - broken
CMake Error at /home/xxx/.local/lib/python3.12/site-packages/cmake/data/share/cmake-3.28/Modules/CMakeTestCXXCompiler.cmake:60 (message):
  The C++ compiler

    "/usr/bin/c++"

  is not able to compile a simple test program.

If I pass CC=/usr/bin/clang-17 CXX=/usr/bin/clang++-17 (which is what crashed me on master as described in #6031) then I get this far:

-- The HIP compiler identification is Clang 17.0.6
CMake Error at /home/xxx/.local/lib/python3.12/site-packages/cmake/data/share/cmake-3.28/Modules/CMakeDetermineHIPCompiler.cmake:217 (message):
  The ROCm root directory:

   /usr/local

  does not contain the HIP runtime CMake package, expected at one of:

   /usr/local/lib/cmake/hip-lang/hip-lang-config.cmake
   /usr/local/lib64/cmake/hip-lang/hip-lang-config.cmake
   /usr/local/lib/x86_64-redhat-linux-gnu/cmake/hip-lang/hip-lang-config.cmake

Call Stack (most recent call first):
  CMakeLists.txt:502 (enable_language)

Shouldn't it first look in /usr?

So basically... either I am not understanding how to use the new options or they do not work on Fedora.

Could you please hint on how the build should be started?

Mar 13 '24 22:03 jin-eld

I was hoping that with these changes you wouldn't need to pass additional parameters to cmake to workaround the issue, beyond enabling HIP. It seems very suspect that cmake thinks that your C++ compiler is broken.

Mar 13 '24 22:03 slaren

CC and CXX can be whatever, that's the point of this PR. To tell CMake the HIP compiler to use, for Fedora's case set HIPCXX=/usr/bin/clang because they use the system Clang as the HIP Clang compiler. Now, since Fedora doesn't install ROCm at /opt/rocm, you should set HIP_PATH=/usr and HIP_DEVICE_LIB_PATH=/usr/lib/amdgcn/bitcode (or wherever .bc files are stored, try searching for the directory that contains the file oclc_abi_version_400.bc under /usr) to tell HIP Clang where to find the HIP runtime libraries.

Mar 13 '24 22:03 GZGavinZhao

or Fedora's case set HIPCXX=/usr/bin/clang

@GZGavinZhao but isn't this exactly what cause the segfault for me in #6031 ? When I was passing system's clang it crashed, when I was passing hipcc for both, CC and CXX the crash went away...

So, based on what you wrote above I tried:

HIPCXX=/usr/bin/clang HIP_PATH=/usr HIP_DEVICE_LIB_PATH=usr/lib/clang/17/amdgcn/bitcode  cmake .. -DLLAMA_HIPBLAS=ON -DAMDGPU_TARGETS=gfx900 -DCMAKE_BUILD_TYPE=Release

-- Detecting HIP compiler ABI info
-- Detecting HIP compiler ABI info - failed
-- Check for working HIP compiler: /usr/bin/clang
-- Check for working HIP compiler: /usr/bin/clang - broken
CMake Error at /home/xxx/.local/lib/python3.12/site-packages/cmake/data/share/cmake-3.28/Modules/CMakeTestHIPCompiler.cmake:73 (message):
  The HIP compiler

    "/usr/bin/clang"

  is not able to compile a simple test program.
  
    It fails with the following output:

    Change Dir: '/home/xxx/Work/llama.cpp/build-pull/CMakeFiles/CMakeScratch/TryCompile-Vc7BIw'
    
    Run Build Command(s): /home/xxx/.local/lib/python3.12/site-packages/cmake/data/bin/cmake -E env VERBOSE=1 /usr/bin/gmake -f Makefile cmTC_c622f/fast
    /usr/bin/gmake  -f CMakeFiles/cmTC_c622f.dir/build.make CMakeFiles/cmTC_c622f.dir/build
    gmake[1]: Entering directory '/home/xxx/Work/llama.cpp/build-pull/CMakeFiles/CMakeScratch/TryCompile-Vc7BIw'
    Building HIP object CMakeFiles/cmTC_c622f.dir/testHIPCompiler.hip.o
    /usr/bin/clang    --offload-arch=gfx900 -o CMakeFiles/cmTC_c622f.dir/testHIPCompiler.hip.o  -c /home/xxx/Work/llama.cpp/build-pull/CMakeFiles/CMakeScratch/TryCompile-Vc7BIw/testHIPCompiler.hip
    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
    gmake[1]: *** [CMakeFiles/cmTC_c622f.dir/build.make:78: CMakeFiles/cmTC_c622f.dir/testHIPCompiler.hip.o] Error 1
    gmake[1]: Leaving directory '/home/xxx/Work/llama.cpp/build-pull/CMakeFiles/CMakeScratch/TryCompile-Vc7BIw'
    gmake: *** [Makefile:127: cmTC_c622f/fast] Error 2

As far as I can tell, if HIP_DEVICE_LIB_PATH was supposed to set --rocm-device-lib-path, then something did not work.

Mar 13 '24 22:03 jin-eld

Ah sorry, HIP_DEVICE_LIB_PATH should be /usr/lib/clang/17/amdgcn/bitcode.

Mar 13 '24 22:03 GZGavinZhao

Ah sorry, HIP_DEVICE_LIB_PATH should be /usr/lib/clang/17/amdgcn/bitcode.

Sorry my bad - I mispasted the /, so this is the "correct" run then:

$ HIPCXX=/usr/bin/clang HIP_PATH=/usr HIP_DEVICE_LIB_PATH=/usr/lib/clang/17/amdgcn/bitcode  cmake .. -DLLAMA_HIPBLAS=ON -DAMDGPU_TARGETS=gfx900 -DCMAKE_BUILD_TYPE=Release 
-- The C compiler identification is GNU 14.0.1
-- The CXX compiler identification is GNU 14.0.1
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found Git: /usr/bin/git (found version "2.44.0") 
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE  
-- The HIP compiler identification is Clang 18.1.0
-- Detecting HIP compiler ABI info
-- Detecting HIP compiler ABI info - done
-- Check for working HIP compiler: /usr/bin/clang - skipped
-- Detecting HIP compile features
-- Detecting HIP compile features - done
-- HIP and hipBLAS found
-- Warning: ccache not found - consider installing it for faster compilation or disable this warning with LLAMA_CCACHE=OFF
-- CMAKE_SYSTEM_PROCESSOR: x86_64
-- x86 detected
-- Configuring done (3.9s)

But then when I make I get:

lang: 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
make[2]: *** [CMakeFiles/ggml.dir/build.make:131: CMakeFiles/ggml.dir/ggml-cuda.cu.o] Error 1

I think Fedora has a slightly non standard setup at the moment, they use vanilla llvm, but add some AMD bits and pieces on top and I guess this is why compiling with hipcc is actually required there.

Mar 13 '24 23:03 jin-eld

Ah, so you should be calling make while passing the HIP_PATH and HIP_DEVICE_LIB_PATH, like this: HIP_PATH=/usr HIP_DEVICE_LIB_PATH=/usr/lib/clang/17/amdgcn/bitcode make.

To clarify, HIPCXX is just to tell CMake what compiler it should use to compile HIP code. On CMake's side, that's all it needs.

HIP_PATH and HIP_DEVICE_LIB_PATH are for the HIP compiler to run correctly. Previously, HIP_PATH and HIP_DEVICE_LIB_PATH are passed to the underlying HIP compiler through the hipcc wrapper script, so now since we're calling the HIP compiler directly we should handle these variables by ourselves. Since CMake checks if a compiler can actually compile code during setup phase, HIP_PATH and HIP_DEVICE_LIB_PATH must be present not only during build phase, but also during setup phase.

Mar 13 '24 23:03 GZGavinZhao

Ok, passing the env vars to make made it build, although I have to say that this is not very intuitive since one usually expects that a cmake or configure run takes cares of those things and you'd just make

This was the line how it compiled:

HIPCXX=/usr/bin/clang HIP_PATH=/usr HIP_DEVICE_LIB_PATH=/usr/lib/clang/17/amdgcn/bitcode  cmake .. -DLLAMA_HIPBLAS=ON -DAMDGPU_TARGETS=gfx900 -DCMAKE_BUILD_TYPE=Release
HIP_PATH=/usr HIP_DEVICE_LIB_PATH=/usr/lib/clang/17/amdgcn/bitcode make -j16

Now, unfortunately the real issue here is that building it like that causes it to crash just as described in #6031 and with this PR applied I can not build it "the old way" anymore, which was needed to avoid the crash, so the "working" build on Fedora works on master like this:

CC=/usr/bin/hipcc CXX=/usr/bin/hipcc cmake .. -DLLAMA_HIPBLAS=ON -DAMDGPU_TARGETS=gfx900 -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_FLAGS="--rocm-device-lib-path=/usr/lib/clang/17/amdgcn/bitcode"

Based on my tests I'd say that the PR would not make it possible to build a working version of llama.cpp on Fedora anymore, seems it all has to happen with hipcc there :(

Mar 13 '24 23:03 jin-eld

Hmmm it's weird that they introduce different compiling results... My suspicion is that hipcc is not calling /usr/bin/clang but something other compiler. Can you paste the output of hipconfig?

Mar 13 '24 23:03 GZGavinZhao

Here it is:

HIP version  : 6.0.32831-

== hipconfig
HIP_PATH     : /usr
ROCM_PATH    : /usr
HIP_COMPILER : clang
HIP_PLATFORM : amd
HIP_RUNTIME  : rocclr
CPP_CONFIG   :  -D__HIP_PLATFORM_HCC__= -D__HIP_PLATFORM_AMD__= -I/usr/include -I/usr/lib64/llvm17/bin/../../../lib/clang/17
 

== hip-clang
HIP_CLANG_PATH   : /usr/lib64/llvm17/bin
clang version 17.0.6 (Fedora 17.0.6-7.fc41)
Target: x86_64-redhat-linux-gnu
Thread model: posix
InstalledDir: /usr/lib64/llvm17/bin
LLVM (http://llvm.org/):
  LLVM version 17.0.6
  Optimized build.
  Default target: x86_64-redhat-linux-gnu
  Host CPU: znver2

  Registered Targets:
    aarch64     - AArch64 (little endian)
    aarch64_32  - AArch64 (little endian ILP32)
    aarch64_be  - AArch64 (big endian)
    amdgcn      - AMD GCN GPUs
    arm         - ARM
    arm64       - ARM64 (little endian)
    arm64_32    - ARM64 (little endian ILP32)
    armeb       - ARM (big endian)
    avr         - Atmel AVR Microcontroller
    bpf         - BPF (host endian)
    bpfeb       - BPF (big endian)
    bpfel       - BPF (little endian)
    hexagon     - Hexagon
    lanai       - Lanai
    loongarch32 - 32-bit LoongArch
    loongarch64 - 64-bit LoongArch
    mips        - MIPS (32-bit big endian)
    mips64      - MIPS (64-bit big endian)
    mips64el    - MIPS (64-bit little endian)
    mipsel      - MIPS (32-bit little endian)
    msp430      - MSP430 [experimental]
    nvptx       - NVIDIA PTX 32-bit
    nvptx64     - NVIDIA PTX 64-bit
    ppc32       - PowerPC 32
    ppc32le     - PowerPC 32 LE
    ppc64       - PowerPC 64
    ppc64le     - PowerPC 64 LE
    r600        - AMD GPUs HD2XXX-HD6XXX
    riscv32     - 32-bit RISC-V
    riscv64     - 64-bit RISC-V
    sparc       - Sparc
    sparcel     - Sparc LE
    sparcv9     - Sparc V9
    systemz     - SystemZ
    thumb       - Thumb
    thumbeb     - Thumb (big endian)
    ve          - VE
    wasm32      - WebAssembly 32-bit
    wasm64      - WebAssembly 64-bit
    x86         - 32-bit X86: Pentium-Pro and above
    x86-64      - 64-bit X86: EM64T and AMD64
    xcore       - XCore
hip-clang-cxxflags :  -O3
hip-clang-ldflags  : --driver-mode=g++ -O3 --hip-link --rtlib=compiler-rt -unwindlib=libgcc

=== Environment Variables
PATH=/usr/lib64/rocm/gfx9/bin:/home/xxx/.local/bin:/home/xxx/bin:/usr/share/Modules/bin:/usr/local/bin:/usr/bin:/usr/local/sbin:/usr/sbin
LD_LIBRARY_PATH=/usr/lib64/rocm/gfx9/lib

== Linux Kernel
Hostname     : skynet
Linux skynet 6.8.0-63.fc41.x86_64 #1 SMP PREEMPT_DYNAMIC Mon Mar 11 19:16:23 UTC 2024 x86_64 GNU/Linux

Mar 13 '24 23:03 jin-eld

Ah okay, so HIPCXX should be HIPCXX=/usr/lib64/llvm17/bin/clang. Please try if that works.

Edit: if that works, then please try if HIPCXX=/usr/bin/clang-17 works as well.

Mar 13 '24 23:03 GZGavinZhao

HIPCXX=/usr/lib64/llvm17/bin/clang seems to work, I get some clang: warning: argument unused during compilation: '--offload-arch=gfx900' [-Wunused-command-line-argument] during the build, but this does not seem to affect anything. The resulting binary did not crash for me, the build seems fine.

/usr/bin/clang-17 and /usr/lib64/llvm17/bin/clang is the same: /usr/bin/clang-17 -> ../..//usr/lib64/llvm17/bin/clang

By the way, now with HIPCXX=/usr/lib64/llvm17/bin/clang I did not have to pass any additional flags to make

Mar 13 '24 23:03 jin-eld

Interesting. Does running cmake work with only HIPCXX set (no HIP_PATH and HIP_DEVICE_LIB_PATH) as well?

Mar 14 '24 00:03 GZGavinZhao

Interesting. Does running cmake work with only HIPCXX set (no HIP_PATH and HIP_DEVICE_LIB_PATH) as well?

Hmm, it does indeed. So now we reduced it all to just:

HIPCXX=/usr/bin/clang-17 cmake .. -DLLAMA_HIPBLAS=ON -DAMDGPU_TARGETS=gfx900 -DCMAKE_BUILD_TYPE=Release
make -j20

And here is the cmake otuput:

-- The C compiler identification is GNU 14.0.1
-- The CXX compiler identification is GNU 14.0.1
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found Git: /usr/bin/git (found version "2.44.0") 
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE  
-- The HIP compiler identification is Clang 17.0.6
-- Detecting HIP compiler ABI info
-- Detecting HIP compiler ABI info - done
-- Check for working HIP compiler: /usr/bin/clang-17 - skipped
-- Detecting HIP compile features
-- Detecting HIP compile features - done
-- HIP and hipBLAS found
-- Warning: ccache not found - consider installing it for faster compilation or disable this warning with LLAMA_CCACHE=OFF
-- CMAKE_SYSTEM_PROCESSOR: x86_64
-- x86 detected

Only the warnings seem odd, I guess some hip-only flags are also passed to the non-hip build:

[ 97%] Linking HIP executable ../../bin/main
clang-17: warning: argument unused during compilation: '--offload-arch=gfx900' [-Wunused-command-line-argument]
clang-17: warning: argument unused during compilation: '--offload-arch=gfx900' [-Wunused-command-line-argument]

I wonder if there is a way for you to autodetect what to set in HIPCXX, it would be awesome if that was possible :>

I'm off for tonight, so if you need more testing I'll be around tomorrow.

Mar 14 '24 00:03 jin-eld

Yes, in fact there is! When you get a chance, please try this:

HIPCXX="$(hipconfig -l)/clang" cmake <cmake-options>
make

If this works, I'm going to add this info to the documentation in this PR.

Mar 14 '24 00:03 GZGavinZhao

OK now that's odd and I am not sure if I messed something up, now suddenly it can't find the ROCm directory again and is looking in /usr/local again...

I am sure I did an rm -rf of the build dir each time I tried, now I'm confused

Mar 14 '24 00:03 jin-eld

So it'd be:

HIPCXX="$(hipconfig -l)/clang" HIP_PATH="$(hipconfig -R)" cmake .. -DLLAMA_HIPBLAS=ON -DAMDGPU_TARGETS=gfx900 -DCMAKE_BUILD_TYPE=Release
make
`

Mar 14 '24 00:03 jin-eld

Can you update the build instructions for HIP in the README?

Mar 15 '24 11:03 slaren

One note though, if I understood Fedora packagers correctly hipcc is actually supposed to be used on Fedora for everything when compiling HIP enabled applications. So there it's not really meant to separate compilers for HIP and non HIP code. This may change in the future, I think the current situation is more of a workaround for not being able to ship a complete ROCm llvm copy due to some policies, but I still wonder if it would be better to also leave an option to use hipcc after all, even though we somehow managed to find a way to build this PR without hipcc.

Mar 15 '24 12:03 jin-eld

I think it is good (and important) to follow AMD's recommendations for building. If Fedora wants to deviate from this process, they should deal with the problems that it causes.

Mar 15 '24 12:03 slaren

I think it is good (and important) to follow AMD's recommendations for building. If Fedora wants to deviate from this process, they should deal with the problems that it causes.

Isn't AMD's recommendation exactly this - to build everything exclusively with their llvm version or am I mistaken? Afaik the whole ROCm suite is also supposed to be built like that?

Mar 15 '24 12:03 jin-eld

Isn't AMD's recommendation exactly this - to build everything exclusively with their llvm version or am I mistaken?

I don't see that in the documentation linked here (https://rocm.docs.amd.com/en/docs-6.0.0/conceptual/cmake-packages.html#using-hip-in-cmake). It may have been that way in the past to deal with poor support in the build tools, but that no longer seems to be the case. I don't see any reason to compile source files without HIP code with the HIP compiler.

Mar 15 '24 12:03 slaren

Can you update the build instructions for HIP in the README?

I will. Sorry for the delay, got a bit busy yesterday. I'll update the docs later today.

Mar 15 '24 16:03 GZGavinZhao

One note though, if I understood Fedora packagers correctly hipcc is actually supposed to be used on Fedora for everything when compiling HIP enabled applications.

It is currently used to compile HIP enabled, but it's not supposed to be used. The only reason is that AMD's official packages are still using the legacy way of masquerading hipcc as the C/C++ compiler, so Fedora had to use hipcc to compile everything. AMD is currently moving away from using hipcc to compile everything (e.g. this commit).

Mar 15 '24 17:03 GZGavinZhao

It is currently used to compile HIP enabled, but it's not supposed to be used. The only reason is that AMD's official packages are still using the legacy way of masquerading hipcc as the C/C++ compiler, so Fedora had to use hipcc to compile everything. AMD is currently moving away from using hipcc to compile everything (e.g. this commit).

I think Fedora will move away from hipcc too as it usually follows upstream, my point was more about not breaking anything too early during the transition period :)

Another thing which may be off topic here, just as a heads up - there is work being done on packaging llama.cpp for Fedora. One thing that was pointed out, that cli tools have names like main which would require renaming them in the package. Sine you seem to be actively working on the build system I am just hinting if you guys would consider renaming main, server and potentially some other binaries that make sense to be installed on the system.

https://bugzilla.redhat.com/show_bug.cgi?id=2255828

Mar 15 '24 17:03 jin-eld

I think Fedora will move away from hipcc too as it usually follows upstream, my point was more about not breaking anything too early during the transition period :)

Makes sense. I will add a fallback check, such that if CMAKE_CXX_COMPILER is set to/ends with hipcc, emit a warning that recommends using native CMake HIP support and continue on with the legacy/original behavior.

Mar 15 '24 17:03 GZGavinZhao

Fallback added, docs added, and added a CI check for HIP builds. I still need to test on Windows though.

Mar 15 '24 23:03 GZGavinZhao

Bruh the docker image for ROCm is so big the runner went out of space. Used a smaller ROCm image and we install dependencies by ourselves.

Mar 15 '24 23:03 GZGavinZhao