Repository navigation
vulkan: use spec constant for matrix matrix multiplication A-type - #25773
Conversation
|
I had experimented with something like this and ended up with one spirv for all the legacy and K quants and then separate spirvs for all the iq quants. That resolves the shared memory issue and probably isn't a ton of bloat because those spirvs are dominated by the table size anyway. Fwiw, even if our compiler dead code eliminated the shared memory arrays, they would still count against the spec limit. |
a619971 to
496b099
Compare
|
I think the shared memory arrays with length 1 could run into this compiler bug again #24924. |
|
I don't see these CI failures locally, not sure what is causing them. Is that the compiler bug? |
I also can't reproduce the cm1 failures locally. It's conceivable it's the Turing-only bug. I think it's more likely to happen if you use more fp16 math, is there any place you've changed fp32 math to fp16? |
|
No, the PR should not contain any functional changes, it's all just restructuring. The running code in the end should be basically identical. |
cbf3468 to
001600c
Compare
001600c to
197898e
Compare
|
@ggerganov The Nvidia-CM pipeline is timing out in test-backend-ops, I haven't seen that before. Is that expected? Is it possible to increase the timeout? |
|
Hm, not sure - I can't see a reason for the increased time. It almost doubled than usual for this job (~15 mins). Also, looking at the most recent runs in https://github.com/ggml-org/llama.cpp/actions/workflows/build-self-hosted.yml it seems to be fluctuating between slow/fast without a clear point where it started happening. Let's keep monitoring. These workflows are currently running on DGX Spark that I self-host. We are already looking for ways to migrate this to some more stable and scalable infrastructure, but don't have ETA atm. |
|
Did it recently switch from T4 to the Spark? That would explain why it now passed some tests that were failing earlier due (likely to the Turing driver bug. |
|
The DGX Spark has been running these and other jobs for about 3 months now (added on May 26). The T4 runners are also running - whoever runner picks of the job first will execute it. |
Maybe this change effects enough shaders that it get no benefit from the shader disk cache from previous runs? If so, a rerun might be enough to make it complete. |
|
I reran it multiple times and it keeps failing, so I don't think that's it. I'll try to reproduce locally. |
|
It consistently happens in the CI, but I cannot reproduce it locally. I get worst case 7 minutes on my DGX Spark, with caching just a little bit more than 3 minutes. I'm not sure what is going on there. |
|
Here are the logs of one slow and one fast run: One thing that draws attention is that in the slow run, the cmake builds at the start are quite a bit slower. F.ex: |
Is this 7 minutes for the entire |
|
Just for |
|
The entire CI script went from 22:46 on master uncached to 12:09 cached, with the PR from 13:14 uncached to 7:44 cached, on my Spark. That isn't consistent with the CI either. |
|
I just tried manually - on both my DGX Sparks, this command deadlocks/blocks with Vulkan on this branch: GGML_VK_DISABLE_COOPMAT2=1 ./bin/test-backend-ops -o MUL_MATThere is a suspicious error though at the start about incompatible driver:
Edit: the same command works fine on |
|
Yes, it completes without issue. It's suspicious that your driver reports the device as The driver warning is a mesa adreno Vulkan driver artifact, you can ignore it. |
|
Here is DetailsHere is the DetailsThe jobs also have this info at the start of the logs: https://github.com/ggml-org/llama.cpp/actions/runs/32224960472/job/97381646994?pr=25773#step:3:54. For example this job, runs on |
|
Sorry, I made a mistake testing, my environment wasn't set up correctly and I didn't notice. I can actually reproduce it, I'll look into it. |
|
I excluded LUT quants from the new unified shader, which seems to avoid whatever bug Nvidia was hitting with coopmat1. I'll clean up the code, then this should be ready for review. |
df88488 to
19e4a29
Compare
vulkan: use map for mul_mm shapes cleanup fix indentation fix cm2 and shmem init fix cm2 spec constants fix cm2 bindings consolidate shmem tables and reduce size by type spec constant fix compiler warning fix missing Q2_0 type fix unused warning when integer dot glslc support is missing use minimal shmem size 8 instead of 1 to workaround cm2 compiler bug fix missing Q2_0 type in cm2 matmul fix types
b363e78 to
b1b70dd
Compare
|
This one is next to merge, please briefly check and reapprove @jeffbolznv |
…ml-org#25773) * vulkan: use spec constant for mul mat type_a vulkan: use map for mul_mm shapes cleanup fix indentation fix cm2 and shmem init fix cm2 spec constants fix cm2 bindings consolidate shmem tables and reduce size by type spec constant fix compiler warning fix missing Q2_0 type fix unused warning when integer dot glslc support is missing use minimal shmem size 8 instead of 1 to workaround cm2 compiler bug fix missing Q2_0 type in cm2 matmul fix types * remove LUT quants from unified shader * clean up * restore coopmat2 q4_k/q5_k optimization * split out q4_k/q5_k cm2 shader to fix Ampere regression * revert iq shmem table renames * simplify cm2 code with single uint8_t buffer * fix fp4 extension use switch being overwritten by generic shader * clean up * adapt TQ1_0 changes * adapt ggml-org#27471 f16 Intel tuning changes
…ml-org#25773) * vulkan: use spec constant for mul mat type_a vulkan: use map for mul_mm shapes cleanup fix indentation fix cm2 and shmem init fix cm2 spec constants fix cm2 bindings consolidate shmem tables and reduce size by type spec constant fix compiler warning fix missing Q2_0 type fix unused warning when integer dot glslc support is missing use minimal shmem size 8 instead of 1 to workaround cm2 compiler bug fix missing Q2_0 type in cm2 matmul fix types * remove LUT quants from unified shader * clean up * restore coopmat2 q4_k/q5_k optimization * split out q4_k/q5_k cm2 shader to fix Ampere regression * revert iq shmem table renames * simplify cm2 code with single uint8_t buffer * fix fp4 extension use switch being overwritten by generic shader * clean up * adapt TQ1_0 changes * adapt ggml-org#27471 f16 Intel tuning changes
…ml-org#25773) * vulkan: use spec constant for mul mat type_a vulkan: use map for mul_mm shapes cleanup fix indentation fix cm2 and shmem init fix cm2 spec constants fix cm2 bindings consolidate shmem tables and reduce size by type spec constant fix compiler warning fix missing Q2_0 type fix unused warning when integer dot glslc support is missing use minimal shmem size 8 instead of 1 to workaround cm2 compiler bug fix missing Q2_0 type in cm2 matmul fix types * remove LUT quants from unified shader * clean up * restore coopmat2 q4_k/q5_k optimization * split out q4_k/q5_k cm2 shader to fix Ampere regression * revert iq shmem table renames * simplify cm2 code with single uint8_t buffer * fix fp4 extension use switch being overwritten by generic shader * clean up * adapt TQ1_0 changes * adapt ggml-org#27471 f16 Intel tuning changes
330 commits behind. Merging current master gives 13 conflicting files / 47 hunks, and ggml-org#25773 alone accounts for 15 of them -- that step is where the ROCmFPx types must be re-expressed in the new spec-constant matmul scheme. Four steps, benchmark each. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ml-org#25773) * vulkan: use spec constant for mul mat type_a vulkan: use map for mul_mm shapes cleanup fix indentation fix cm2 and shmem init fix cm2 spec constants fix cm2 bindings consolidate shmem tables and reduce size by type spec constant fix compiler warning fix missing Q2_0 type fix unused warning when integer dot glslc support is missing use minimal shmem size 8 instead of 1 to workaround cm2 compiler bug fix missing Q2_0 type in cm2 matmul fix types * remove LUT quants from unified shader * clean up * restore coopmat2 q4_k/q5_k optimization * split out q4_k/q5_k cm2 shader to fix Ampere regression * revert iq shmem table renames * simplify cm2 code with single uint8_t buffer * fix fp4 extension use switch being overwritten by generic shader * clean up * adapt TQ1_0 changes * adapt ggml-org#27471 f16 Intel tuning changes
Overview
This is an attempt to reduce the amount of matrix multiplication shaders we need by using a spec constant for the type of the A input, similar to how it works for KV cache types in Flash Attention. Additionally, it replaces the current static s, m and l shapes with a map and allows improved tuning through more shapes and a custom selection function, which should help with work like #24407.
It's still draft because currently it regresses Nvidia, because for dynamic type selection all quants with shared memory requirements must also be available. I thought it would eliminate those that are not used, this works on AMD, but not on Nvidia. @jeffbolznv Do you know why? There are other ways to resolve this, hopefully I'll find something that works.
Requirements