vulkan : incremental shader builds #16341

Acly · 2025-09-29T19:04:28Z

Alternative to #15993

based on jeffbolznv/llama.cpp/shader_gen_per_file
added dep-file support to handle includes
streamlined vulkan-shader-gen a bit (removed --input-dir, fewer branches, apply file filter much earlier)
use stringstream for string composition and only write to file if there are changes to avoid recompilation of ggml-vulkan.cpp
fixed a bug where vulkan-shader-gen hangs if enough processes have compilation errors

Note: changes in shaders are just renaming glsl "headers" to use .glsl extension. Otherwise they are GLOB'd together with the actual compute shaders and trying to compile them fails.

…ve incremental build times

…hange * rename shader files which are used as "headers" to use .glsl extension * move glslc extension detection shaders to separate folders * the above is to prevent them from getting glob'd with the actual compute shaders that need to be compiled

* avoid recompiling ggml-vulkan.cpp when editing shaders * pass single --source argument instead of --input-dir & --filter to shader gen * check for source file match earlier

* early out did not decrement compile_count

jeffbolznv

(stray partial comment deleted)

jeffbolznv · 2025-09-29T22:02:53Z

ggml/src/ggml-vulkan/vulkan-shaders/vulkan-shaders-gen.cpp

+
+void write_file_if_changed(const std::string& path, const std::string& content) {
+    std::vector<unsigned char> existing = read_binary_file(path, true);
+    if (existing.size() != content.size() || memcmp(existing.data(), content.data(), content.size()) != 0) {


I think this is preventing dependencies from being satisfied. If I touch a shader source but don't change it, it'll keep recompiling it each time I build.

Yes, at least for generators which check output file timestamps.

I've changed the .cpp files to be written unconditionally. They compile quickly now that they're much smaller.

For the .hpp I think the check is still a good trade-off - it is quick and often avoids compilation of ggml-vulkan.cpp which is very slow.

jeffbolznv · 2025-09-29T22:10:09Z

ggml/src/ggml-vulkan/CMakeLists.txt

    test_shader_extension_support(
        "GL_EXT_integer_dot_product"
-        "${CMAKE_CURRENT_SOURCE_DIR}/vulkan-shaders/test_integer_dot_support.comp"
+        "${CMAKE_CURRENT_SOURCE_DIR}/vulkan-shaders/feature-tests/integer_dot_product.comp"


This doesn't match the new file name.

I'm seeing some weird behavior in my WSL build where sometimes it doesn't run these checks, and it is somehow getting the wrong value of GGML_VULKAN_INTEGER_DOT_GLSLC_SUPPORT. I don't know what's going on yet.

I manually deleted the cmake cache files for vulkan-shader-gen (deep in the build) and now this is behaving normally.

Ow I forgot to check those, thanks for noticing. Fixed the filename.

0cc4m

From my side the change is fine. I miss the direct load from disk mode, though, it was kinda cool how instant it was. But maybe not worth the amount of code. 16s is still great for me.

Here's a test run where I change something in the matmul_mm shader and then recompile:
Master: 1:34.44 total
PR: 15.972 total

0cc4m · 2025-10-03T09:46:07Z

@jeffbolznv Any concerns left?
@bandoti You could also take a look, if you want.

jeffbolznv · 2025-10-03T12:19:29Z

I'm happy with it

netrunnereve · 2025-10-03T15:21:42Z

So I tried making a change to the .comp shaders and that worked well with only that shader getting recompiled. However I tried making changes to the .glsl shaders and there was no recompilation happening there at all. I could literally fill mul_mm_funcs.glsl with random text and it wouldn't give me a compile error.

Acly · 2025-10-03T16:15:33Z

@netrunnereve That should work, but there are some limitations for cmake DEPFILE support. Which cmake version and generator are you using?

There should be a file build/ggml/src/ggml-vulkan/mul_mm.comp.cpp.d which tracks the dependency.

bandoti · 2025-10-03T17:32:42Z

It looks like the CMakeLists.txt file specifies version 3.19, but per the cmake docs:

Added in version 3.21: Added support for Visual Studio Generators with VS 2012 and above, and for the Xcode generator. Support for generator expressions was also added.

...

Using DEPFILE with generators other than those listed above is an error.

Is there a need for DEPFILE or is there another way to solve that?

bandoti · 2025-10-03T17:46:46Z

I just noticed there's an inconsistency in all of the required CMakeLists.txt version requirements. I think we need to keep them in sync with the top-level ggml CMakeLists.txt. For example, ggml-vulkan requires 3.19, but the top-level requires 3.14.

So, in order to have DEPFILE the wider discussion needs to be whether it's okay to bump all those versions to 3.21, and they should all be updated and the same. Personally, I am okay with increasing the version but someone else might oppose it. I am not sure what the impact would be there. :)

@Acly Which generators have been tested so far? I would recommend testing with Visual Studio, Make, and Ninja at the miminum using CMake having the minimum required version. I had many issues with the generators earlier on and it required a lot of manual testing!

bandoti · 2025-10-03T17:55:10Z

@0cc4m What did you mean by this? "I miss the direct load from disk mode, though, it was kinda cool how instant it was." Just curious what is being removed from this, that might not need to be removed.

0cc4m · 2025-10-03T18:19:36Z

I meant the previous PR: #15993

netrunnereve · 2025-10-03T21:04:20Z

@netrunnereve That should work, but there are some limitations for cmake DEPFILE support. Which cmake version and generator are you using?

There should be a file build/ggml/src/ggml-vulkan/mul_mm.comp.cpp.d which tracks the dependency.

I use cmake 3.28.3 and generate regular makefiles. If I look inside build/ggml/src/ggml-vulkan/mul_mm.comp.cpp.d I see that it includes my changed mul_mm_funcs.glsl shader but it seems like cmake can't tell that it has been changed.

Acly · 2025-10-03T21:59:30Z

I use cmake 3.28.3 and generate regular makefiles.

Thanks. I think I see what's happening, the depfile is created by glslc when compiling a .spv output, but in CMake the embed .cpp is used as output. The filenames don't match. It doesn't seem to be a problem for ninja, but I think make imports it as a rule directly so it won't work.

I think shader-gen will have to replace the filename in the depfile, can't think of another solution. Or go back to the other PR which doesn't have this issue. But I'll give it a try here first.

I miss the direct load from disk mode, though, it was kinda cool how instant it was.

With the much smaller embed .cpps it's only mul_mm.comp with its 125MB worth of variants where it still makes a difference, so it didn't feel worth :<

Which generators have been tested so far? I would recommend testing with Visual Studio, Make, and Ninja at the miminum using CMake having the minimum required version.

Yea I usually test MSBuild, Make and Ninja, though not always after every change.

Regarding CMake minimum version: I did not change them because that would restrict compilation of llama.cpp to a higher version even when Vulkan backend is not enabled. Likewise the minimum version for Vulkan's CMakeLists is high enough to work with Ninja. The DEPFILE mechanism could even be conditionally excluded when not supported without breaking clean builds.

I don't really have a strong opinion on that, but it feels like llama.cpp is deliberately conservative to compile on as many exotic setups as possible, so if it can work, I don't want to artificially block it. Meanwhile I think it's reasonable to expect active developers to run a somewhat recent version of cmake.

…keLists

Acly · 2025-10-03T23:46:39Z

Dependency tracking works now (tested Ubuntu+Makefile, Ubuntu+Ninja, Windows+MSBuild)

netrunnereve · 2025-10-04T01:00:03Z

Great it's working for me as well.

0cc4m · 2025-10-04T09:42:35Z

Works for me too. Thank you, this makes shader development a lot nicer!

This reverts commit e29acf7.

jeffbolznv and others added 5 commits September 22, 2025 16:36

vulkan (DRAFT): split shader generation by GLSL source file, to impro…

38624ab

…ve incremental build times

vulkan : only write embedded shader .hpp/.cpp when they change

a96b7d1

* avoid recompiling ggml-vulkan.cpp when editing shaders * pass single --source argument instead of --input-dir & --filter to shader gen * check for source file match earlier

fix hang in vulkan-shaders-gen when there are compilation errors

08a90ed

* early out did not decrement compile_count

clean up

f19cc77

Acly requested a review from 0cc4m as a code owner September 29, 2025 19:04

Acly mentioned this pull request Sep 29, 2025

vulkan : shader development improvements #15993

Closed

github-actions bot added Vulkan Issues specific to the Vulkan backend ggml changes relating to the ggml tensor library for machine learning labels Sep 29, 2025

jeffbolznv reviewed Sep 29, 2025

View reviewed changes

Acly added 2 commits September 30, 2025 10:03

fix glslc integer dot product test

29c768d

unconditionally write the embedded shader cpp output

66a4760

0cc4m approved these changes Oct 3, 2025

View reviewed changes

jeffbolznv approved these changes Oct 3, 2025

View reviewed changes

replace output filepath in generated dep-files to match output in CMa…

44ad3fe

…keLists

0cc4m merged commit e29acf7 into ggml-org:master Oct 4, 2025
66 of 68 checks passed

Nexesenex added a commit to Nexesenex/croco.cpp that referenced this pull request Oct 4, 2025

Revert "vulkan : incremental shader builds (ggml-org#16341)"

5a372f6

This reverts commit e29acf7.

vulkan : incremental shader builds #16341

vulkan : incremental shader builds #16341

Conversation

Acly commented Sep 29, 2025

Uh oh!

jeffbolznv left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeffbolznv Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

Acly Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

jeffbolznv Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

jeffbolznv Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

jeffbolznv Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

Acly Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

0cc4m left a comment

Choose a reason for hiding this comment

Uh oh!

0cc4m commented Oct 3, 2025

Uh oh!

jeffbolznv commented Oct 3, 2025

Uh oh!

netrunnereve commented Oct 3, 2025

Uh oh!

Acly commented Oct 3, 2025

Uh oh!

bandoti commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bandoti commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bandoti commented Oct 3, 2025

Uh oh!

0cc4m commented Oct 3, 2025

Uh oh!

netrunnereve commented Oct 3, 2025

Uh oh!

Acly commented Oct 3, 2025

Uh oh!

Acly commented Oct 3, 2025

Uh oh!

netrunnereve commented Oct 4, 2025

Uh oh!

0cc4m commented Oct 4, 2025

Uh oh!

Uh oh!

Uh oh!

jeffbolznv left a comment •

edited

Loading

bandoti commented Oct 3, 2025 •

edited

Loading

bandoti commented Oct 3, 2025 •

edited

Loading