Skip to content

npu: the forward derives GEMM K from hidden, but a Q4NX container pads K to the 256-column grid (hidden 896 -> 1024) #845

Closed

Description

@bong-water-water-bong

Found while making the OGA pack importer whole (PRs #842, #844): the shipped Qwen2.5-0.5B-Instruct_rai_1.8.0_npu_4K pack now converts to a complete Q4NX container (291 tensors / 67,088 q4_1 tiles, no refusals), and the next step is to run it on the NPU. That is blocked on one mismatch between how the container stores K and how the forward names and derives its GEMM designs.

The mismatch

A Q4NX q4_1 chunk covers 256 K-columns, so the container pads K up to that grid. Qwen2.5-0.5B has hidden 896 = 3.5 × 256, so every K=896 projection is stored with K = 1024 (the importer records this in the manifest; nothing reads a padding column as a weight).

The forward derives its designs from the model's own dimensions:

  • npu/forward/src/q4nx_pack.c:136 q4nx_derive_designs() — out[i].K = d->H (896) for QKV, O and G/U, and d->IM (4864) for D;
  • npu/forward/src/q4nx_forward.cpp:742 — the artifact probes and the elf names are formatted from the same dims.H, e.g. full_i8_QKV_K896_N1152.elf.

So the runtime looks for full_i8_QKV_K896_N1152.elf while the weights it is about to feed are packed over K = 1024, and the kernel that matches the container's geometry is full_i8_QKV_K1024_N1536.elf (built and bit-exact-verified on the array on 2026-11-01: qkv / gate_up / o / down / lm_head, all five passing against forge_bench's float64 reference).

And the two K values cannot simply be renamed into agreement: addons/npu-dx/forge/build.sh requires K % 256 == 0 (it pads K itself), so a legal 896-K ELF does not exist, while the store pads to 1024 by construction.

Every validated model so far has hidden == 0 mod 256 (5120, 6144, 2048 for the packs that use the int8 path), so nothing has exercised this. Packs whose hidden is not a multiple of 256 are the common case in the OGA collection (896, 1280, 2304, 2560, …).

What it needs (either is fine; one is smaller)

  1. Derive K on the padded grid: q4nx_derive_designs() uses K = ceil(H / 256) * 256 (and ceil(IM / 256) * 256 for D), so the design, the elf name and the launch all agree with the container's chunk grid. The importer already records the padded K in the container manifest, so this is a one-line derivation plus making the forward use the container's K rather than config.json's. It also gives the M=16 twins something to be found by.
  2. Dispatch per 256-column chunk inside one design (loop K in chunks of the chunk width), keeping K = H. Larger, and it changes the launch count per token.

Option 1 is the one I would take: it is the same padding decision the container already made, made once more in the layer that names the kernels.

Evidence available

