Skip to content

feat(rocm): grouped + non-grouped keep-quant expert GEMM — the MoE-path terminus (issue #41) - #523

Draft
VikashLoomba wants to merge 4 commits into
mudler:mainfrom
VikashLoomba:row/ROCM-GG-ON-MOE
Draft

feat(rocm): grouped + non-grouped keep-quant expert GEMM — the MoE-path terminus (issue #41)#523
VikashLoomba wants to merge 4 commits into
mudler:mainfrom
VikashLoomba:row/ROCM-GG-ON-MOE

Conversation

@VikashLoomba

Copy link
Copy Markdown
Contributor

Row

BACKEND-ROCM — the last kernel blocker for MoE-bearing models on discrete ROCm. Issue #41. Stacked on #509 (MoE combine/gate chain).

What changed

NEW src/vt/rocm/rocm_grouped_gemm.hip — ports the keep-quant expert GEMM family from cuda_quant_dot.cu 1:1:

  • kMatmulBTQuant (op 74, non-grouped) + kMatmulBTQuantGrouped (op 75, grouped over expert_ids)
  • Q8_0 / Q4_K / Q5_K / Q6_K weight formats (the ones the target GDN-MoE GGUFs carry); Q8_0 + Q8_K activation quantizers
  • DotQ8_0/DotQ4K/DotQ5K/DotQ6K superblocks, __dp4a → portable Dp4a (bit-identical integer core), __shfl_down_sync reduction
  • Registering the non-grouped op is what flips GgufQuantComputeAvailable() on ROCm — the grouped op alone leaves the loader dequantizing experts to bf16.

Cross-device case: all four formats vs the CPU keep-quant oracle, valid random blocks + real activations.

Evidence (4× gfx1100, ROCm 7.14, Release)

  • grouped-quant case: 16/16 assertions across Q8_0/Q4_K/Q5_K/Q6_K, NMSE ≤ 5e-4 vs the CPU keep-quant reference
  • ctest -R 'rocm|cross_device': 4/4; full ctest: only the 5 pre-existing host/lane failures (zero new)
  • E2E: Qwen3.6-35B-A3B Q4_K_M (21GB GDN-MoE GGUF) runs end to end on one gfx1100 with keep-quant active — op 74 AND op 75 resolve vt-native, zero CPU-ref fallback, correct output. Requires --max-num-seqs 1 to fit one 24GB card (the GDN state pool otherwise pushes past — a residency note, not a kernel defect; confirmed via a backend-Alloc instrumentation run showing genuine cumulative ~23.8 GiB).
  • preflight --staged + trailers green

Speed claims

  • This PR makes NO speed claim.

Honest gaps

  • The K-quant formats Q2_K/Q3_K and the IQ2/IQ3 family are NOT ported (throw loudly); the target models don't use them. They follow the same skeleton.
  • The per-call hipMalloc/hipFree activation scratch is correctness-grade; a queue-owned grow-only pool is a perf lever (decode-step churn).
  • The 35B fits only with --max-num-seqs 1 on 24GB; multi-GPU expert sharding or host streaming is the follow-on for bigger MoE / longer context.

@localai-bot

Copy link
Copy Markdown
Collaborator

Reviewed as part of a sweep over the open external PRs. The GEMM port is the strongest part of this — I diffed DotQ4K/DotQ5K/DotQ6K against the donor line by line and DotQ6K is byte-for-byte; all four cited anchors in cuda_quant_dot.cu resolve. Registration goes through the existing seam.

One blocker, and it is in how the op is switched on rather than in the kernel.

Registering kMatmulBTQuant flips keep-quant loader-wide for formats this kernel cannot execute.

GgufQuantComputeAvailable() (gguf_keep_quant.cpp:74) is a boolean: OpRegistered(kMatmulBTQuant, current_device). FromEnv() sets p.keep_quant from it, and KeepQuantDType admits anything vt::cpu::HasQuantDotKernel accepts — which per cpu_quant_traits.cpp:30-98 is Q4_0, Q8_0, Q2_K, Q3_K, Q4_K, Q5_K, Q6_K, IQ2_XXS, IQ3_XXS, IQ2_S and MXFP4. Twelve formats.

rocm_grouped_gemm.hip:519,592 implements four and throws for the rest. CUDA survives the same boolean because cuda_quant_dot.cu:1841-1846 falls back to the CPU kernel on an unsupported dtype. This PR's own docs/USAGE.md edit states the opposite for ROCm: "On a discrete card there is no CPU fallback tier."

So a Q4_0 or Q2_K or IQ2 GGUF that loads and generates correctly on ROCm today would now keep its blocks quantized at load and throw vt rocm: matmul_bt_quant: unsupported weight dtype on the first forward pass — after the whole model is resident on the card. That is a functional regression of a working path, discovered at the worst possible moment, and it is not mentioned in the PR body.

Same flip has collateral: expand_nkkeep_f16 turns ON, so an F16 file weight stays kF16 and MatmulBTKernelRocm (rocm_matmul_hipblaslt.hip:446-450) accepts only bf16/bf16 or f32/f32 → vt rocm: matmul_bt: unsupported.

The fix is a per-dtype availability query rather than a boolean, or a refusal at load time rather than forward time. Related: the refusal message at :592-594 lists Q5_K on both the ported and the not-yet sides, and does not name Q4_0 or MXFP4 — the two formats that actually drive this.

Second thing, worth knowing before you invest further. The per-call hipMalloc/hipFree + hipStreamSynchronize (:478,488-489,500,515-516,545,558-559,572,587-588) is not only the decode-step churn you name honestly in the body. Both are illegal under hipGraph stream capture, which hard-blocks the ROCm decode-graph work in #473/#332 — the row worth ~3x decode. The donor uses EnsureScratch(), a per-stream grow-only pool, and never synchronizes; rocm_device_bind.h:21 shows the codebase already cares about capture-safety. (Also: qact leaks if Check() throws between the malloc and the free.)

Smaller: Dp4a at :168-172 unrolls into four scalar MACs where HIP exposes __dp4a natively (lowers to v_dot4_i32_i8 on gfx9/gfx10+) — that is the innermost loop of every expert GEMM, so roughly 4x the instruction count on the hottest path for a one-line change. And there is no spec: grep -rn 'ROCM-GG-ON-MOE' .agents/ docs/ returns nothing, so the scope, gates and stop conditions for this work are not written down anywhere, and the unported arms are not recorded as owed.

Note also that the non-grouped op — the one that carries this PR's headline mechanism — has no test at all; only kMatmulBTQuantGrouped is exercised. And both new cases guard with if (!OpAvailable(op, dt)) continue;, so deleting the RegisterOp line makes them skip silently and stay green, which means nothing currently proves the registration.

On CI: the four red checks are all infrastructure, none yours. pr-size and agent-record both die on base must be an ancestor of head (unrebased fork) — and that is now fixed on our side in #619, so a rebase should clear them. Windows is the known-broken arm.

No AMD hardware here, so your 16/16, the ctest results and the 35B e2e run could not be reproduced and I am not disputing them; the finding above is read from the loader chain end to end.

localai-bot pushed a commit that referenced this pull request Aug 13, 2026
…(W1, #332) (#473)

Implements the vt::Backend graph-capture seam on hipGraph (W1 of #332), mirroring
src/vt/cuda/cuda_backend.cu call for call.

Merged with the row's performance rationale REFUTED and recorded as such. W3
measured capture at +3.2% / +0.6% / -1.0%, not the ~2.2-3x §1 predicted, and the
spec's D7 plus the inline note in §1 now say so rather than leaving a live
rationale for the next agent to re-derive. That refutation was reported by the
contributor against their own interest, which is the behaviour this protocol
exists to produce.

Merged anyway on the seam argument, which is independent of the decode number:
graph capture had exactly one real implementation (CUDA), and a one-implementation
abstraction is unproven. hipGraph is the cheapest available second. Runtime cost
today is zero -- RocmPlatform does not override support_static_graph_mode(), so
nothing in the engine reaches the new code.

Review verified isolation three ways: every decode-graph call site ANDs
SupportsGraphCapture() with support_static_graph_mode(); rocm_backend.hip appears
zero times in a CPU build's compile_commands.json; and there is zero drift on
every touched file across the 194-commit gap. A mutation of the EndCaptureGraph
seam signature turns the new test red, so it genuinely guards seam drift on
machines with no AMD hardware.

Known-broken windows-msvc-* are the PR-only arm (#584), not this change.

Carried forward: #523's per-call hipMalloc/hipFree and hipStreamSynchronize are
illegal under hipGraph capture and must be reconciled before this capability
could ever be switched on.
VikashLoomba added a commit to VikashLoomba/vllm.cpp that referenced this pull request Aug 14, 2026
…ler#523 review rework

Review sweep findings (localai-bot, 2026-08-13), all accepted:

1. BLOCKER -- registering kMatmulBTQuant flipped keep-quant loader-wide via
   the boolean GgufQuantComputeAvailable() while the ROCm kernel implements 4
   of the 12 admitted formats, and there is no CPU fallback tier on a discrete
   card: a Q4_0/Q2_K/IQ2 model that worked before would keep blocks quantized
   and throw at first forward. FIX: RouteGgufTensor now consults
   DeviceKeepQuantSupported() -- ROCm keeps {Q8_0,Q4_K,Q5_K,Q6_K} quantized
   and everything else keeps its pre-existing expand_bf16 residency, so no
   load regresses and no forward throws. The same flip's keep_f16 collateral
   (MatmulBTKernelRocm accepts bf16/f32 only) is gated off on ROCm the same
   way. The device gate lives in the ROUTING decision, not KeepQuantDType --
   the dtype query also serves the device-independent residency tests.
2. Capture safety: the per-call hipMalloc/hipFree/hipStreamSynchronize on the
   activation-quant scratch is illegal under hipGraph stream capture (blocks
   mudler#473/mudler#332) and the free-after-launch leaked qact when Check() threw. Now a
   grow-only per-stream pool on hipMallocAsync, retire-never-free, mirroring
   the donor's EnsureScratch + graph_safe_scratch.h discipline; the syncs are
   gone with the frees.
3. Teeth: the non-grouped kMatmulBTQuant (the headline mechanism) gains its
   own cross-device case at the real shapes, and both new cases REQUIRE the
   ROCm registration instead of skipping silently when it is dropped. The
   loader-side routing gate gets a dedicated case in test_gguf_keep_quant
   (Q4_0/Q2_K expand, Q8_0/Q4_K/Q5_K/Q6_K keep, keep_f16 off on ROCm) plus a
   device-aware expectation in the TOTAL routing-table case.
4. The refusal messages name the owed formats (Q4_0/Q2_K/Q3_K/IQ2_*/IQ3_*/
   MXFP4) instead of double-listing Q5_K.
5. __dp4a is NOT used: ROCm 7.14's gfx1100 toolchain has no __dp4a and no
   dot1-insts target feature (RDNA3 dropped the int8 dot4 hardware) --
   verified by compile probe; the portable Dp4a helper stands, with the
   evidence recorded in .agents/specs/rocm-gg-keep-quant.md.

Gates (gfx1100, flock): test_gguf_keep_quant 38/38 (incl. the new routing
case); test_backend_cross_device 22/22 (376 assertions) incl. the new
non-grouped case; 35B Q4_K_M e2e still correct ('The capital of France is' ->
' Paris, a city that has been the').

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: pi:kimi-k3 [pi]
VikashLoomba added a commit to VikashLoomba/vllm.cpp that referenced this pull request Aug 14, 2026
…ed -- the work predated it in mudler#523, which the review correctly flagged; recorded as the process miss it was)

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: pi:kimi-k3 [pi]
VikashLoomba added a commit to VikashLoomba/vllm.cpp that referenced this pull request Aug 14, 2026
…udler#506 review rework

Review sweep findings (localai-bot, 2026-08-13), all accepted and verified
against the donor at pin 55596792:

1. OOB device write on odd output dim: the donor launches wvSplitK_hf_sml_
   only under (Kbp*N <= max_lds) && (M_in % YTILE == 0) (skinny_gemms.cu:1217)
   and the port dropped the %YTILE half -- the unguarded y=1 store lands past
   the output on odd N. Restored as N % 2 == 0 (our YTILE=2).
2. No arch guard: the port carries only the wave32 reduction arm
   (__shfl_xor(x,16)); the donor branches to ROW_BCAST15/31 on gfx9
   (skinny_gemms.cu:489-496) and double-guards dispatch
   (on_gfx9()/on_gfx1x() + compile-time ifdef). The gfx9 arm is NOT ported, so
   dispatch now refuses non-wave32 arches via CapabilityFromGcnArch(
   DeviceArchName()) instead of compiling and silently mis-reducing.
3. The m > 8 lower bound (utils.py:181, the feature-dim bound) restored as
   N > 8 -- at N==1 the first wave already wrote OOB.
4. The upstream test is now ported for real: the applicable
   NKM_FACTORS_WVSPLITK list (tokens 1-4 = our template arms), xavier on/off,
   and the ELEMENTWISE tolerance (atol = eps_bf16*sqrt(K), rtol = 1e-2 --
   torch assert_close semantics) replacing the aggregate NMSE that ~10 wrong
   elements would have passed. Added boundary shapes: features<=8 and odd
   features must route BLAS and stay correct; K%8!=0 declines; K%512!=0
   exercises the K-tail the old test never reached. Outputs write into a
   0xDEAD-sentinel guard band so any residual OOB store fails outright.
   Mutation-proven: with the N%2 guard removed the sentinel band is corrupted
   and the case fails; restored, green.

Deferred with reason (recorded in .agents/specs/rocm-skinny-gemm.md): fp16
(port is bf16-only), bias (the vt::MatmulBT seam has no bias operand), padded
strides (the dispatch precondition is contiguous rows), the fp8/rc variants.
The gfx9 wave64 arm is owed future work, guarded out loudly for now.

Allowlist: main's re-sorted file taken wholesale + VT_ROCM_SKINNY inserted
once in sorted position (per the review's merge note).

Gates (gfx1100, flock): test_backend_cross_device 20/20 incl. the ported
sweep; the 35B Q4_K_M decode e2e was re-measured for mudler#523's stack, unchanged
by this guard-only dispatch change (the skinny path fires identically at the
production shapes).

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: pi:kimi-k3 [pi]
VikashLoomba added a commit to VikashLoomba/vllm.cpp that referenced this pull request Aug 14, 2026
…udler#506 review rework

Review sweep findings (localai-bot, 2026-08-13), all accepted and verified
against the donor at pin 55596792:

1. OOB device write on odd output dim: the donor launches wvSplitK_hf_sml_
   only under (Kbp*N <= max_lds) && (M_in % YTILE == 0) (skinny_gemms.cu:1217)
   and the port dropped the %YTILE half -- the unguarded y=1 store lands past
   the output on odd N. Restored as N % 2 == 0 (our YTILE=2).
2. No arch guard: the port carries only the wave32 reduction arm
   (__shfl_xor(x,16)); the donor branches to ROW_BCAST15/31 on gfx9
   (skinny_gemms.cu:489-496) and double-guards dispatch
   (on_gfx9()/on_gfx1x() + compile-time ifdef). The gfx9 arm is NOT ported, so
   dispatch now refuses non-wave32 arches via CapabilityFromGcnArch(
   DeviceArchName()) instead of compiling and silently mis-reducing.
3. The m > 8 lower bound (utils.py:181, the feature-dim bound) restored as
   N > 8 -- at N==1 the first wave already wrote OOB.
4. The upstream test is now ported for real: the applicable
   NKM_FACTORS_WVSPLITK list (tokens 1-4 = our template arms), xavier on/off,
   and the ELEMENTWISE tolerance (atol = eps_bf16*sqrt(K), rtol = 1e-2 --
   torch assert_close semantics) replacing the aggregate NMSE that ~10 wrong
   elements would have passed. Added boundary shapes: features<=8 and odd
   features must route BLAS and stay correct; K%8!=0 declines; K%512!=0
   exercises the K-tail the old test never reached. Outputs write into a
   0xDEAD-sentinel guard band so any residual OOB store fails outright.
   Mutation-proven: with the N%2 guard removed the sentinel band is corrupted
   and the case fails; restored, green.

Deferred with reason (recorded in .agents/specs/rocm-skinny-gemm.md): fp16
(port is bf16-only), bias (the vt::MatmulBT seam has no bias operand), padded
strides (the dispatch precondition is contiguous rows), the fp8/rc variants.
The gfx9 wave64 arm is owed future work, guarded out loudly for now.

Allowlist: main's re-sorted file taken wholesale + VT_ROCM_SKINNY inserted
once in sorted position (per the review's merge note).

Gates (gfx1100, flock): test_backend_cross_device 20/20 incl. the ported
sweep; the 35B Q4_K_M decode e2e was re-measured for mudler#523's stack, unchanged
by this guard-only dispatch change (the skinny path fires identically at the
production shapes).

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: pi:kimi-k3 [pi]
@VikashLoomba

Copy link
Copy Markdown
Contributor Author

The blocker and the rest accepted and reworked (commits 2f46038 + 29aed2f + 22ed7b9, rebased onto current main):

The blocker: keep-quant residency is now gated per-format by the running device's actual kernel set (DeviceKeepQuantSupported in the routing decision, not in the dtype-validity query). ROCm keeps {Q8_0, Q4_K, Q5_K, Q6_K} quantized; everything else keeps its pre-existing expand_bf16 residency — so a Q4_0/Q2_K/IQ2 model loads and generates exactly as before, and nothing reaches forward-time throw. The keep_f16 collateral is gated the same way (MatmulBTKernelRocm accepts bf16/f32 only). The refusal messages now name the actually-owed formats.

Capture safety: the per-call hipMalloc/hipFree/hipStreamSynchronize is replaced by a grow-only per-stream pool on hipMallocAsync, retire-never-free — mirroring the donor's EnsureScratch + graph_safe_scratch.h discipline. The qact throw-leak is gone with the frees. This also unblocks the hipGraph lane (#473).

Teeth: the non-grouped kMatmulBTQuant gains its own cross-device case (it had none), and both quant cases now REQUIRE the ROCm registration instead of skipping — which immediately caught a real regression during the rebase (the merge had silently dropped the registrations; restored, and the teeth are why it was caught). test_gguf_keep_quant gains the device-gated routing case (Q4_0/Q2_K expand, Q8_0/Q4_K/Q5_K/Q6_K keep, keep_f16 off on ROCm) plus a device-aware expectation in the TOTAL routing-table case.

On __dp4a: I verified rather than accepted — ROCm 7.14's gfx1100 toolchain has no __dp4a intrinsic, and __builtin_amdgcn_sdot4 errors with "needs target feature dot1-insts" (RDNA3 dropped the int8 dot4 hardware). The portable helper stands; the probe evidence is recorded in the spec.

The spec debt: .agents/specs/rocm-gg-keep-quant.md (the rework) + the original spike spec committed belatedly with the process miss named in its commit message.

Gates (gfx1100, flock): test_gguf_keep_quant 38/38, test_backend_cross_device 22/22, 35B Q4_K_M e2e correct on one card.

VikashLoomba added a commit to VikashLoomba/vllm.cpp that referenced this pull request Aug 14, 2026
…ler#523 review rework

Review sweep findings (localai-bot, 2026-08-13), all accepted:

1. BLOCKER -- registering kMatmulBTQuant flipped keep-quant loader-wide via
   the boolean GgufQuantComputeAvailable() while the ROCm kernel implements 4
   of the 12 admitted formats, and there is no CPU fallback tier on a discrete
   card: a Q4_0/Q2_K/IQ2 model that worked before would keep blocks quantized
   and throw at first forward. FIX: RouteGgufTensor now consults
   DeviceKeepQuantSupported() -- ROCm keeps {Q8_0,Q4_K,Q5_K,Q6_K} quantized
   and everything else keeps its pre-existing expand_bf16 residency, so no
   load regresses and no forward throws. The same flip's keep_f16 collateral
   (MatmulBTKernelRocm accepts bf16/f32 only) is gated off on ROCm the same
   way. The device gate lives in the ROUTING decision, not KeepQuantDType --
   the dtype query also serves the device-independent residency tests.
2. Capture safety: the per-call hipMalloc/hipFree/hipStreamSynchronize on the
   activation-quant scratch is illegal under hipGraph stream capture (blocks
   mudler#473/mudler#332) and the free-after-launch leaked qact when Check() threw. Now a
   grow-only per-stream pool on hipMallocAsync, retire-never-free, mirroring
   the donor's EnsureScratch + graph_safe_scratch.h discipline; the syncs are
   gone with the frees.
3. Teeth: the non-grouped kMatmulBTQuant (the headline mechanism) gains its
   own cross-device case at the real shapes, and both new cases REQUIRE the
   ROCm registration instead of skipping silently when it is dropped. The
   loader-side routing gate gets a dedicated case in test_gguf_keep_quant
   (Q4_0/Q2_K expand, Q8_0/Q4_K/Q5_K/Q6_K keep, keep_f16 off on ROCm) plus a
   device-aware expectation in the TOTAL routing-table case.
4. The refusal messages name the owed formats (Q4_0/Q2_K/Q3_K/IQ2_*/IQ3_*/
   MXFP4) instead of double-listing Q5_K.
5. __dp4a is NOT used: ROCm 7.14's gfx1100 toolchain has no __dp4a and no
   dot1-insts target feature (RDNA3 dropped the int8 dot4 hardware) --
   verified by compile probe; the portable Dp4a helper stands, with the
   evidence recorded in .agents/specs/rocm-gg-keep-quant.md.

Gates (gfx1100, flock): test_gguf_keep_quant 38/38 (incl. the new routing
case); test_backend_cross_device 22/22 (376 assertions) incl. the new
non-grouped case; 35B Q4_K_M e2e still correct ('The capital of France is' ->
' Paris, a city that has been the').

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: pi:kimi-k3 [pi]
VikashLoomba added a commit to VikashLoomba/vllm.cpp that referenced this pull request Aug 14, 2026
…ed -- the work predated it in mudler#523, which the review correctly flagged; recorded as the process miss it was)

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: pi:kimi-k3 [pi]
VikashLoomba added a commit to VikashLoomba/vllm.cpp that referenced this pull request Aug 14, 2026
…ler#523 review rework

Review sweep findings (localai-bot, 2026-08-13), all accepted:

1. BLOCKER -- registering kMatmulBTQuant flipped keep-quant loader-wide via
   the boolean GgufQuantComputeAvailable() while the ROCm kernel implements 4
   of the 12 admitted formats, and there is no CPU fallback tier on a discrete
   card: a Q4_0/Q2_K/IQ2 model that worked before would keep blocks quantized
   and throw at first forward. FIX: RouteGgufTensor now consults
   DeviceKeepQuantSupported() -- ROCm keeps {Q8_0,Q4_K,Q5_K,Q6_K} quantized
   and everything else keeps its pre-existing expand_bf16 residency, so no
   load regresses and no forward throws. The same flip's keep_f16 collateral
   (MatmulBTKernelRocm accepts bf16/f32 only) is gated off on ROCm the same
   way. The device gate lives in the ROUTING decision, not KeepQuantDType --
   the dtype query also serves the device-independent residency tests.
2. Capture safety: the per-call hipMalloc/hipFree/hipStreamSynchronize on the
   activation-quant scratch is illegal under hipGraph stream capture (blocks
   mudler#473/mudler#332) and the free-after-launch leaked qact when Check() threw. Now a
   grow-only per-stream pool on hipMallocAsync, retire-never-free, mirroring
   the donor's EnsureScratch + graph_safe_scratch.h discipline; the syncs are
   gone with the frees.
3. Teeth: the non-grouped kMatmulBTQuant (the headline mechanism) gains its
   own cross-device case at the real shapes, and both new cases REQUIRE the
   ROCm registration instead of skipping silently when it is dropped. The
   loader-side routing gate gets a dedicated case in test_gguf_keep_quant
   (Q4_0/Q2_K expand, Q8_0/Q4_K/Q5_K/Q6_K keep, keep_f16 off on ROCm) plus a
   device-aware expectation in the TOTAL routing-table case.
4. The refusal messages name the owed formats (Q4_0/Q2_K/Q3_K/IQ2_*/IQ3_*/
   MXFP4) instead of double-listing Q5_K.
5. __dp4a is NOT used: ROCm 7.14's gfx1100 toolchain has no __dp4a and no
   dot1-insts target feature (RDNA3 dropped the int8 dot4 hardware) --
   verified by compile probe; the portable Dp4a helper stands, with the
   evidence recorded in .agents/specs/rocm-gg-keep-quant.md.

Gates (gfx1100, flock): test_gguf_keep_quant 38/38 (incl. the new routing
case); test_backend_cross_device 22/22 (376 assertions) incl. the new
non-grouped case; 35B Q4_K_M e2e still correct ('The capital of France is' ->
' Paris, a city that has been the').

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: pi:kimi-k3 [pi]
VikashLoomba added a commit to VikashLoomba/vllm.cpp that referenced this pull request Aug 14, 2026
…ed -- the work predated it in mudler#523, which the review correctly flagged; recorded as the process miss it was)

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: pi:kimi-k3 [pi]
VikashLoomba added a commit to VikashLoomba/vllm.cpp that referenced this pull request Aug 14, 2026
…udler#506 review rework

Review sweep findings (localai-bot, 2026-08-13), all accepted and verified
against the donor at pin 55596792:

1. OOB device write on odd output dim: the donor launches wvSplitK_hf_sml_
   only under (Kbp*N <= max_lds) && (M_in % YTILE == 0) (skinny_gemms.cu:1217)
   and the port dropped the %YTILE half -- the unguarded y=1 store lands past
   the output on odd N. Restored as N % 2 == 0 (our YTILE=2).
2. No arch guard: the port carries only the wave32 reduction arm
   (__shfl_xor(x,16)); the donor branches to ROW_BCAST15/31 on gfx9
   (skinny_gemms.cu:489-496) and double-guards dispatch
   (on_gfx9()/on_gfx1x() + compile-time ifdef). The gfx9 arm is NOT ported, so
   dispatch now refuses non-wave32 arches via CapabilityFromGcnArch(
   DeviceArchName()) instead of compiling and silently mis-reducing.
3. The m > 8 lower bound (utils.py:181, the feature-dim bound) restored as
   N > 8 -- at N==1 the first wave already wrote OOB.
4. The upstream test is now ported for real: the applicable
   NKM_FACTORS_WVSPLITK list (tokens 1-4 = our template arms), xavier on/off,
   and the ELEMENTWISE tolerance (atol = eps_bf16*sqrt(K), rtol = 1e-2 --
   torch assert_close semantics) replacing the aggregate NMSE that ~10 wrong
   elements would have passed. Added boundary shapes: features<=8 and odd
   features must route BLAS and stay correct; K%8!=0 declines; K%512!=0
   exercises the K-tail the old test never reached. Outputs write into a
   0xDEAD-sentinel guard band so any residual OOB store fails outright.
   Mutation-proven: with the N%2 guard removed the sentinel band is corrupted
   and the case fails; restored, green.

Deferred with reason (recorded in .agents/specs/rocm-skinny-gemm.md): fp16
(port is bf16-only), bias (the vt::MatmulBT seam has no bias operand), padded
strides (the dispatch precondition is contiguous rows), the fp8/rc variants.
The gfx9 wave64 arm is owed future work, guarded out loudly for now.

Allowlist: main's re-sorted file taken wholesale + VT_ROCM_SKINNY inserted
once in sorted position (per the review's merge note).

Gates (gfx1100, flock): test_backend_cross_device 20/20 incl. the ported
sweep; the 35B Q4_K_M decode e2e was re-measured for mudler#523's stack, unchanged
by this guard-only dispatch change (the skinny path fires identically at the
production shapes).

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: pi:kimi-k3 [pi]
…mbineGate) (mudler#41)

The next links in the generic MoE path after the router/silu-mul. Hand-
translated from cuda_moe.cu (MoeCombineKernel :473, MoeCombineGateKernel :555)
and the SharedExpertGate CPU oracle (cpu_ops.cpp:2387). Grid-stride, f32 math,
bf16/f32 dtype arms via boundary conversions; the combine-gate folds the
shared-expert sigmoid gate rounded through bf16 exactly as the donor.

Evidence (4x gfx1100, ROCm 7.14, Release):
- new MoE combine/gate cross-device case: 9/9 assertions (MoeCombineGate's
  oracle is the host-computed composite — no CPU op registration exists)
- ctest -R 'rocm|cross_device': 4/4
- full ctest: pre-existing failure set shrinks 7 -> 5; test_bench and
  test_capi now PASS (they failed at op 77 / the router dtype before the
  chain). test_loaded_engine_dense now fails only on the async-scheduling
  assertion (a lane capability gap, not a kernel throw).
- Named remaining blocker: the grouped quant expert GEMM
  (kMatmulBTQuantGrouped), the DeepSeek-V4 keep-quant family.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: pi:kimi-k3 [pi]
…ms -- the mudler#509 review rework

Review sweep findings (localai-bot, 2026-08-13), all accepted:

1. The donor's dtype refusals were dropped: cuda_moe.cu:520-524/:597-604 open
   with VT_CHECKs refusing non-f32/bf16; without them an f16 expert_out passes
   the seam's IsFloat gate and Tensor::Ptr<T>()'s unchecked cast reads 4 bytes
   per element from a 2-byte allocation. Refusals added to all three ROCm
   entry points (SharedExpertGate included: its 4-arm dispatch has the same
   f16 hazard on sd).
2. The case now exercises the PRODUCTION dtype mix, not only f32: the model
   path runs expert_out bf16 (qwen3_5.cpp DBuf ddown), shared bf16, out bf16.
   The new bf16 arm is asserted BIT-EXACT against the CPU reference (both
   sides thread-per-element, same sequential K order, single store rounding,
   -ffp-contract=off), and the f32 MoeCombine arm is tightened from NMSE to
   bit-exact per the donor's design comment (cuda_moe.cu:465-468). Writing the
   bf16 arm caught a construction bug in the first version of it (an f32-typed
   tensor over a bf16 buffer -- exactly the OOB class the review predicted the
   missing arm hid); fixed and re-verified against an independent host
   composite (0/320) plus a raw-hipMalloc scratch replica of both backends.
3. CMakeLists.txt: the mangled duplicate rocm_moe_chain.hip line removed.
4. docs/FEATURES.md op count 44 -> 47 (counted: the registration sites).

Gates (gfx1100, flock): test_backend_cross_device 20/20 (bit-exact bf16 +
f32 arms).

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: pi:kimi-k3 [pi]
…#684) plumbed through the ROCm arm

The rebase onto current main brought main's new MoeCombineFn signature
(routed_scale, default 1.0f, scaling the ROUTED sum before the shared term --
upstream apply_routed_scale_to_output). The ROCm kernel applies it in the same
f32 accumulator in the same order (one standalone multiply on the finished
sum, bit-identical to the CPU reference under -ffp-contract=off); the forward
declaration in rocm_ops.hip is updated to match. The bf16 test arm now runs
at scale 0.7 so the multiply is exercised, not just the 1.0 passthrough.

Gates (gfx1100, flock): test_backend_cross_device 20/20 (bit-exact arms
unchanged at 1.0, bit-exact at 0.7).

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: pi:kimi-k3 [pi]
VikashLoomba added a commit to VikashLoomba/vllm.cpp that referenced this pull request Aug 14, 2026
…ler#523 review rework

Review sweep findings (localai-bot, 2026-08-13), all accepted:

1. BLOCKER -- registering kMatmulBTQuant flipped keep-quant loader-wide via
   the boolean GgufQuantComputeAvailable() while the ROCm kernel implements 4
   of the 12 admitted formats, and there is no CPU fallback tier on a discrete
   card: a Q4_0/Q2_K/IQ2 model that worked before would keep blocks quantized
   and throw at first forward. FIX: RouteGgufTensor now consults
   DeviceKeepQuantSupported() -- ROCm keeps {Q8_0,Q4_K,Q5_K,Q6_K} quantized
   and everything else keeps its pre-existing expand_bf16 residency, so no
   load regresses and no forward throws. The same flip's keep_f16 collateral
   (MatmulBTKernelRocm accepts bf16/f32 only) is gated off on ROCm the same
   way. The device gate lives in the ROUTING decision, not KeepQuantDType --
   the dtype query also serves the device-independent residency tests.
2. Capture safety: the per-call hipMalloc/hipFree/hipStreamSynchronize on the
   activation-quant scratch is illegal under hipGraph stream capture (blocks
   mudler#473/mudler#332) and the free-after-launch leaked qact when Check() threw. Now a
   grow-only per-stream pool on hipMallocAsync, retire-never-free, mirroring
   the donor's EnsureScratch + graph_safe_scratch.h discipline; the syncs are
   gone with the frees.
3. Teeth: the non-grouped kMatmulBTQuant (the headline mechanism) gains its
   own cross-device case at the real shapes, and both new cases REQUIRE the
   ROCm registration instead of skipping silently when it is dropped. The
   loader-side routing gate gets a dedicated case in test_gguf_keep_quant
   (Q4_0/Q2_K expand, Q8_0/Q4_K/Q5_K/Q6_K keep, keep_f16 off on ROCm) plus a
   device-aware expectation in the TOTAL routing-table case.
4. The refusal messages name the owed formats (Q4_0/Q2_K/Q3_K/IQ2_*/IQ3_*/
   MXFP4) instead of double-listing Q5_K.
5. __dp4a is NOT used: ROCm 7.14's gfx1100 toolchain has no __dp4a and no
   dot1-insts target feature (RDNA3 dropped the int8 dot4 hardware) --
   verified by compile probe; the portable Dp4a helper stands, with the
   evidence recorded in .agents/specs/rocm-gg-keep-quant.md.

Gates (gfx1100, flock): test_gguf_keep_quant 38/38 (incl. the new routing
case); test_backend_cross_device 22/22 (376 assertions) incl. the new
non-grouped case; 35B Q4_K_M e2e still correct ('The capital of France is' ->
' Paris, a city that has been the').

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: pi:kimi-k3 [pi]
VikashLoomba added a commit to VikashLoomba/vllm.cpp that referenced this pull request Aug 14, 2026
…ed -- the work predated it in mudler#523, which the review correctly flagged; recorded as the process miss it was)

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: pi:kimi-k3 [pi]
VikashLoomba added a commit to VikashLoomba/vllm.cpp that referenced this pull request Aug 14, 2026
…udler#506 review rework

Review sweep findings (localai-bot, 2026-08-13), all accepted and verified
against the donor at pin 55596792:

1. OOB device write on odd output dim: the donor launches wvSplitK_hf_sml_
   only under (Kbp*N <= max_lds) && (M_in % YTILE == 0) (skinny_gemms.cu:1217)
   and the port dropped the %YTILE half -- the unguarded y=1 store lands past
   the output on odd N. Restored as N % 2 == 0 (our YTILE=2).
2. No arch guard: the port carries only the wave32 reduction arm
   (__shfl_xor(x,16)); the donor branches to ROW_BCAST15/31 on gfx9
   (skinny_gemms.cu:489-496) and double-guards dispatch
   (on_gfx9()/on_gfx1x() + compile-time ifdef). The gfx9 arm is NOT ported, so
   dispatch now refuses non-wave32 arches via CapabilityFromGcnArch(
   DeviceArchName()) instead of compiling and silently mis-reducing.
3. The m > 8 lower bound (utils.py:181, the feature-dim bound) restored as
   N > 8 -- at N==1 the first wave already wrote OOB.
4. The upstream test is now ported for real: the applicable
   NKM_FACTORS_WVSPLITK list (tokens 1-4 = our template arms), xavier on/off,
   and the ELEMENTWISE tolerance (atol = eps_bf16*sqrt(K), rtol = 1e-2 --
   torch assert_close semantics) replacing the aggregate NMSE that ~10 wrong
   elements would have passed. Added boundary shapes: features<=8 and odd
   features must route BLAS and stay correct; K%8!=0 declines; K%512!=0
   exercises the K-tail the old test never reached. Outputs write into a
   0xDEAD-sentinel guard band so any residual OOB store fails outright.
   Mutation-proven: with the N%2 guard removed the sentinel band is corrupted
   and the case fails; restored, green.

Deferred with reason (recorded in .agents/specs/rocm-skinny-gemm.md): fp16
(port is bf16-only), bias (the vt::MatmulBT seam has no bias operand), padded
strides (the dispatch precondition is contiguous rows), the fp8/rc variants.
The gfx9 wave64 arm is owed future work, guarded out loudly for now.

Allowlist: main's re-sorted file taken wholesale + VT_ROCM_SKINNY inserted
once in sorted position (per the review's merge note).

Gates (gfx1100, flock): test_backend_cross_device 20/20 incl. the ported
sweep; the 35B Q4_K_M decode e2e was re-measured for mudler#523's stack, unchanged
by this guard-only dispatch change (the skinny path fires identically at the
production shapes).

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: pi:kimi-k3 [pi]
localai-bot added a commit that referenced this pull request Aug 14, 2026
… validates an outside contributor's trailers (#773) (#782)

Closes #773.

`check-pr-size.py` and `check-commit-trailers.py` both required the base revision
to be an ANCESTOR of head. CI passes `github.event.pull_request.base.sha`, the
TIP of the base branch, which stops being an ancestor the moment main advances
after the branch was cut -- continuously, on this repo.

Measured on three open PRs before changing anything. Base is not an ancestor in
any of them, and a merge base exists in all three:

    #506  ancestor=NO  merge-base=e1087a881
    #523  ancestor=NO  merge-base=fdd452637
    #559  ancestor=NO  merge-base=fafa16f0f

The consequence was not a noisy red check. Both checkers aborted BEFORE
examining anything, so CI had never validated commit trailers on an external
contribution: the gate enforcing FOLLOWING_AGENTS_PROTOCOL and Assisted-by:
exited before reading a single commit. Across the external PRs reviewed this
week, hand-checking by a reviewer was the only verification those trailers
received. pr-size aborted identically, so path classification and the
checker-evidence contract went unenforced on forks too.

THE FIX. Diff from the merge base, which is what a pull request IS: `git diff
A...B` is defined as `git diff $(git merge-base A B) B` and is what GitHub
shows. Two-dot diffing against a moved main is not merely stricter, it is WRONG
-- main's own commits render as reversions inside the contributor's diff, so
paths they never touched get classified and charged to them. The new pr-size
test asserts both halves: the PR's file present, main's absent.
`executable_evidence` gets the same treatment, since the BASE version of a
checker for the red-before half is the one at the merge base.

WHAT DELIBERATELY DOES NOT MOVE. The old rule conflated two situations:
ordinary divergence (merge base exists) now examines merge_base..head;
unrelated histories (no merge base) STILL RAISES. Absence of information must
never look like absence of work -- the script's own require_origin_main()
docstring already states that principle for the other input.

test_missing_and_nonancestor_objects_fail_closed uses an ORPHAN branch, so it
still raises; only its regex changed, because the message now names what is
actually wrong. Its assertRaises(ValueError) is untouched. The trailers case was
SPLIT, not deleted: its divergent-branch half built two branches off a common
root -- which share a merge base and are the ordinary shape of every PR -- so
that half now asserts it validates, with the genuinely-unrelated case asserted
separately. Nothing that used to fail closed stopped failing closed.

Range changed, contract unchanged:
test_a_bad_trailer_in_the_merge_base_range_is_still_reported puts a trailerless
commit inside the new range and requires it still be reported.

RED before on the unmodified checkers, GREEN after (74 passed, 148 subtests).
Stop conditions checked individually rather than inferred. Full tests/scripts:
9 failed / 1359 passed, all nine pre-existing and reproduced on main.

CI: agent-record and pr-size both SUCCESS on this PR -- the two checkers it
repairs passing on a live PR. Remaining red is baseline only: windows-msvc-*
are the PR-only arm (#584), and sanitize-cpu is red on main itself for #775
(test_nemotron_h_scaffold, nemotron_h_registry.cpp:112 downcasting a doctest
StubModel to NemotronHLoadedModel). This PR touches no C++.
…ant, kMatmulBTQuantGrouped) with device-scoped loader gating — the MoE-path terminus (mudler#41)

Registers the keep-quant GEMM pair on ROCm (Q8_0/Q4_K/Q5_K/Q6_K superblock
dot-cores ported 1:1 from cuda_quant_dot.cu, portable Dp4a since gfx1100 has no
int8 dot4 hardware -- verified by toolchain probe), taking Qwen3.6-35B-A3B
Q4_K_M to a working e2e decode on one gfx1100 card.

Review-sweep rework folded in (localai-bot, 2026-08-13):
- Loader gating is per-format by the RUNNING device's kernel set
  (DeviceKeepQuantSupported in the routing decision): the boolean
  OpRegistered flip would have kept Q4_0/Q2_K/IQ2 blocks quantized on a card
  with no CPU fallback tier and thrown at first forward. Unsupported formats
  keep their pre-existing expand_bf16 residency; keep_f16 is likewise gated
  off on ROCm (MatmulBT accepts bf16/f32 only).
- The activation-quant scratch is a grow-only per-stream pool on
  hipMallocAsync (retire-never-free), mirroring the donor's EnsureScratch +
  graph_safe_scratch.h discipline -- the per-call hipMalloc/hipFree/
  hipStreamSynchronize was illegal under hipGraph stream capture (the mudler#473
  lane) and leaked on a throwing Check().
- Teeth: both ops have cross-device cases (the non-grouped arm had none), and
  both REQUIRE the ROCm registration instead of skipping; the loader-side
  routing gate has its own case in test_gguf_keep_quant plus a device-aware
  expectation in the TOTAL routing-table case. The REQUIRE teeth caught a
  real rebase-drop of the registrations during development.
- Refusal messages name the owed formats (Q4_0/Q2_K/Q3_K/IQ2_*/IQ3_*/MXFP4).

Specs: .agents/specs/rocm-grouped-quant-gemm.md (the spike) +
rocm-gg-keep-quant.md (the review rework).

Gates (gfx1100, flock): test_backend_cross_device 22/22,
test_gguf_keep_quant 38/38, 35B Q4_K_M e2e decode correct
('--max-num-seqs 1' on a single 24GB card).

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: pi:kimi-k3 [pi]
@bakon11

bakon11 commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Coordinating to avoid overlap: I'm scoping a (deferred) WMMA-fused FP8 expert GEMM — our rocm_fp8_channel_gemv.hip expert path currently runs scalar on gfx1201. Does #523 cover the FP8 E4M3 expert path, or only the GGUF keep-quant families (Q8_0/Q4_K/Q5_K/Q6_K)? Want to settle FP8-vs-keep-quant ownership so we don't duplicate the expert-GEMM work.

Also FYI for landing order: my #837 (GetBlas dual-slot TLS) edits rocm_matmul_hipblaslt.hip, which your #506 also touches (skinny-GEMM dispatch) — no semantic overlap with GetBlas itself, just flagging the shared file.

@bakon11

bakon11 commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

@VikashLoomba — also read #523 against our ROCm lane, and it's a clean split: it touches none of our files, and semantically it's a different quant family — GGUF Q8_0/Q4_K/Q5_K/Q6_K, activations to Q8_0/Q8_K, Dp4a integer-dot per output wave.

We're separately looking at an FP8 E4M3 + per-channel BF16 fused-dequant WMMA expert GEMM (Fireworks-style) for the Gemma-4 prefill path — a different quant ABI, loader route, and math core — so there's no overlap and #523 doesn't subsume it. We'll keep those as explicitly separate lanes rather than fold FP8 work into this GGUF PR.

Where we can help: gfx1201 (dual R9700, wave32) correctness/portability validation across all four formats, grouped + non-grouped, M/P = 1 and > 1. Wave32 matches and your Dp4a path is software-portable, so this is validation/perf characterization rather than joint kernel design. Ping us if you'd like RDNA4 numbers before landing.

localai-bot pushed a commit to bakon11/vllm.cpp that referenced this pull request Aug 17, 2026
Fold research 6195: ProductGetBlasStreamIsCapturing calls the exact
HipBlasHooks hook. HIP product probe begins capture and asserts true;
always-false hook mutation is RED. Host fake-capture case unchanged.

1a1153d6 is not a review target. Adjacent mudler#785/mudler#523/mudler#509/mudler#834 noted
in spec; no pickup.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: Hermes:grok-4.6 [Hermes]
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.

3 participants