Repository navigation
Vulkan: RMS norm for short rows (per-head Q/K norm 2.4-2.7x faster, bit-identical) - #37
Open
danielhanchen wants to merge 1 commit into
Open
danielhanchen wants to merge 1 commit into
danielhanchen wants to merge 1 commit into
Conversation
…st 128 elements (bit-identical)
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on the upstream sync branch (#35,
sync-upstream-a77d362, now at upstreammaster-995-fd21ecf, commit344a3b66). Addsscripts/unsloth/ggml-patches/0015-ggml-vulkan-rms-norm-short-rows.patch(numbered 0015 because #36 adds 0014; the CUDA and CPU branches use 0016 to 0018, and all of them apply together in order on ggmlc1f80e5).Problem
ggml-vulkan's RMS norm runs one 512-thread workgroup per row. The DiTs Studio runs on sd.cpp normalise Q and K per head, so the rows are 128 elements long and there are many of them: Z-Image at 1024x1024 has 30 heads x 4128 tokens = 123,840 rows per call. Three quarters of every workgroup only adds zeros and waits on barriers.
Per-op GPU time (
GGML_VK_PERF_LOGGER=1, B200, Vulkan, one steady denoise step of Z-Image Turbo Q4_K_M at 1024x1024, 1198 ms in total):RMS_NORM_MULRMS_NORM_MULThe per-head norm was 15% of the step, behind only the quantised matmuls (48%) and flash attention (18%).
Fix
A second build of
rms_norm.compwithBLOCK_SIZE = 128(rms_norm_small_f32, plus the fused-multiply variant through the same specialisation constant as today).ggml_vk_op_get_pipelinepicks it forGGML_OP_RMS_NORMwhenne00 <= 128and the add-partials path is not in use. Everything else keeps the 512-thread pipelines.The result is bit-identical by construction. With at most 128 columns, thread
tholdsx[t]^2(or 0) in both versions, and the 512-thread tree only adds exact zeros in its first two halving steps (strides 256 and 128) before running the same strides 64 down to 1 as the 128-thread tree. The scale and the multiply loop are unchanged.Results
B200 (Vulkan,
NV_coopmat2), base =sync-upstream-fd21ecf, head = this branch, same build flags (the prebuilt Vulkan leg's). Studio's Vulkan argv (--diffusion-fa, with--conv-direct onhere because the perf logger aborts in ggml-vulkan on the im2col VAE graph at 1024). The card is shared with other jobs, so totals move between runs; base and head ran back to back, interleaved. Medians over the steady denoise steps, two runs per arm:RMS_NORMGPU time per step, baseAt Z-Image 1024 that is about 30 to 55 ms of a 390 to 570 ms step in these runs, roughly 8%. Wall-clock sampling did not resolve it on this host (host load moves Vulkan wall time by tens of percent between identical runs), and the exclusive-card timing was not run, so the end-to-end win is the GPU-time estimate above, not a measured wall-clock number.
Reproduce:
scripts/sdcpp_speed/vk_bench.py --cases zimage1024,klein1024,zimage512 --arms base=<base sd-cli>,rms=<head sd-cli> --variants studio_cd_on,studio_cd_on+perf --env studio_cd_on+perf:GGML_VK_PERF_LOGGER=1 --reps 2, thenscripts/sdcpp_speed/vk_perf_parse.py <log>.Accuracy
test-backend-ops test -o RMS_NORM -b Vulkan0on the head: 55/55 OK (includes 64-column rows, which take the new pipeline).Platforms