Repository navigation
hrx: admit element-wise dense MUL_MAT at input_size % 32, not only % 256 (engine #991) - #199
Conversation
|
Scope note for the review, so nothing here is expected to grow: I tried the follow-up class (relaxing the decode path, where What that leaves for this PR's claim: prefill element-wise contractions at |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at ad8aed5d
The change is a policy predicate, so I checked the predicate itself and the kernel's own contract, and re-derived both here.
The predicate does what the body says, and nothing else. Compiled against this head's dispatch-mul-mat-weight-format.h (+23/−5, three files):
F32 K=3712 old=0 new=1 <- Zamba-7B's n_embd
F32 K=1472 old=0 new=1 <- BlackMamba-2.8B's n_embd
F32 K=2560 old=1 new=1 <- Zamba2, unchanged
F32 K=288 old=0 new=1
F32 K=32 old=0 new=0 <- below the 256 floor, unchanged
F32 K=255 old=0 new=0
F32 K=32768 old=1 new=1 ; K=32800 old=0 new=0
Q4K K=3712 old=0 new=0 <- block formats NOT relaxed (the safety property)
Q6K K=3712 old=0 new=0
Q8_0 K=3712 old=1 new=1 <- Q8_0 already had the %32 arm
The range stays >= 256 && <= 32768, i.e. identical to the arm it replaces, so the only thing that moved is the granularity, and only for common_mul_mat_dense_float_format() (F16/BF16/F32 — value-by-value formats, no block layout to honour).
And the kernel really does tolerate a K tail, which is the whole premise. At this head:
kernels/loom-libs/ops/mul_mat_f32_f32_wmma.loom:44 declares range(%value, 256, 32768), multiple_of(%value, 32); the core rounds up (%padded_input_size = input_size + 255, %quant_block_count = padded / 256) and masks both operands (%valid_weight = valid_channel && valid_k at :118, %valid_input = input_k < bounded_input_size and vector.load.mask at :133-138). The decode kernel takes input_size as a runtime argument and pads the same way (mul_mat_f32_f32_decode.loom:79-80).
I also checked the blast radius you claim is untouched: the fused motifs that stay on 256 are still reached without the flag, and I confirmed the corpus agrees with your split — mul_mat_add_f32_f32_wmma.loom and mul_mat_id_f16_f16_wmma.loom declare multiple_of 256, mul_mat_swiglu_f32_f32_wmma.loom declares 32 (it is not in your "needs 256" list, correctly).
What I did not re-run: the device matrix. test-backend-ops -o MUL_MAT -b HRX0 404/404 and MUL_MAT_ID 852/852 are yours; I have no HRX build at this head and the box is carrying other lanes' builds tonight (load 21-27), so I did not add one to run two shapes. The predicate + the masks above are what makes those numbers plausible; if you get a quiet window, the two shapes worth pinning in a regression are exactly K=3712 and K=1472 with an element-wise weight format, since those are the only newly admitted ones.
One nit: the comment says "padded_input_size = input_size + 255" while the motif also derives quant_block_count from it; worth naming that the weight decode uses the block count so a K tail can never read past the last block — that is the argument a future reader will look for.
ad8aed5 to
38ae12a
Compare
…256 (engine ggml-org#991) The dense MUL_MAT admission required input_size % 256 == 0, and F16/BF16/F32 weights fall into the block-quantised arm of that predicate, so a model whose hidden size is not a multiple of 256 lost every dense matmul to the CPU while the rest of it stayed on the device (Zamba-7B-v1 n_embd 3712: 383 of 5113 nodes; BlackMamba-2.8B 1472: 145 of 1179). The refusal was policy, not the kernel's contract: the wmma kernel declares multiple_of(input_size, 32) and its core already rounds the contraction up to whole 256-blocks and masks both operands past input_size, so a K tail contributes zeros. Only element-wise formats can be unaligned at all - block formats are laid out in 256-value blocks, and a quantised tensor with ne[0] % 256 != 0 cannot exist in ggml - so the relaxation is one predicate about the element-wise formats. The relaxation is an explicit flag on the shared matcher, passed only by the three callers that dispatch the plain %32 kernel (plain, fused-unary and decode). The fused motifs that genuinely need 256 - mul_mat_postops, mul_mat_id_f16_f16, llm.attention_qkv, q6_K packed/shortlist, symmetric i8 - keep the old predicate: had the shared one been relaxed, a fused match would have compiled K=3712 against a kernel whose declared constraint forbids it. With the flag a fused match simply does not engage and falls through to the plain matmul, so the node stays on the device and only the fusion is lost. gfx1151: Zamba-7B-v1 383 -> 254 of 5113 nodes off (MUL_MAT 281 -> 165, UNARY 13 -> 0, CONT 89 unchanged), splits 384 -> 179; Zamba2-2.7B (2560) unchanged at 0 off; test-backend-ops -o MUL_MAT 404/404 and -o MUL_MAT_ID 852/852 on HRX0 vs CPU; greedy output byte-identical to the pre-change run. Not closed: the remaining nodes are the K % 32 != 0 class (ne=[264,512,2,1] etc.) and the %256 fused/MoE-id motifs.
38ae12a to
b50229d
Compare
|
Nit taken, at
Your note about the regression shapes is right, and K=3712 / K=1472 with an element-wise weight format is where those live in the |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at b50229d50
Comment-only push, and it is the nit I raised: the header now says what the weight decode actually walks —
padded_input_size = input_size + 255 ; quant_block_count = padded / 256 (weight decode, `valid_k` gates every value)
vector.load.mask + `valid_input` (activation side)
— so "a K tail contributes zeros on both sides and can never read past the last weight block" is in the code where the next reader will look. I checked that no predicate line changed (diff of the two heads' own diffs shows only the comment hunk), so the review above stands as posted: F32 at K=3712/1472 flips 0→1, K=2560 unchanged, K<256/K>32768/K%32 still refused, and Q4_K/Q6_K still refused at those K.
|
#199 ready for review at b50229d Nit taken: the comment now names why a K tail cannot read past the last weight block ( |
…AT_ID (engine ggml-org#991) (#204) BlackMamba-2.8B loses 90 of 1179 nodes to the CPU and 72 of them are one MoE block pattern: MUL_MAT_ID x 54, GLU x 18. Its experts are [1472, 3872, 8] and [3872, 1472, 8], so the contractions are 1472 and 3872 -- both multiples of 32, neither a multiple of 256. common_mul_mat_id_supported_dense_input_size() forwarded to the dense table, whose default branch is % 256, and F16/BF16/F32 are not among its listed cases -- so element-wise expert weights inherited the block-quant default and were refused. The refusal is policy, not hardware: the three matchers that consult this predicate bind five kernels and all five declare or assume multiple_of(32), and the wmma core already pads the contraction (padded_input_size = input_size + 255) with a valid_k gate and masked activation loads. Route the predicate through common_mul_mat_supported_plain_dense_input_size(), the same element-wise/block split the plain dense path took in #199, so only F16/BF16/F32 verdicts change. Adds three test-backend-ops cases at the real shapes next to the existing gpt-oss k=2880 ones. Co-authored-by: agent <agent@local>
Root cause
HRX refuses a dense
MUL_MATwhoseinput_size(the contraction length,weight->ne[0]) is not a multipleof 256, so those matmuls silently run on the CPU while the rest of the model stays on the device. The refusal
is policy, not the kernel's contract:
ggml/src/ggml-hrx/kernel-corpus/kernels/loom-libs/ops/mul_mat_f32_f32_wmma.loom:44declares its own@ggml.mul_mat.input_sizeconstraint asrange(256, 32768), multiple_of(32).motifs/mul_mat_f32_f32_wmma_core.loom) already rounds the contraction up to whole 256-blocks(
padded_input_size = input_size + 255) and masks both sides pastinput_size—%valid_kon the weightdecode (
:117,valid_weight = valid_channel && valid_k) andvector.load.mask+%valid_inputon theactivation (
:133-138) — so a K tail contributes zeros.The 256 lives in the engine:
common_mul_mat_supported_dense_input_size()sendsF16/BF16/F32into itsdefault:arm, which is the block-quantised arm (%256). That arm is correct for block formats — a Q4_K/Q6_Krow is laid out in 256-value blocks, which is also why a quantised tensor with
ne[0] % 256 != 0cannotexist in ggml at all. Element-wise formats have no such structure.
Change (3 files, +23/-5)
dispatch-mul-mat-weight-format.h:common_mul_mat_supported_plain_dense_input_size()— element-wiseformats (
common_mul_mat_dense_float_format()) need only%32, everyone else keeps the existing predicate.dispatch-mul-mat-common.h: the wrapper plus an explicitelementwise_k_tailflag oncommon_match_mul_mat_any_format(), default off.dispatch-mul-mat.cpp: the flag is passed only by the three callers that dispatch the plain%32kernel —match_mul_mat_dispatch,match_mul_mat_unary_dispatch(unary is fused inside that same kernel) andmatch_decode_mul_mat_dispatch(decode wave64 takesinput_sizeas a runtime template argument).Deliberately not relaxed: the fused motifs that really do need 256 (
mul_mat_postopsbias/add/bias-add/add-rmsnorm,
mul_mat_id_f16_f16,llm.attention_qkv, the q6_K packed/shortlist and symmetric-i8 kernels —19 of the corpus's
input_sizedeclarations say 256, 10 say 32). Relaxing the shared predicate would havehanded
K = 3712to a kernel whose declared constraint forbids it. With the flag, a fused match simply doesnot engage and the node falls through to the plain matmul: the node stays on the device and only the fusion
is lost, instead of the whole node moving to the CPU.
Measured (gfx1151, gpu-hold, this branch;
GGML_HRX_STRICT=1reports totals)test-backend-ops -o MUL_MAT -b HRX0: 404/404,-o MUL_MAT_ID: 852/852 vs CPU, i.e. the newlyadmitted unaligned-K shapes match the reference exactly.
llama-serveris still' Paris.\nThe capital of the United States is Washington, D.C.', identical to the pre-change run; BlackMamba-2.8B decodes coherently(
' Paris, and the capital of France is Paris. Paris is the capital of France').llama-benchrelinked and the identicalarms re-run, so 145 -> 90 is not a cross-build comparison.
What this does not fix, and what I could not attribute
Three separate classes remain, and only the first is close to this change:
K % 32 != 0inside them - instrumentingcommon_match_mul_mat_any_formatabove its guards gives 48distinct signatures over 505,593 calls on Zamba-7B-v1 (p64), and the split is unambiguous: every 2-D signature has
K % 32 == 0(0 calls with
2-D && k % 32 != 0), so the size rule leaves nothing 2-D behind on this model. The remainder is rank>2:k=3712 -> o=264withinput->ne[2] = 2(K aligned, refused purely by the 2-D admission rule), andk=232 -> o=3712/k=464 -> o=256withne[2] = 2/16- those are3712/16and3712/8, so they areK % 32 != 0(232 % 32 = 8, 464 % 32 = 16)and rank>2, i.e. they need both a batched dispatch and a padded activation tile. On BlackMamba the same second class shows up in a
single shape:
dt_projis 2-D withK = ssm.time_step_rank = 92(92 % 32 = 28). None of that is addressable by this predicate, andnone of it is attempted here.
%256fused/MoE motifs -MUL_MAT_ID(54 on BlackMamba) andGLU(18) are matched by kernels whose owninput_sizeconstraint is 256, so they keep the old predicate.MUL_MATs on BlackMamba that I could not attribute. The first one STRICT aborts on isne=[2944,64,1,1]. Aninstrumented run of the shared matcher at p64 (82,403 lines, the complete log) contains no
tok=64 / out=2944row at all, so anode with those dims is either blocked before the shared matcher or fails its shape/type admission rather than the size rule. The
probe's coverage limit is exact, so I state it: the print sat after the matcher's own admission checks, so it only saw nodes that
are 2-D with an F32 activation — anything failing shape or type would be silently absent from it. Which of those two it is, I did
not determine, and I am not claiming those 18 as an alignment case.