vulkan : cmake integration #8119
Conversation
mofosyne
added
the
Review Complexity : Low
github-actions
bot
added
build
|
The |
|
It's worth noting here that this will add |
github-actions
bot
added
the
devops
|
I agree the Vulkan SDK is somewhat heavy dependency and pulls in lots of graphics-related dependencies. For Arch and MSYS2 there are the |
github-actions
bot
added
the
nix
|
@MaggotHATE You're correct, the change doesn't force either method. The user may choose to put the libraries within w64devkit if they so choose! Nothing requires one or the other because it depends what is done in the vulkan.pc file. To get it to build though I had to add the $(shell pkg-config --cflags) because it couldn't find the Vulkan includes. This shouldn't affect your setup as it's not required to have Cflags:... |
Yes, I understand it, and that's what I'm hesitant about: w64devkit is about portability - which is lost in currently suggested workflow from Readme. I think "copy folders + create a correct vulkan.pc file" should be preferable for that reason: Windows (w64devkit)Download w64devkit and unpack it into a preferable folder. Run w64devkit.exe, switch into |
|
Understood—I'll add those steps in the readme. 😊 |
github-actions
bot
added
the
documentation
This is FYI only (for now):I'm trying to get llama.cpp to work with Vulkan on SnapDragon X Copilot+PCs on Windows for Arm. I installed the Vulkan-SDK for Windows (x64). Then I built Vulkan-Loader's vulkan1.lib as MSVC Windows arm64 .lib and replaced the Vulkan SDK's x64 lib with it. The main llama.cpp branch seems to build OK with 'cmake -B build -DGGML_VULKAN=1' (on MSVC), but it does not work. I will try and maybe find/fix the issue with the main branch, before trying to test your PR. I will update you, when I have better results. But any ideas very welcome. llama.cpp llama-cli log: [1720599200] Log start Windows vulkaininfoSDK --summary: Vulkan Instance Version: 1.3.277 Instance Extensions: count = 13 Instance Layers: count = 10 Devices: GPU0: GPU1: GPU2: |
|
@AndreasKunar That's great, can you open an issue about that and track your progress/problems there? @bandoti I'm sorry I haven't managed to review this yet, I don't have much time at the moment. I'll try to do it this evening. |
Thanks! I will first try and debug the vulkan1.lib, to make sure that it works (with the Vulkan samples). Once I know, that my WoA vulkan1.lib is OK, I will open an issue (probably not before late tomorrow). |
There was a problem hiding this comment.
Looks good to me. I tested it and it works perfectly, on Linux.
What I noticed that if you have Vulkan, but not glslc, you get a lot of spam like this:
cannot compile matmul_f32_f16_fp32
Vulkan_GLSLC_EXECUTABLE-NOTFOUND -fshader-stage=compute --target-env=vulkan1.2 -O /home/user/llama.cpp/ggml/src/vulkan-shaders/mul_mm.comp -o /home/user/llama.cpp/build_vk/ggml/src/vulkan-shaders.spv/matmul_f32_f16_fp32.spv -DB_TYPE=float16_t -DDATA_A_F32=1 -DD_TYPE=float -DFLOAT_TYPE=float
sh: 1: Vulkan_GLSLC_EXECUTABLE-NOTFOUND: not found
It should be possible to have it throw an error earlier if the shader compiler isn't found, right?
|
Ah—yeah, in that case we should probably add those deps to find_package(Vulkan COMPONENTS glslc REQUIRED). I'll get that change in shortly. In a future pull request we can add checks for MoltenVk to get it working on Apple too. Would rather defer that though to get the broader change in there first. 😊 |
|
Why would MoltenVK not work with these changes? It was working before. |
|
I'd imagine it would work still! Just haven't tested it myself. I guess more specifically I meant in the find_package it can check for MoltenVk and cause an error if it's not found. However, I don't think that'll be an issue because it's kind of assumed on MacOS that this will be installed with Vulkan (unless it's an intel mac I guess). |
|
@ggerganov Considering this touches quite a few files beyond my Vulkan backend, can this be merged, or is there something we should wait for (like another PR or someone else's approval)? |
|
I tested your scripts on Windows for Arm.
This is NOT because of your cmake/vulkan but because of a recent issue with ggml-aarch64.c raised by me with issue#8446 - they do not check for MSVC correctly and generate a gcc _asm_ directive, which aborts the compile. If I fix ggml-aarch64.c with the correct MSVC-excluding compiler-conditionals, you also build for MSVC.
|
There was a problem hiding this comment.
@0cc4m It's good to merge 👍
@AndreasKunar The Arm issue is noted and will be resolved in the scope of #8446
| get_directory_property(GGML_DIR_DEFINES DIRECTORY ggml/src COMPILE_DEFINITIONS) | ||
| get_target_property(GGML_TARGET_DEFINES ggml COMPILE_DEFINITIONS) | ||
| set(GGML_TRANSIENT_DEFINES ${GGML_TARGET_DEFINES} ${GGML_DIR_DEFINES}) | ||
| get_target_property(GGML_LINK_LIBRARIES ggml LINK_LIBRARIES) |
There was a problem hiding this comment.
I don't understand why this is needed - an example with a specific flag that is not exported would help. It's OK to merge as it is though
There was a problem hiding this comment.
@ggerganov I will take a closer look at this. The flags which should be exported should be all interface defines (public interface only) on the ggml/llama libraries. I will follow up with a new pull request.
* Add Vulkan to CMake pkg * Add Sycl to CMake pkg * Add OpenMP to CMake pkg * Split generated shader file into separate translation unit * Add CMake target for Vulkan shaders * Update README.md * Add make target for Vulkan shaders * Use pkg-config to locate vulkan library * Add vulkan SDK dep to ubuntu-22-cmake-vulkan workflow * Clean up tabs * Move sudo to apt-key invocation * Forward GGML_EXTRA_LIBS to CMake config pkg * Update vulkan obj file paths * Add shaderc to nix pkg * Add python3 to Vulkan nix build * Link against ggml in cmake pkg * Remove Python dependency from Vulkan build * code review changes * Remove trailing newline * Add cflags from pkg-config to fix w64devkit build * Update README.md * Remove trailing whitespace * Update README.md * Remove trailing whitespace * Fix doc heading * Make glslc required Vulkan component * remove clblast from nix pkg
This change introduces a
makeandCMakebuild target for Vulkan shaders per #5356. This ensuresggml-vulkan-shaders.hppis generated at build time (instead of storing in SCM). In addition,ggml-vulkan-shaders.cppis added to move compiled shaders into its own translation unit.In addition, this change updates the relocatable CMake package to link against the new
ggmllibrary.