Not in scope here: the .fconst side (that is #734, measure-don't-derive) and the pack importer itself (done).

Activity

  1. bong-water-water-bong commented on Nov 2, 2026

    @bong-water-water-bong
    CollaboratorAuthor

    Triaged — bug + npu added (it was enhancement only), bucket A, owner pi-ogaload. Two of the three code claims verified at origin/main = 5ef7d801d; the third is outside this repo. Plus one axis the issue does not mention that the same ELF set exposes, and which decides the fix's shape.

    Verified here

    claim at the pin
    the forward derives K from the model dims ✔ npu/forward/src/q4nx_pack.c:136 q4nx_derive_designs(), K = d->H at :149 (QKV), :171/:181/:190 (GU/G/U), :197-201 d->IM (D); O is :161 q_rows = NH*HD — 896 on this model, so the same value by coincidence, not by construction
    the artifact names come from the same dims ✔ npu/forward/src/q4nx_forward.cpp:742-744 formats full_i8_GU_K%d_N%d%s.elf / G from dims.H, and set_elf_name() (q4nx_pack.c) builds the rest from the same K/N
    the container pads K to the chunk grid ✔ npu/pack_import.cpp:529 KP = ceil(q.K / q4nx::kCols) * kCols, kCols = 256 (npu/q4nx.h:60); padding is zero-filled (scale 0, zero 0, codes 0 at :531-566), and :524-530 says so in prose
    addons/npu-dx/forge/build.sh requires K % 256 == 0 not checkable from this repo — git ls-tree origin/main addons/ is empty. Taken as your report, not verified

    Correction to one sentence in the issue, in your favour: the manifest does not need a new padded-K field. It records each tensor's real shape {N, K} plus declared_k (pack_import.cpp:720-730), and the padded K is a pure function of kCols — so remedy 1 is not "one-line plus a manifest change", it is one padding function plus reading K from the container's tensor shape instead of config.json's hidden. Same width, one less dependency.

    The in-tree precedent is real_n, and it is the same problem already solved once

    derive_mla() pads N and carries the real one beside it: kv_pad = ((kv_real + 127) / 128) * 128, out[i].N = kv_pad; out[i].real_n = kv_real (q4nx_pack.c:53-56,66), and the packer scatters only real_n columns and zero-fills the pad (q4nx_pack.c:367-383, contract in q4nx_pack.h:122-124). So the K pad has a shape to copy: pad K at derivation, keep real_k if anything needs it, and the elf name follows automatically because it is formatted from K.

    And the padded columns are numerically inert, which is the objection worth answering in the issue rather than in review: the container stores the pad as scale = 0, zero = 0, codes = 0 (pack_import.cpp:545-566), i.e. w = 0 in every padding column, so 128 extra K columns add exactly 0 to every dot product. The padded-K design is an exact reading of the container, not an approximation of it.

    The axis this issue does not mention: N is padded too, and not by 256

    Derived from config.json for Qwen2.5-0.5B-Instruct (H 896, IM 4864, 14/2 heads, HD 64, vocab 151936), against the five ELF names you point at in ~/oga-elfs:

    design derived K derived N ELF on disk
    QKV 896 1152 (896 + 2*128) full_i8_qkv_K1024_N1536_m16
    O 896 896 full_i8_o_K1024_N1024_m16
    gate_up 896 9728 full_i8_gate_up_K1024_N9728_m16 ✔
    down 4864 896 full_i8_down_K4864_N1024_m16
    lm_head 896 151936 full_i8_lm_head_K1024_N152064_m16

    So a K-only change still probes for full_i8_O_K1024_N896.elf and full_i8_down_K4864_N896.elf, which do not exist — the row would stay blocked with a different message. Those four N pads are also not one rule (896→1024, 151936→152064 = +128, 1152→1536 = +384), and two of them (1536, 152064) are not multiples of 1024, so under the decode-twin guard at q4nx_pack.c:123 (N % 128 == 0 && (N/128) % 8 == 0) they would not be found as M=16 twins at all — yet the directories carry _m16. That inconsistency is about how those five were built, and it is why I am not assuming the convention.

    The one fact I would fetch before writing the patch: the geometry that was actually used to build those five ELFs — the build record in npu-kernels ogaload/oga-qwen25-0.5b-bench-20261101 (and npu-pack info on the converted container beside it). If their N came from rounding each projection's rows up to the 256 grid (q 896→1024, k 128→256, v 128→256 = 1536, which is the only rule I can find that produces all five), then the derivation pads K and N per projection to kCols and the whole set agrees for every OGA geometry, not just this one.

    Next action, one

    Land remedy 1 as K and N on the container's grid, with real_k/real_n following the derive_mla idiom, and gate it on this container answering with no device: NPU_INFER_HOST_GEMM=1 on the converted pack (that path no longer demands an ELF set since #833 → f2d56162d), then the same five answers on the array under one npu-hold ticket. Closes when the pack answers; the ELF-set convention above is a prerequisite, not a separate issue.

    Links: #671 (the importer this unblocks — items 2–3 are the open PRs #842/#844, so this row and #671 close together or one after the other), #734 (the .fconst half; this one is the same "measure, do not re-derive" lesson on the geometry side), #833 (the host path that lets the acceptance run without the array), #754 (the M=16 family the twins belong to). Not claimed: no device time, no build, nothing run — this comment is a read of origin/main plus the ELF names on disk.

  2. added
    bugSomething isn't working
    npuThe NPU (XDNA2) forward/serve route
    on Nov 2, 2026
  3. bong-water-water-bong commented on Nov 2, 2026

    @bong-water-water-bong
    CollaboratorAuthor

    Remedy 1 is implemented and pushed: #854 (1bit/845-kn-grid @ 9fa8649).

    Your review comment asked for two things beyond the padding, and both are answered by measurement rather than assumption:

    1. The fact you said you would fetch first — the geometry the five ELFs were actually built at — is in npu-kernels ogaload/oga-qwen25-0.5b-bench-20261101: forge/forge.py's SHAPES[oga-qwen25-0.5b] plus pad_rows(n, cols) = 32*4*4 = 512. So the rule you inferred per projection (N up to the 256 grid — q 896→1024, k/v 128→256 ⇒ 1536) is the same rounding, and it is one rule, not one per projection: N to the 512 grid and K to the 256 grid, which is exactly the pads in bench-2026-11-01.json (1536, 1024, 9728, 152064).
    2. Your twin-guard observation was right and it was a second blocker: (N/128) % 8 == 0 (N % 1024) refuses QKV 1536, GU 9728 and the head 152064, i.e. three of the five ELFs that exist. Since a missing twin already falls back to the M=128 kernel, the guard is now the build family's own grid (q4nx_pad_n), and the oga geometry derives all four twins.
    3. Your correction is taken as given: no manifest field was added — real_k/real_n sit on Q4nxDesignGeom following derive_mla's idiom, and the padded K is a pure function of the grids.

    Acceptance as you wrote it: the container answers device-free (NPU_INFER_HOST_GEMM=1 NPU_NO_DEVICE_LOCK=1, Qwen2.5-0.5B OGA pack, derived names are now exactly the five built ELFs). Its text is not right yet — that is #855, filed separately so this row does not hide it. The array leg (npu-hold -l ogaload -t 2 on the same container) is next, and it needs the one thing this fix buys: kernels whose names the forward can now find.

  4. bong-water-water-bong commented on Nov 2, 2026

    @bong-water-water-bong
    CollaboratorAuthor

    Array-leg result — the container now initialises on the array and the ELF set is what stops it. Device time: 1 s in a npu-hold -t 3 ticket, 0 amdxdna errors, owner serve untouched.

    [INFO]  Model loaded: 291 tensors, 24 layers
    [forward] container K grid: H 896 -> 1024, IM 4864 -> 4864 (pad columns are zero)
    [forward] derived 4 designs (fused_gu=1, H=896 NH=14 NKV=2 HD=64 IM=4864)
    I8Ctx::init_elf ep=…/kernels-m16/full_i8_QKV_K1024_N1536.elf M=128 K=1024 N=1536 nl=1
    full ELF ok: uuid=f8891ca222a8f6e2083112178d716b89
    I8Ctx::init_elf failed: npu runtime: unsupported program (code 502): main:seq
    1bit forward-serve: forward init failed: I8Ctx init_elf failed for …/full_i8_QKV_K1024_N1536.elf
    

    So this PR's own acceptance — the derived name is the one that exists — is verified on the array: the forward derived full_i8_QKV_K1024_N1536.elf, found it (it is the M=16 ELF under its prefill name), and xrt::elf parsed it (full ELF ok, uuid printed). The stop is the next layer down.

    502 is kernel_missing ("no such kernel on this context", message carries the name — npu_rt_errors.h), and the name it carries is main:seq: the engine's forward dispatches through the control-packet/sequential flow, while the five oga-qwen25-0.5b ELFs built on 2026-11-01 are the 3-argument no-ctrl-packet kind (kernel main args=3, all global — the 11-01 probe line). They run bit-exact through the bench path (forge_bench_npurt, five tickets, 62 s, 0.0015-0.016 of the f32 allowance), which is why the bench record and this failure are both true.

    Two ways forward, and it is a kernel-side choice, not an engine one:

    1. rebuild the set with the control-packet variant (forge/build.sh knob) so main carries the seq argument the forward expects; or
    2. teach the forward the no-ctrl-packet dispatch for this family (the bench proves the ELFs are correct; what is missing is only the call convention).

    Nothing else about the array leg is unresolved: the container (291 tensors, complete pack), the derived names, the kernel directory layout (*.elf + *_m16.elf both present — the m16 label is a directory convention, byte-identical files) and the environment are all now in place, in one script: ~/ogaload/oga5/array_only.sh.

  5. bong-water-water-bong commented on Nov 2, 2026

    @bong-water-water-bong
    CollaboratorAuthor

    The 502 is the kernel instance name, not the call convention — measured, fixed, and the ELF set is functionally unchanged. Host-side only (no device time; a ticket with the fixed set is queued). Correcting my own 18:29 comment above, because both of the remedies it offered were aimed at the wrong layer.

    What main:seq actually is

    npu_rt names kernels "<kernel>:<instance>", takes both halves from the ELF's own COMDAT group symbols (STT_OBJECT = instance, its section index → STT_FUNC = mangled kernel; npu_rt.cpp:449-453), and find() requires both to match (:362) — a mismatch is errc::kernel_missing = 502 (npu_rt_errors.h:78). Bare "<kernel>" matches any instance, which is the one asymmetry that matters here.

    readelf -s on the two sets settles it:

    ELF symbols
    bonsai2-27b-NPU/kernels-m16/full_i8_O_K6144_N5120_m16.elf (deployed, i.e. what the engine's kname = "main:seq" is written for) _Z4mainPcPcPc + OBJECT seq
    forge/build.sh output from 2026-11-01 (all five shapes) _Z4mainPcPcPc + OBJECT sequence

    And the origin is a constant in the toolkit, not a design decision: aie/iron/runtime/runtime.py:333 emits RuntimeSequenceOp(sym_name="sequence"), so every IRON-built design lands as main:sequence. The op prints without a symbol when unnamed, which is why the MLIR text never showed it.

    So remedy 1 above ("rebuild with the control-packet variant") was unnecessary — these ELFs do carry the packet: .ctrltext.0 = 1,288 B, .pdi.1 = 114,128 B, and xrt::elf parsed the file (it printed full ELF ok: uuid=… before the 502). And remedy 2 ("teach the forward the no-ctrl-packet dispatch") would have papered over a name mismatch.

    Independent confirmation from our own side: addons/npu-dx/forge/bench/forge_bench.cpp:221 builds its kernel with xrt::ext::kernel(ctx, "main") — bare, no instance — and that is exactly why the bench ran these same files bit-exact on 11-01 (five tickets, 0.0015–0.016 of the f32 allowance) while the engine could not find them. Two consumers, same artifacts, one of them asking for a name the artifact does not have.

    Fix, and the proof it changes nothing else

    forge/build.sh now names the sequence at build time (aie.runtime_sequence @seq(), in the same injection step that already adds aiex.npu.load_pdi, with a hard failure if it cannot find exactly one such line. Rebuilt the five shapes (~/oga-elfs-seq/); against the 11-01 set (~/oga-elfs/):

    shape (n×k) instance .pdi.1 .ctrltext.0 .dynsym .rela.dyn size
    qkv 1536×1024 seq identical identical identical identical 123,032 B (same)
    o 1024×1024 seq identical identical identical identical 116,632 B (same)
    gate_up 9728×1024 seq identical identical identical identical 131,480 B (same)
    down 1024×4864 seq identical identical identical identical 139,672 B (same)
    lm_head 152064×1024 seq identical identical identical identical 116,376 B (same)

    The only differing bytes in the whole file are .strtab/.shstrtab/.symtab (.strtab is 5 B shorter: sequence → seq). So the rename is name-only; nothing was recompiled differently, nothing about the design moved.

    State of the array leg

    Kernel directory for the run now: ~/ogaload/oga4/kernels-m16-seq/ (five ELFs under both the _m16 and the prefill name, as before), wrapper ~/ogaload/oga5/array_seq.sh. The ticket is queued behind another lane's (npu-hold -l ogaload -t 3 -w 2700), and it reports the same four things as before: derived-vs-found kernel names, whether the M=16 twin loads, one short generation's whole-request tok/s, and the amdxdna error count — plus, for the first time, whether the kernels actually dispatch. Text correctness is still #855 and is not claimed by this run.

    FACTS + evidence for the naming contract, device-free and reproducible: npu-re 3b248eb (docs/FACTS-aie-artifacts-and-dispatch.md §6, evidence/oga/elf-seq-instance-2026-11-02/probe.sh). No engine change is needed for this — and I am deliberately not "fixing" it by asking for bare main, because that would silently pick the first group of a multi-instance ELF instead of resolving the name.

  6. bong-water-water-bong commented on Nov 3, 2026

    @bong-water-water-bong
    CollaboratorAuthor

    Array leg RESULT: the container loads and answers on the array — #845's acceptance is met. Two tickets, 11 s of device time, 0 amdxdna errors, owner serve untouched.

    [forward] container K grid: H 896 -> 1024, IM 4864 -> 4864 (pad columns are zero)
    [forward] derived 4 designs (fused_gu=1, H=896 NH=14 NKV=2 HD=64 IM=4864)
    [forward] QKV/O/GU/D   M=128 K=1024 …      ← prefill family, N 1536 / 1024 / 9728 / (K 4864, N 1024)
    [forward] QKV/O/GU/D   M=16 …              ← decode twins, all four roles
    [forward] decode family: 4 role(s) at M=16, 0 on M=128
    [forward] lm head: 16 packed weight tile(s) resident (H=896 T=9728), f32 head + f32 tile gather released
    
    {"choices":[{"text":"יהןיהןיהןיהןיהןיהןיהןיהן"}],"usage":{"completion_tokens":8,"prompt_tokens":5}}
    generated 8 tokens in 5.57 s -> 1.44 tok/s (whole request, incl. prefill)
    

    So: the derived names are the five that exist, K is the padded grid on the device, the M=16 twins load and are what decode uses, and the head's tiles are packed once instead of per token.

    Two faults stood between "initialises" and "answers", and both were this lane's, not the array's:

    1. main:seq — the ELF's kernel instance was sequence (IRON hard-codes the runtime-sequence symbol). Rebuilt the five ELFs with @seq; every functional section (.pdi.1, .ctrltext.0, .dynsym, .rela.dyn) is byte-identical to before, file sizes identical. Generator fix + FACTS §6, npu-kernels 6b6c70f28 / npu-re 3b248eb.
    2. the head's design lookup compared K == H — correct only while a model's hidden is already on the chunk grid, so after this PR's K pad no design matched (hidden 896 is the first non-multiple of 256 in the family). Both sites now use kpad_; fixed as 73ebf6f4 on npu: derive GEMM K and N on the container's and the kernel's grids, so a pack whose hidden is not a multiple of 256 has kernels to find (closes #845) #854, documented there with the reason CI cannot see it.

    Speed, with its bounds stated: 1.44 tok/s is a whole-request number (5 prompt + 8 generated tokens, so it includes load, the head's one-off pack and prefill). It is not a decode rate and should not be quoted as one. A ticket is queued to split prefill from decode at ctx 128/512 (~/ogaload/oga6/bench_rows.sh, the model_benchmark units this lane adopted on 10-31), and the ctx-2048 row follows if the 128/512 rows are affordable inside a ticket.

    Text is still #855 and this result does not touch it: the array answers יהן… and the host-GEMM path answers plementхudge… — different garbage from the same container, i.e. a degenerate argmax rather than a broken kernel path (the kernels are the ones verified bit-exact on 2026-11-01).

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingenhancementNew feature or requestnpuThe NPU (XDNA2) forward/serve route

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions