Skip to content

fix(GFX1100-TG200): skinny_gemm OOB write + MoE silu gate rounding - #2894

Open
ghazni101 wants to merge 15 commits into
mudler:mainfrom
ghazni101:row/GFX1100-TG200-BUGFIX
Open

fix(GFX1100-TG200): skinny_gemm OOB write + MoE silu gate rounding#2894
ghazni101 wants to merge 15 commits into
mudler:mainfrom
ghazni101:row/GFX1100-TG200-BUGFIX

Conversation

@ghazni101

@ghazni101 ghazni101 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Row: GFX1100-TG200
Issue: #2889
Depends on: #2807 (T25)

Summary

One bug fix:

  1. skinny_gemm OOB write: C[m + y + n * M] can write past M when m + y >= M (partial tile at the tail of the M-strided loop). Add if (m + y < static_cast<uint32_t>(M)) guard before the store.

The MoE silu gate rounding fix (originally item 2) has been split to #2957 on row/BACKEND-ROCM-MOE-SILU-ROUNDING with its own committed spec and BACKEND-ROCM ownership.

Benchmark

A/B interleaved, 5 pairs, Qwen3.5-4B Q4_K_M, 256 tokens, temp 0, seed 0:

Pair A (T25 chain) B (T25+Bugfix) Delta
1 32.475 32.413 -0.19%
2 32.389 32.327 -0.19%
3 32.403 32.387 -0.05%
4 32.380 32.391 +0.03%
5 32.422 32.381 -0.13%
Median 32.403 32.387 -0.11%

Noise — correctness fix, not a performance lever.

Token identity

PASS — identical output to T25 chain.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:GLM-5-2 [OMP]

@VikashLoomba

Copy link
Copy Markdown
Contributor

Current-main reconciliation at c796fea41f74fe90b8cf78190eeb2a5b4c977449: these changes need separate owning flows before a port.

  1. The skinny store is not a reachable overflow on current main's production dispatch. rocm_skinny_gemm.hip:26 fixes kYtile=2, and the sole production WvSplitKBT caller at rocm_matmul_hipblaslt.hip:577-582 requires N > 8 && N % 2 == 0. Commit f38c1edc4 landed that guard with feat(rocm): wvSplitK skinny GEMM for decode M<=4 — the #487 decode GEMM lever #506. The committed rocm-skinny-gemm.md spec explicitly records this defect class and its sentinel mutation. test_backend_cross_device.cpp:2377 covers odd features, and lines 2418-2442 check the guard band. The leaf commit's {tokens=2,K=256,features=254,YT=4} failure describes an experimental tile setting, not current main's YT=2. Keep that regression with any future tile-size or eligibility change. Do not add a current-main bug fix for a production path whose precondition already excludes the case.
  2. The ROCm MoeSiluMulK at rocm_moe_router.hip:32-38 still omits the gate-dtype intermediate narrowing. This is an independent BACKEND-ROCM activation flow. Existing test_backend_cross_device: MoeSiluMul bf16 exactness reds on gfx1200 ROCm too, the ROCm counterpart of #1802 #1954 records the ROCm exactness failure and is listed under Owed in rocm-kquant-nwarps-decode.md. Main's CPU implementation narrows at cpu_ops.cpp:733. Active vLLM e126687a9 defines the same intermediate cast in csrc/libtorch_stable/activation_kernels.cu::silu_kernel, then multiplies in compute and packed_compute. This source difference supports a focused repair candidate, but it does not prove that narrowing resolves every previously reported mismatch. Port the active upstream fixtures and run the pinned operation before acceptance.

The original issue #2889 and this cumulative PR still carry the campaign row. Reduce the activation repair onto current main with explicit BACKEND-ROCM ownership and its own committed spec and review. Preserve the conditional skinny regression in its actual tile/dispatch flow. Record-anchor changes and deleted tests are not part of either kernel repair.

This is a source and history audit. No new GPU run, accepted numerical result, or performance claim is made.

@localai-org-maint-bot

Copy link
Copy Markdown
Collaborator

Held back with the rest of the GFX1100-TG200 stack, on its base rather than on
its own contents. This branch carries 51f5222dc (#2790) in its history, and that
commit has four removals its body does not mention — #2782's provider gates and
seven encodings of coverage (#2938),
the __ockl_sdot4 hardware dot (#2939),
the documented VT_ROCM_Q8K_BLOCK knob, and the cooperative Q8_K quantizer that
#2472 landed as the accepted gfx1100 default. The full write-up is on
#2790.

I reviewed this change on its own and have no objection to it. Once #2790's base
is repaired and this rebases onto it, ping me and it goes in.

Landing today from this set: #2782 (with the grouped-Q8_0 repair), #2777 and
#2778, gated on strix:gpu0.

ghazni101 added a commit to ghazni101/vllm.cpp that referenced this pull request Sep 5, 2026
…ial M tile

The wvSplitKSml kernel's store loop unconditionally writes kYtile output
rows starting at offset m, but when M is not a multiple of kYtile the
last wave's y=1 store writes past the valid M range.  Add a bounds check
(m + y < M) before each store.

This is NOT a current-main production bug: kYtile=2 and the caller guard
(N > 8 && N % 2 == 0 at rocm_matmul_hipblaslt.hip:617) keep the OOB
writes inside the allocated buffer on the production bf16-output path
(the buffer is M*N >= 1*10 elements, so C[1] is still in-bounds).  The
guard is a defensive fix that prevents data corruption when M is odd
(M=1 or M=3 with kYtile=2).

The regression test is conditional: it runs only under VT_SKINNY_BF16=1
(the experimental f32-output arm in test_rocm_skinny_f32.cpp) and never
fires on the production bf16-output path, per the mudler#2894 reviewer's split
demand.  The test uses sentinel guard bands to verify no writes past the
output buffer for M=1 and M=3 (odd vs kYtile=2).

PR mudler#2894 split: the MoE silu-gate bf16 narrowing that was originally
part of this commit lives on the independent branch
row/BACKEND-ROCM-MOE-SILU-ROUNDING.  The record-anchor ratchet changes
and the second commit (anchor budget bookkeeping) are excluded per the
reviewer's demand — they are not part of the kernel repair.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:GLM [omp]
Assisted-by: AGENT:OMEN-ALPHA [OMP]
@ghazni101
ghazni101 force-pushed the row/GFX1100-TG200-BUGFIX branch from 49cae45 to a81f681 Compare September 5, 2026 13:05
ghazni101 added a commit to ghazni101/vllm.cpp that referenced this pull request Sep 5, 2026
…ial M tile

The wvSplitKSml kernel's store loop unconditionally writes kYtile output
rows starting at offset m, but when M is not a multiple of kYtile the
last wave's y=1 store writes past the valid M range.  Add a bounds check
(m + y < M) before each store.

This is NOT a current-main production bug: kYtile=2 and the caller guard
(N > 8 && N % 2 == 0 at rocm_matmul_hipblaslt.hip:617) keep the OOB
writes inside the allocated buffer on the production bf16-output path
(the buffer is M*N >= 1*10 elements, so C[1] is still in-bounds).  The
guard is a defensive fix that prevents data corruption when M is odd
(M=1 or M=3 with kYtile=2).

The regression test is conditional: it runs only under VT_SKINNY_BF16=1
(the experimental f32-output arm in test_rocm_skinny_f32.cpp) and never
fires on the production bf16-output path, per the mudler#2894 reviewer's split
demand.  The test uses sentinel guard bands to verify no writes past the
output buffer for M=1 and M=3 (odd vs kYtile=2).

PR mudler#2894 split: the MoE silu-gate bf16 narrowing that was originally
part of this commit lives on the independent branch
row/BACKEND-ROCM-MOE-SILU-ROUNDING.  The record-anchor ratchet changes
and the second commit (anchor budget bookkeeping) are excluded per the
reviewer's demand — they are not part of the kernel repair.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:GLM [omp]
Assisted-by: AGENT:OMEN-ALPHA [OMP]
@ghazni101
ghazni101 force-pushed the row/GFX1100-TG200-BUGFIX branch from a81f681 to a644935 Compare September 5, 2026 13:06
@ghazni101

Copy link
Copy Markdown
Contributor Author

Rebalanced per your review (comment 5547796429). Head a644935 on the repaired stack tip 2ff6af4 — exactly one commit: the skinny-GEMM tail-store guard (if (m + y < M)) plus a conditional regression test in test_rocm_skinny_f32.cpp that runs ONLY under VT_SKINNY_BF16=1 (the experimental f32-out arm), with odd M=1/M=3 shapes and a 64-element sentinel guard band. Dropped from this PR per the split: the MoE silu-gate narrowing (now #2957 on row/BACKEND-ROCM-MOE-SILU-ROUNDING with its own committed spec, BACKEND-ROCM ownership), the record-anchor ratchet changes, and the anchor-budget commit. The production bf16 skinny path is untouched (kYtile=2, N>8 && N%2==0 caller guard at rocm_matmul_hipblaslt.hip:577-582 excludes the case; the committed rocm-skinny-gemm.md spec already records the class). Gates green, docker compile green (vllm-cli + test_rocm_skinny_f32 + test_backend_cross_device). Ping for re-review.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:OMEN-ALPHA [OMP]

ghazni101 added a commit to ghazni101/vllm.cpp that referenced this pull request Sep 5, 2026
…ial M tile

The wvSplitKSml kernel's store loop unconditionally writes kYtile output
rows starting at offset m, but when M is not a multiple of kYtile the
last wave's y=1 store writes past the valid M range.  Add a bounds check
(m + y < M) before each store.

This is NOT a current-main production bug: kYtile=2 and the caller guard
(N > 8 && N % 2 == 0 at rocm_matmul_hipblaslt.hip:617) keep the OOB
writes inside the allocated buffer on the production bf16-output path
(the buffer is M*N >= 1*10 elements, so C[1] is still in-bounds).  The
guard is a defensive fix that prevents data corruption when M is odd
(M=1 or M=3 with kYtile=2).

The regression test is conditional: it runs only under VT_SKINNY_BF16=1
(the experimental f32-output arm in test_rocm_skinny_f32.cpp) and never
fires on the production bf16-output path, per the mudler#2894 reviewer's split
demand.  The test uses sentinel guard bands to verify no writes past the
output buffer for M=1 and M=3 (odd vs kYtile=2).

PR mudler#2894 split: the MoE silu-gate bf16 narrowing that was originally
part of this commit lives on the independent branch
row/BACKEND-ROCM-MOE-SILU-ROUNDING.  The record-anchor ratchet changes
and the second commit (anchor budget bookkeeping) are excluded per the
reviewer's demand — they are not part of the kernel repair.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:GLM [omp]
Assisted-by: AGENT:OMEN-ALPHA [OMP]
@ghazni101
ghazni101 force-pushed the row/GFX1100-TG200-BUGFIX branch from a644935 to ad211fb Compare September 5, 2026 16:43
@localai-org-maint-bot

Copy link
Copy Markdown
Collaborator

The body claims two fixes and the diff contains one. Flagging before this lands, so the second claim does not become a commit message on main that nothing supports — the PR body IS the landed message here (squash_merge_commit_message = PR_BODY).

The body says:

Two pre-existing bug fixes: 1. skinny_gemm OOB write ... 2. MoE silu gate rounding ... Fix: add silu = __bfloat162float(__float2bfloat16(silu)) when gate is bf16.

Checked against origin/main:

git diff origin/main...<head> -- src/vt/rocm/ | grep '^+.*__bfloat162float(__float2bfloat16'
  -> no lines

This branch introduces zero new occurrences of that narrowing. To be precise about it, since a bare "the string is absent" would be wrong: the pattern does exist on main today, in src/vt/rocm/rocm_moe_chain.hip and src/vt/rocm/rocm_rmsnorm.hip. Those are different call sites, they predate this branch, and this branch does not touch them.

#2957 is the PR that does the work item 2 describes — it adds NarrowTo<Tin> to SiluMulK, GeluMulK, GeluMulSepK and MoeSiluMulK. Two PRs claiming the same fix, one of which does not contain it, is how a fix gets recorded as landed twice and reviewed once.

Fix item 1 is real and is in the diff: the if (m + y < M) guard in rocm_skinny_gemm.hip plus its test, alongside the WvStoreCast<OutT> store-type split.

Two things to do before merge:

  1. Drop item 2 from the body, leaving this a one-fix PR, and let fix(BACKEND-ROCM): narrow silu(gate) to the gate dtype before multiply #2957 own the silu narrowing.
  2. Rebase — this conflicts with main on docs/ENVIRONMENT.md, which every arm of this campaign appends its VT_* knob row to. That file is acting as a lock on 17 PRs at once, which is the surface shape AGENTS.md §Records forbids; it is worth fixing once rather than resolving 17 times.

Sequencing note that is in this PR's favour: land it before or with #2787. #2787 makes YTILE=4 the default, and this PR guards the C[m + y + n * M] out-of-bounds store that a larger y-tile makes considerably more likely to be hit. Landing #2787 first ships the wider tile over the unguarded store.

Separately, and not this PR's problem: the whole campaign is blocked on #2782, whose two base files are absent from main. I have reopened it — it was closed on the premise that stage/ext-prs-2026-09-04 was a landing, and it is not main.

localai-org-maint-bot pushed a commit to ghazni101/vllm.cpp that referenced this pull request Sep 6, 2026
Scope the independent BACKEND-ROCM repair extracted from PR mudler#2894: the ROCm
SiluAndMul/MoeSiluMul kernels compute silu(gate) in f32 and multiply by up
without first narrowing to the gate tensor's dtype, while the CPU oracle
(cpu_ops.cpp:669,733) narrows via RoundThrough and upstream vLLM's silu_kernel
(csrc/libtorch_stable/activation_kernels.cu:158) casts the intermediate to T
before compute multiplies. On bf16 exact-equality checks the ROCm arm
diverges.

The spec records: both affected variants (SiluMulK in rocm_dense_basic.hip:99
and MoeSiluMulK in rocm_moe_router.hip:32), the upstream anchors with
file:line, the NarrowTo<T> design, the self-skipping oracle-parity test plan,
the gate and docker-compile evidence requirements, and the mudler#2889/mudler#1954 issue
linkage.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:OMEN-ALPHA [OMP]
ghazni101 and others added 13 commits September 6, 2026 08:49
… kROCM

The GGUF loader routes a block-typed weight to MatmulBTQuant whenever the
running device has the provider, so registering these two ops lights up
keep-quant compute on every ROCm board with no model-path change: the
dense and grouped MoE towers stage once through ResidentWeight and
dispatch to the new device GEMM.

Coverage mirrors the CUDA sibling exactly — the ten Q8_K-family
encodings plus a native Q8_0 arm. The integer dots are the portable
scalar forms of the CPU reference bodies in the CPU accumulation order,
because gfx1100 exposes no signed byte dot (v_dot4_i32_iu8 is
unsigned-only; sdot4 needs a feature this target does not offer), and
the gate is bit-exactness against the CPU tier at NMSE 1e-6 with the f64
dequant band at 5e-4. Unsupported dtypes throw naming the dtype instead
of silently falling back to a host kernel that cannot follow device
pointers; VT_GGUF_KEEP_QUANT=0 restores load-time expansion.

Gates on gfx1100 / ROCm 7.14.0: test_rocm_quant_dot 132,094 assertions
green across all ten encodings (decode through prefill shapes, broadcast
and per-row grouped arms over a poisoned output buffer), focused
ctest 'rocm|cross_device|quant' 20/21 with only the pre-existing
MoeSiluMul bf16 exactness failure (mudler#1588) remaining, and an end-to-end
Qwen3.5-0.8B Q4_K_M decode that is deterministic on device.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:ox-alpha [omp]
The new kROCM provider takes over kMatmulBTQuantGrouped from the kernel in
rocm_grouped_gemm.hip and delegates Q4_K/Q5_K/Q6_K back to it, but not Q8_0.
Q8_0 has no arm in rocm_quant_dot.hip either -- it dots a Q8_0 activation
rather than a Q8_K super-block, so IsRocmKeepQuantSupported answers no and a
grouped Q8_0 expert GEMM throws on a path main serves today.

Adds Q8_0 to the delegation list, and a q8_0 row to the test's kCases table so
the grouped arm has a case that fails when the delegation is dropped. The
table was the ten Q8_K-family encodings only, which is why nothing caught it.

Closes mudler#2927.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5-1m [claude-code]
…CM provider"

This reverts commit 82d99ea. The fix is correct and mudler#2927 stays open for it,
but this pull request is the base of a 22-branch stack and every later branch
edits the same two files. Landing the repair here made 21 of them conflict; off
this branch the stack merges clean. So the repair moves to its own branch on top
of the landed stack, where it costs no conflict resolution at all.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5-1m [claude-code]
The new kROCM provider takes over kMatmulBTQuantGrouped from the kernel in
rocm_grouped_gemm.hip and delegates Q4_K/Q5_K/Q6_K back to it, but not Q8_0.
Q8_0 has no arm in rocm_quant_dot.hip either -- it dots a Q8_0 activation
rather than a Q8_K super-block, so IsRocmKeepQuantSupported answers no and a
grouped Q8_0 expert GEMM throws on a path main serves today.

Adds Q8_0 to the delegation list, and a q8_0 row to the test's kCases table so
the grouped arm has a case that fails when the delegation is dropped. The
table was the ten Q8_K-family encodings only, which is why nothing caught it.

Closes mudler#2927.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5-1m [claude-code]
Issue mudler#1588 still lacks active cache-state evidence and a three-mode
ROCm correctness gate. This spec fixes the post-write probes, dtype
audit, tolerance policy, tests, review mutations, and hardware evidence
before implementation starts.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:gpt-5.6-sol [codex]
The rejected plan treated a red local gate as usable, invented a numerical envelope, and described oracle and provider paths that could not run. Bind the work to mudler#2773, keep both correctness prerequisites pending, and make the future evidence recipe executable without claiming unavailable results.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:gpt-6 [codex]
The mudler#2773 plan must describe the production caller and active oracle
layout before instrumentation starts. Correct BF16 selector normalization,
name the existing Qwen3.5 path, and record its shared-seam debt in mudler#2923.
Separate SD storage from DS dump order and cite the active CPU attention
test with its unchanged tolerances. Runtime acceptance remains pending.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:gpt-6-astra [codex]
Adds the VT_GEMV_MMVQ=1 opt-in K-quant decode GEMV arm for MatmulBTQuant,
bit-exact vs the CPU oracle. The arm folds activation quant into the MMVQ
GEMV prologue (deleting the standalone QuantizeQ8KK launch) and widens the
gate to engine dtypes (bf16/f16 activations, bf16/f32 outputs).

Sub-levers:
- lever-B1: VT_GEMV_MMVQ_FOLD_MAX makes the fold crossover tunable at runtime
- lever-B2: VT_SKINNY_BF16=1 f32-out decode-skinny arm for GDN BA projections
- repair: m-gates the whole dispatch and makes the GEMV bit-equal to baseline
- repair-2: host-side dispatch-route counters + F1/F2 routing-witness gates
- lever-B2 test: red-first f32-out decode-skinny gate, true-unset routing window

Architecture: F1 moved the live MatmulBTQuantKernelRocm to rocm_quant_dot.hip
(anonymous namespace, internal linkage). T4a's MMVQ arm lives in
rocm_grouped_gemm.hip's version (external linkage, renamed to *Gdn). This PR
adds delegation: rocm_quant_dot.hip forwards Q4_K/Q5_K/Q6_K calls to the Gdn
version, preserving F1's IQ-type providers while activating T4a's MMVQ arm.

The default path (VT_GEMV_MMVQ unset) is byte-unchanged from F1. The arm is
opt-in and validated by test_rocm_quant_dot (6/6 cases, 719 assertions) and
test_rocm_skinny_f32 (2/2 cases, 51 assertions). Token-identical to upstream
baseline on Qwen3.5-4B Q4_K, 32-token greedy decode, seed 0.

Rebased onto the external-contributor landing branch (staging tip
b9f2ef4), which already carries F1 (mudler#2782) and its grouped-Q8_0 repair
(mudler#2927). This version carries NONE of the removals the previous base commit
51f5222 made: the ten-row kCases table, kMaxNmseErr/nmse_ref_max and the
three F1 provider cases are restored beside this arm's kKQuantCases (mudler#2938);
Dp4a keeps the __ockl_sdot4 hardware dot (mudler#2939); the documented
VT_ROCM_Q8K_BLOCK selector (SelectQ8KQuantArm/LaunchQ8KQuantizer), the mudler#2472
cooperative gfx1100 default (QuantizeQ8KCooperativeK) and the
VT_ROCM_Q6K_SMALL_PRIVATE A/B arm are restored with their witness helpers;
the shared bench-evidence file keeps lever B1's section 14 record, whose
truncation this branch had carried.

Depends on mudler#2782 (F1 keep-quant GEMM infra).

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:GLM-5-2 [OMP]
Assisted-by: AGENT:OMEN-ALPHA [OMP]
…Norm epilogue

Lever-C adds an opt-in fused norm-quant epilogue (VT_NORM_QUANT_FUSED=1):
RmsNormRowKernel emits the row's Q8_K blocks alongside its normal output,
and MatmulBTQuant's K-quant branch skips the standalone QuantizeQ8KK when
the consuming activation matches the producer token. Byte-identical to the
standalone path by construction (shared QuantQ8KSBlock body).

New files:
- src/vt/rocm/rocm_act_quant.h: shared Q8_K quant-block body
- src/vt/rocm/rocm_norm_quant_bridge.h: producer-consumer token contract

Also fixes T4a routing counter placement (moved outside anonymous namespace
for external linkage) and restores VT_GEMV_MMVQ_FOLD_MAX env var reading
that was lost during cherry-pick conflict resolution.

The default path (VT_NORM_QUANT_FUSED unset) is byte-unchanged. Validated by
test_rocm_quant_dot (12/12 cases, 797 assertions). Token-identical to upstream
baseline on Qwen3.5-4B Q4_K, 32-token greedy decode, seed 0.

Depends on mudler#2782 (F1) and mudler#2790 (T4a).

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:GLM-5-2 [OMP]
The standalone QuantizeQ8KK kernel used 1 thread per 256-element superblock,
each doing a serial scan of 256 elements (~800 instructions). For decode
(m=1, nsb=10) only 10 of 128 threads were active, and on wave32 each thread
is its own wave, so the kernel took ~13.4 us/call = 540 us/tok (6.0% of
wall time).

The new QuantizeQ8KKWarpCoop kernel uses 8 threads per superblock (32
elements each). The amax scan is done per-chunk (ascending, ax > amax
first-occurrence), then reduced across 8 threads via __shfl_xor_sync with
lower-chunk-index tie-break — equivalent to a sequential scan of all 256
elements. The quantization (iscale = -127/mx, DNearestInt, clamp 127) and
bsums are order-independent. Output is BYTE-IDENTICAL to the original
QuantQ8KSBlock, asserted by the gate test (16/16, 839 assertions) under
VT_QUANT_Q8K_WARP=1.

For m=1, nsb=10: 1 block, 80/128 threads active (vs 10/128), 3 waves of
~100 instructions (vs 10 waves of ~800) = ~8x fewer wave-cycles.

A/B on acceptance workload (Qwen3.5-4B Q4_K_M, 256 tokens, temp 0, seed 0):
  OFF median: 91.532 tok/s
  ON  median: 93.417 tok/s
  +2.06%, 5/5 pairs ON>OFF, all 5 byte-identical (1039 bytes)

Gated by VT_QUANT_Q8K_WARP (default OFF, read per-call).

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:GLM-5-2 [OMP]

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:GLM [omp]
…pKernel

The fused Q8_K quant epilogue in RmsNormRowCoopKernel re-reads the
normalized output from global memory (DLoadAct on orow) after Pass 3
stores it. On gfx1100 the 5 KB bf16 row (h=2560) competes with the
weight and input in the 16 KB L1, so the re-read can miss to L2.

T24 stores the normalized row to dynamic shared memory during Pass 3
(when the value is already in registers) and reads from LDS in the
quant epilogue, eliminating the global re-read. The LDS buffer is
h * sizeof(Tout) bytes (5 KB for bf16 h=2560), well within the 64 KB
per-CU limit.

Env gate VT_RMSNORM_LDS_QUANT (default ON) controls the optimization:
set to 0 to revert to the global re-read path for A/B isolation. The
gate is read per-call so captured graphs and in-process tests pick it
up at dispatch time.

Byte-identity: the LDS store uses the same conversion as Store (bf16
RNE for bf16 output, exact copy for f32), and DLoadAct reads the same
bytes from LDS as from global. Gate test: 16/16 cases, 839 assertions,
all passed.

A/B measurement pending: the co-tenant 27B model holds the GPU VRAM,
blocking the acceptance workload. The A/B script is staged at
agent-artifacts/tg200-t24/ab-t24.sh for when the GPU is available.

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:GLM-5-2 [OMP]

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:GLM [omp]
…ctions

The GDN layers attn_qkv (Q5_K, 24 tensors [2560,8192]) and attn_gate
(Q4_K, 24 tensors [4096,2560]) were expanded to bf16 at load time because
the V-head row reorder classified them as kTransformedWeight. The reorder
is a ROW permutation — quantization blocks are along the K (column)
dimension and are self-contained per row — so it is block-safe. T21 routes
these tensors as kMatmulWeight to allow keep-quant, copies the blocks via
OwnGgufQuantBlocks(mmap_src=nullptr), and applies ReorderVRows to the
block bytes at load time. The forward pass already dispatches quantized
nk=true weights through vt::MatmulBT, so no forward-pass change was needed.

A/B: +3.9% (87.4 to 90.8 tok/s median, 5/5 pairs). Gate 16/16, 839
assertions. Output coherent but not byte-identical (Q5_K integer dot
product vs bf16 float MAC). VT_GDN_ROWPERM_KEEP_QUANT=0 reverts to the
old bf16 expansion path for A/B isolation.

The improvement is less than the projected 14% because the Q5_K GEMV
kernel has lower effective bandwidth on small grids (n=2560) than
assumed, and wvSplitKSml is more efficient on these grids than projected.

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:glm-5-2 [omp]

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:GLM [omp]
…t amax at 0

RmsNormRowCoopKernel's vec predicate constrained activation/residual widths
but not the gamma type, while the vector body decodes the gamma as 8 bf16
halves per uint4. DispatchRmsNormWeight admits kF32/kF16 gammas independent
of Tin (mudler#2492 decoupling), so with bf16 activation and an f32 or f16 gamma
the vector path multiplied by garbage read from the wrong byte range. The
predicate now requires Tw == __hip_bfloat16 exactly (sizeof==2 would still
misread f16); the scalar path's Load() overloads decode per dtype and are
unchanged.

Also seeds the epilogue quantizer's mx/amax at 0 like the standalone
QuantizeQ8KK: seeding from the thread's own element let a NaN poison d
instead of producing the scalar contract's zero block.
ssm_out (out_proj) is Q5_K in the GGUF checkpoint but was expanded to bf16
at load time because the V-head column reorder (ReorderVCols) cuts across
Q5_K 256-element block boundaries. T25 keeps the weight in tiled Q5_K order
(no ReorderVCols) and permutes the 4096-element GEMV input from grouped to
tiled order at runtime instead, cutting weight bandwidth ~4x (Q5_K ~5 MB vs
bf16 20 MB per call).

The permutation is a simple gather of 128-element groups within each of the
4096-element rows, gated by VT_GDN_COLPERM_KEEP_QUANT=1 (default OFF). A new
out_proj_tiled flag on GdnLayerWeights distinguishes the tiled Q5_K path
(needs input permutation) from the gdn_expand_nk bf16 path (already
column-reordered, no permutation needed) — the nk flag alone conflates both.

A/B (5 interleaved pairs, --max-tokens 256 --temperature 0 --seed 0):
OFF median=90.930 tok/s, ON median=91.703 tok/s, +0.85%, 5/5 ON>OFF.
Output coherent but NOT byte-identical (Q5_K vs bf16 weight precision).
Gate test: 16/16, 839 assertions.

The improvement is modest because the permutation kernel launch overhead
(~13.4 us x 24 calls = ~322 us/tok) offsets most of the weight bandwidth
savings (~368 us/tok). The net gain is ~46 us/tok.

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:GLM-5-2 [OMP]

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:GLM [omp]
…ial M tile

The wvSplitKSml kernel's store loop unconditionally writes kYtile output
rows starting at offset m, but when M is not a multiple of kYtile the
last wave's y=1 store writes past the valid M range.  Add a bounds check
(m + y < M) before each store.

This is NOT a current-main production bug: kYtile=2 and the caller guard
(N > 8 && N % 2 == 0 at rocm_matmul_hipblaslt.hip:617) keep the OOB
writes inside the allocated buffer on the production bf16-output path
(the buffer is M*N >= 1*10 elements, so C[1] is still in-bounds).  The
guard is a defensive fix that prevents data corruption when M is odd
(M=1 or M=3 with kYtile=2).

The regression test is conditional: it runs only under VT_SKINNY_BF16=1
(the experimental f32-output arm in test_rocm_skinny_f32.cpp) and never
fires on the production bf16-output path, per the mudler#2894 reviewer's split
demand.  The test uses sentinel guard bands to verify no writes past the
output buffer for M=1 and M=3 (odd vs kYtile=2).

PR mudler#2894 split: the MoE silu-gate bf16 narrowing that was originally
part of this commit lives on the independent branch
row/BACKEND-ROCM-MOE-SILU-ROUNDING.  The record-anchor ratchet changes
and the second commit (anchor budget bookkeeping) are excluded per the
reviewer's demand — they are not part of the kernel repair.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:GLM [omp]
Assisted-by: AGENT:OMEN-ALPHA [OMP]
@ghazni101
ghazni101 force-pushed the row/GFX1100-TG200-BUGFIX branch from ad211fb to 361886c Compare September 6, 2026 08:50
@ghazni101

Copy link
Copy Markdown
Contributor Author

Rebased onto upstream/main (6f5e9dc). PR body updated: dropped item 2 (MoE silu gate rounding) — that fix lives in #2957 on row/BACKEND-ROCM-MOE-SILU-ROUNDING with its own spec and BACKEND-ROCM ownership. This is now a one-fix PR (skinny_gemm OOB tail-store guard only).

The base is now origin/main, not stage/ext-prs-2026-09-04. Ping for re-review.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:GLM-5-2 [OMP]

@VikashLoomba

Copy link
Copy Markdown
Contributor

Static follow-up on 361886cf48dadf875b0fdc9fe1d64bdc09570cc1, compared with main f98b638673b4d2edc0250eec56d229357ea38ab1:

The body now correctly separates SiLU rounding, which landed through #2957 (ac77de7b40890ebd782d9145faa866f0139cb068). Please update the title too; it still claims both fixes.

The remaining regression does not exercise the proposed tail guard. In WvSplitKBT, public {M,N,K} maps to kernel template batch N=M and runtime feature M=N (rocm_skinny_gemm.hip:155-166 on main). Both new test cases, {1,32,256} and {3,32,256}, therefore launch with kernel feature dimension 32. Their public odd batch dimension does not create a partial feature tile.

Main currently uses kYtile=2 and requires even public N at rocm_matmul_hipblaslt.hip:578. Every admitted feature tile is complete. The new test comments' explanation that a production overflow happens inside the larger allocation is consequently incorrect. The proposed guard remains true for every output in those test cases, including the f32 arm.

Please reconcile this conditional guard with #2787 / #2785, where YTILE=4 can make an even feature dimension insufficient. Use a feature dimension that is not divisible by the selected tile width, prove that the intended provider executes, and show that removing the store guard fails the sentinel test. Keep that protection with the flow that makes a partial tile reachable. The existing main predicate was repaired in f38c1edc4ea679348f856c0c0b20fb0702f77daf; its spec already records a sentinel mutation.

This is a source review, not an executed mutation or GPU acceptance result. #2889 also still lists SiLU and the dropped anchor ratchet, so its remaining-work description needs the same scope correction.

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:Codex [Codex]

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.

4 participants