Skip to content

hip: shared perf (MMQ MoE tiles + mmvdq, prefill plumbing) - #154

Open
raufaser wants to merge 2 commits into
Anbeeld:v0.4.7from
raufaser:pr-b-shared-perf
Open

hip: shared perf (MMQ MoE tiles + mmvdq, prefill plumbing)#154
raufaser wants to merge 2 commits into
Anbeeld:v0.4.7from
raufaser:pr-b-shared-perf

Conversation

@raufaser

@raufaser raufaser commented Sep 10, 2026

Copy link
Copy Markdown

Shared-code performance work (compiles for all backends; may affect CUDA/Vulkan behavior -- maintainer decides). This is the sibling of PR #153 (HIP-only, provably no CUDA impact). KVarN-specific changes are NOT here -- they live in PR #149.

What's inside

1. MMQ tile sizing + dequant-float matvec mmvdq (ggml-cuda/mmq.cuh, all 9 mmq-config-*.cuh, new mmvdq.cu/.cuh, ggml-cuda.cu dispatch)
Ideas from nasone32's RDNA3-7900xtx-opt (MMQ tile sized to MoE tokens-per-expert, dequant-float matvec for Q4_K/Q5_K/Q6_K) plus the upstream llama.cpp #24546 MMQ cherry-pick. The new mmvdq kernel is default-OFF except RDNA3_5 but env-activatable on any backend. Behaviorally CUDA/CDNA-neutral by analysis (new branches require RDNA3 arch checks or the opt-in env), but this is shared code, so review + CI decide. Review fix: GGML_CUDA_DQ_MMV/GGML_CUDA_DQ_Q6K now parse strict 0/1 (anything else warns once and keeps the arch default); all three knobs (DQ_MMV, DQ_Q6K, DQ_ROWS) are documented in docs/beellama-args.md.

2. Prefill plumbing (ggml-backend-meta.cpp, src/llama.cpp, ggml-cuda.cu, ggml-cpu/llamafile/sgemm.cpp)
Ideas from stew675's llama-cpp-rdna-boosts blocks 05/06/07/09/11: Meta headroom 32->128, single-device Meta-wrapper skip, mmvdq decode/GLU dispatch, prefill graph-skip + graph-opt default. Measured on the final tree (27B Q5_K_S, kvarn6, gfx1100): pp512 ~466 t/s, pp4096 ~173 t/s -- neutral vs stock v0.4.7 (same numbers; the earlier 823 figure was a stale dev-tree reading, corrected). (Task 6 block 08 fused-core prefill kernels deliberately deferred -- needs rework against this fork's fusion dispatch.) Review fix: the prefill graph skip is now #if defined(GGML_USE_HIP)-gated (compiles out on CUDA -- zero NVIDIA behavior change by construction) with the keying theory trimmed; final-tree A/B shows it perf-neutral (gate on 466.5 vs off 465.4-467.5 -- the earlier +6.7% dev-tree reading did not reproduce and is scheduled for investigation).

3. MMQ vec-dot register hoists (ggml-cuda/mmq-vec-dot.cuh)
Q8_1/Q6_K scale hoists in the shared MMQ path. The Q6_K fold re-associates one scale multiply -- empirically bit-identical KLD on the test corpus, but numerics-touching shared code, hence PR B and not PR A. Review fix: GGML_CUDA_MMQ_J_MAX is now validated (multiples of 8 in 8..512, warn + ignore otherwise) and cached instead of re-read per call.

4. Single-device split state (src/llama.cpp)
Review fix: both single-device branches now publish n_devices = 1 + model pointer, so the split-state callback is never reached with uninitialized data. Parity: 4B PPL identical with explicit single-device tensor-split vs default (HIP 13.4884 both legs, CUDA 13.3835 both legs).

Deliberately NOT in this PR

Validation (gfx1100 RX 7900 XTX + RTX 4090 CUDA gate)

Kudos

Base: v0.4.7.

@Anbeeld

Anbeeld commented Sep 10, 2026

Copy link
Copy Markdown
Owner

Requesting changes on the prefill graph skip; the other items are minor.

Blocking: ggml/src/ggml-cuda/ggml-cuda.cu:4543. if (cgraph->n_nodes > 0 && cgraph->nodes[0]->ne[1] > 1) { use_cuda_graph = false; } is not arch-gated, so it turns off CUDA graph replay for multi-token graphs on NVIDIA as well. The description calls the change CUDA/CDNA-neutral, and the stated reason (prefill graphs use varying ubatch sizes, so each becomes a separate graph key) does not match the code: ggml_cuda_graph_get_key() returns cgraph->nodes[0] (ggml-cuda.cu:2652-2654), a tensor pointer rather than a shape, and the cgraph->uid fast path at ggml-cuda.cu:2662-2667 exists so a stable prefill graph is reused without rescanning properties. Please gate this to the RDNA path behind the gfx1100 measurement, or submit it as a CUDA default change with NVIDIA llama-bench -p 512 and -p 2048 numbers and maintainer sign-off.

Minor:

  • ggml/src/ggml-cuda/mmvdq.cu:7-11: dq_env_override returns 1 for every value except the exact string "0", so GGML_CUDA_DQ_MMV=false, =off, and ="" all enable the prototype matvec. Because the override ignores arch_default (mmvdq.cu:13-20), the same variable can force it on NVIDIA and on CDNA/RDNA2/RDNA4, where only gfx1100 and gfx1151 were validated. Parse these as explicit 0/1 and document GGML_CUDA_DQ_MMV, GGML_CUDA_DQ_Q6K, and GGML_CUDA_DQ_ROWS in docs/beellama-args.md.
  • ggml/src/ggml-cuda/mmq.cuh:402-405: the new GGML_CUDA_MMQ_J_MAX is applied after ret -= ret % 8 and is not rounded or validated, so 4, 5, or 0 make ggml_cuda_mmq_get_J_max return 0. That value sizes the q8_1 scratch (mmq.cu:155,224), while mul_mat_q_switch_J (mmq.cuh:1698-1707) still selects J from the 8..128 table without consulting it, so sizing and selection can disagree. I did not establish an actual out-of-bounds write from this. Please validate and cache the value, or drop the knob until it is used.
  • src/llama.cpp:170-171 and 220-221: skipping the single-device Meta wrapper leaves model.get_split_state_ud unset (src/llama-model.h:710 has no initializer and src/llama-model.cpp:373-376 dereferences it), and switches every is_meta branch in the fork to the non-meta path (src/llama-context.cpp:275, src/llama-kv-cache.cpp:66,991, src/llama-kv-cache-kvarn.cpp:58 onward). I confirmed no current caller reaches the callback without a Meta device, so the unset field is latent, but -sm tensor on one device is user-reachable and the KVarN/DFlash/KV-tail validation for that configuration is now untested. Initialize the struct in that path and record a single-GPU tensor-split parity check.

What I checked: the nine mmq-config-*.cuh edits keep identical tile numbers for every NVIDIA-reachable config, both mmq-vec-dot.cuh hunks sit inside AMD_MFMA_AVAILABLE || AMD_WMMA_AVAILABLE, and I derived the dq_setup_q4_K/dq_dot_q4_K element, activation, and get_scale_min_k4 mapping against the Q4_K layout. Q5_K and Q6_K geometry got a structural read only, not the same derivation. No builds or tests were run.

…ing, J_MAX validation, split-state init)
@raufaser

Copy link
Copy Markdown
Author

All items addressed in 31a7396 (on top of 93247bc, no force-push). Builds + validation below are new runs on the final tree: HIP = RX 7900 XTX gfx1100 (TheRock, clean GPU_TARGETS=gfx1100 configure), CUDA = RTX 4090 sm_89.

Blocking (graph skip): wrapped in #if defined(GGML_USE_HIP) -- it compiles out on CUDA, so zero NVIDIA behavior change by construction. The keying-theory sentences are trimmed to the measurement. One correction to my own comment: final-tree A/B (27B Q5_K_S, kvarn6, pp512) shows the skip perf-neutral -- gate on 466.5 vs gate off 465.4 (pr-a) / 467.5 (stock v0.4.7), all within 0.5%. The earlier +6.7% dev-tree reading did not reproduce; the comment now reports neutrality and the original claim is scheduled for investigation. Decode keeps graph replay.

  • mmvdq.cu: strict 0/1 parsing with warn-once + arch-default fallback (mirrors the DQ_ROWS pattern); mmvdq.cuh + ggml-cuda.cu comments updated; GGML_CUDA_DQ_MMV, GGML_CUDA_DQ_Q6K, GGML_CUDA_DQ_ROWS documented in docs/beellama-args.md. Functional proof on HIP: =bogus warns and falls back (MUL_MAT still 1293/1293), =1 forces the DQ path on (MUL_MAT 1293/1293 -- DQ kernels correct on gfx1100), =0/="" behave as unset.
  • mmq.cuh: GGML_CUDA_MMQ_J_MAX accepts multiples of 8 in 8..512, warns + ignores otherwise, and is cached (no more per-call getenv). Covered by MUL_MAT on HIP + CUDA.
  • llama.cpp: both single-device branches publish n_devices = 1 + model. Parity: 4B PPL identical with explicit single-device tensor-split vs default -- HIP 13.4884 both legs (--device ROCm0 -sm tensor), CUDA 13.3835 both legs (--device CUDA0 -sm tensor).

Validation table:

The only build failure is the known pre-existing test-kv-history.exe link error on Windows (documented v0.4.7 quirk); everything else links.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants