npu forward: read the batched prefill's GEMM outputs at the kernel's N stride (engine#855 part 2) by bong-water-water-bong · Pull Request #885 · 1bit-MONSTER/engine

bong-water-water-bong · 2026-11-03T04:19:56Z

Root cause

#854 derives every GEMM design's N on the kernel's column grid (q4nx_pad_n: 32 rows × 4 cols × 4 = 512), so npu_gemm_rows() writes M rows of the design's N. The batched prefill path read those buffers back at the real width:

X[m * H + k] += O[m * H + k] * residual_scale_;      // O's row stride is pad_n(H)
X[m * H + k] += D[m * H + k] * residual_scale_;      // D's row stride is pad_n(H)
av[m*2*im + i] = gv[m * im + i];                     // gv/uv row stride is pad_n(im)
act_row.assign(ACT.begin() + m * 2 * im, ...);        // the fused design's row stride is pad_n(2*im)

Row 0 is always right; row m is read m × (padded − real) floats early. So on any prompt longer than one token, every position after the first is corrupted — exactly the signature reported in #855: with the container's contents correct (#849), position 0 matches a numpy reference to 6 digits and positions ≥1 do not (layer-0 residual m=0 0.142969 6.0 both, m=1 0.113883 vs 0.162738).

Models already on the grid (Qwen3-0.6B/1.7B/4B, the 27B) are unaffected because q4nx_pad_n() is the identity for them — that is why this went unseen and why the generic path still answered sanely.

Measured before/after

OGA 0.5B container (AMD's Qwen2.5-0.5B-Instruct_rai_1.8.0_npu_4K, imported), host-GEMM, greedy:

path	raw completion of `The capital of France is`
batched (default), before	`意为： - 带有 …`
batched (default), this commit	`Paris. The capital of the United States is Washington`
sequential (`NPU_NO_BATCH=1`), for reference	`Paris. The capital of the United States is Washington`

and the chat prompt What is the capital of France? returns The capital of France is Paris. — AMD's own oracle string (~/ogaload/window/oracle.json). The engine's built-in oracle agrees: NPU_BATCH_CHECK=1 printed seq_argmax=12095 batch_argmax=36589 MISMATCH before; the two paths now produce the same text.

Localisation, so the credit is right: the divergence was not in the projections or the attention math. q/k/v (pre-RoPE) and attention()'s own output are byte-identical between the batched and sequential paths at every position of the prompt (temporary instrumentation inside attention(), removed); only the post-attention buffers differed, which is what pointed at the row strides.

Verification

fresh Release/ninja/gcc-16 configure off current main (23d6972d), with ONEBIT_NPU: builds clean; ctest -R "npu_design_grid|npu_forward_loader|npu_pack_import|npu_q4nx" → 4/4 pass (the fix is the identity for every on-grid geometry, so nothing else moves).
tools/copyright.py --check → rc 0.
the end-to-end text above was run on the #842 + #844 + #849 stack, because a container with a padded N cannot be produced from main alone (npu-pack import still refuses the MLP + final norm there). The fix itself does not depend on those PRs.

Relation to #855

Together with #879 (the importer's zero points paired even/odd like the codes, without which the container itself was wrong) this fixes #855 end to end: the container's weights, the sequential path and the batched path now all agree with AMD's own output on this pack. Not marked closes #855 because #879 has to land for the container half.

…N stride (engine#855 part 2) #854 derives each GEMM design's N on the kernel's column grid (q4nx_pad_n, 32*4*cols = 512), so npu_gemm_rows() writes M rows of the *design's* N. The batched prefill read those buffers back at the REAL width instead -- O[m*H], D[m*H], gv/uv[m*im], ACT[m*2*im] -- so row 0 was right and every later row was read k*(padded-real) floats early. On a prompt of >1 token that corrupts every position after the first, which is exactly the shape of #855: with the container's contents fixed (#849), position 0 matched a numpy reference to 6 digits and every later position did not. Models whose hidden/intermediate are already on the grid (Qwen3-0.6B/1.7B/4B, the 27B) are unaffected, because for them q4nx_pad_n() is the identity -- which is why this went unseen. Measured on the OGA 0.5B container (AMD's pack), host-GEMM, greedy, raw completion "The capital of France is": batched (default) "意为：" / "意为： - 带有" <- before batched, this commit " Paris. The capital of the United States is Washington" sequential (NPU_NO_BATCH=1) " Paris. The capital of the United States is Washington" and the chat prompt "What is the capital of France?" returns AMD's own oracle string, "The capital of France is Paris." The engine's own diagnostic agrees: NPU_BATCH_CHECK=1 printed `seq_argmax=12095 batch_argmax=36589 MISMATCH` before, and the two paths return the same text now.

github-actions · 2026-11-03T04:20:40Z

PR Reviewer Guide 🔍

(Review updated until commit `8cc5f1d`)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 854 - Partially compliant Compliant requirements: Derive GEMM K and N on the container's and the kernel's grids Pad K with q4nx_pad_k() and N with q4nx_pad_n() Assemble_B() scatters real rows and zeroes the kernel's n-tile pad The M=16 twin guard moves from `N % 1024 == 0` to the same 512 grid Fix the container-index walk that made the gate impossible Non-compliant requirements: None Requires further human verification: None 855 - Partially compliant Compliant requirements: The forward derives the geometry its kernels were actually built at Host-GEMM (no device, `NPU_INFER_HOST_GEMM=1 NPU_NO_DEVICE_LOCK=1`), greedy, on the imported container produces correct output Non-compliant requirements: None Requires further human verification: None 849 - Partially compliant Compliant requirements: Fix the q4_1 zero-point nibbles pairing in `npu/pack_import.cpp` Pair the zero-point nibbles as the codes do (`r0 + 2i` low, `r0 + 2i + 1` high) Regenerate `tests/golden/oga_attn_tile.bin` Non-compliant requirements: None Requires further human verification: None
⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Incorrect GEMM output stride handling The batched prefill path reads GEMM outputs at the kernel's N stride instead of the real width, which causes corruption in positions after the first token. This issue was introduced in the handling of the gemv family's weight image and the way GEMM outputs are read back. // The GEMM wrote M rows of the design's N, which is H only when H is already on the kernel's // column grid (engine#854 pads it otherwise): reading at H put every row after the first at the // wrong offset, which is why only position 0 of a prompt came out right (engine#855). { const int nrow = q4nx_pad_n(H); for (int m = 0; m < M; m++) for (int k = 0; k < H; k++) X[(size_t)m * H + k] += O[(size_t)m * nrow + k] * residual_scale_; } Incorrect GEMM output stride handling in MLP Similar to the attention path, the MLP path also reads GEMM outputs at the kernel's N stride instead of the real width, which can cause corruption in positions after the first token. { const int nrow = q4nx_pad_n(H); // as above: the design's N, identity when H is on the grid for (int m = 0; m < M; m++) for (int k = 0; k < H; k++) X[(size_t)m * H + k] += D[(size_t)m * nrow + k] * residual_scale_; }

The same class as the previous commit, in the path a QKV2 pack takes: qh0/qh1/kvk/kvv are written by npu_gemm_rows() as M rows of their design's N (padded, engine#854) but were read at the real widths nhd/nkv, so row 0 was right and every later row was shifted. The size guard follows the buffers to the same stride, so a short return is still caught. Correct by construction, and the identity whenever nothing is padded: the design's N is literally the row width npu_gemm_rows() assigned, so this cannot be wrong for an on-grid geometry (it compiles to the previous arithmetic there). No QKV2 pack is imported on this box, so this one is reasoning + the existing tests (4/4) rather than an end-to-end text, and that is stated rather than glossed.

bong-water-water-bong · 2026-11-03T04:22:36Z

Added ebc5c305 (appended, not amended, so the first commit's CI result stays attached): the QKV2 split path has the same class — qh0/qh1/kvk/kvv are written as M rows of their design's N (padded by #854) but were read at the real widths nhd/nkv, so row 0 was right and later rows shifted. The size guard now follows the buffers to the design's stride as well, so a short return is still caught.

Why this is safe without an end-to-end run: the design's N is the row width npu_gemm_rows() assigned, so the expression is the identity whenever q4nx_pad_n() padded nothing — for an on-grid model it compiles to the previous arithmetic. Stated honestly: no QKV2 pack is imported on this box, so that one is reasoning plus the 4/4 tests, not a measured text, unlike the OGA path in the first commit.

Scope of the class, for the record: I swept every npu_gemm_rows() consumer in q4nx_forward.cpp (14 call sites). The four fixed here plus O, D, gv/uv and the fused ACT are the ones that read with a real width; QKV/ZZ already used g->N, and run_lm_head_rows' gather indexes its own tile grid deliberately.

github-actions · 2026-11-03T04:23:16Z

Persistent review updated to latest commit ebc5c30

github-actions · 2026-11-03T04:29:33Z

Persistent review updated to latest commit ebc5c30

bong-water-water-bong

VERDICT: CHANGES — one of the five sites is right for one branch and wrong for the other. Everything else in the diff is
correct and I checked it.

What is right, verified at this head (ebc5c305): O and D are read at q4nx_pad_n(H) (they are npu_gemm_rows
outputs, so their rows are at the design's N); gv/uv at gg->N/gu->N; the QKV2 split's qh0/qh1 at gq->N and
kvk/kvv at gk->N, with the four short-buffer checks updated to the same widths (those vectors are resized by
npu_gemm_rows, so the checks are consistent). Identity on the grid is the right claim: q4nx_pad_n(1024) = 1024, and the
NPU lane builds clean with ctest -R "npu_design_grid|npu_forward_loader|npu_pack_import|npu_q4nx" 4/4.

The item — ACT needs two strides, not one:

3353:   std::vector<float> av((size_t)M * (size_t)im * 2);      // host-packed, REAL width
3369:       av[(size_t)m * 2 * im + i]        = gv[(size_t)m * gg->N + i];
3371:       av[(size_t)m * 2 * im + im + i]   = uv[(size_t)m * gu->N + i];
3375:   ACT.swap(av);                                          // ACT now holds rows at 2*im
...
3384:   const size_t act_w = (size_t)q4nx_pad_n(2 * im);
3385:       act_row.assign(ACT.begin() + (size_t)m * act_w, …);

ACT reaches that read from two branches with different layouts:

fused (i_gu >= 0): npu_gemm_rows(…, ACT, 2, "ffn_gate_up", …) writes it directly, so its rows are at the design's
N — reading at act_w is correct there, and that is the branch the header comment describes;
split (i_g/i_u): ACT is swapped from av, which is packed at the real width — reading at act_w is wrong
there. When pad_n(2*im) > 2*im, row m starts m × (act_w − 2*im) floats beyond its data, and the last row reads past
the end of the vector (an OOB read of (M−1) × (act_w − 2*im) floats).

It is latent on this box, which is why 4/4 tests and the OGA text are green: every intermediate_size in the containers
here is 512-aligned (2*im = 1536 / 6144 / 7168 / 34816, so act_w == 2*im), and so is the OGA pack's 4864. The first
container whose im % 256 != 0 — a plain im = 896-style or any non-multiple — takes the split branch and gets shifted
rows plus a read past the buffer, with no test in this repo covering it.

Two ways to fix, either is fine by me: pack the split branch at the design stride (std::vector<float> av((size_t)M * act_w)
and av[m * act_w + …), or keep av at real width and use 2*im for the split branch / act_w for the fused one. The
comment should then say which layout it is describing, because today it says "for the same reason as O and D" and the
reason does not hold for the swapped buffer.

Not reproduced by me: the end-to-end table. The body says it was run on the #842 + #844 + #849 stack, and none of those
three are on main (a container with a padded N cannot be imported from main alone), so the text claim is yours. The
stride reasoning above is checked against the source at this head, and the O/D/gv/uv/QKV2 fixes are the same
argument applied where it holds.

…oo (review of #885) ACT is read once, after the if/else, at the fused design's N (pad_n(2*im)) -- but it reached that read from two branches with different layouts: the fused branch by construction (npu_gemm_rows writes M rows of the design's N), and the split branch via a buffer packed at the REAL width (2*im). With pad_n(2*im) > 2*im that read puts row m at m*(act_w - 2*im) floats past its data and the last row past the end of the vector. Latent rather than live on this box, which is why every test and the OGA text stayed green: every intermediate_size in the containers here is 512-aligned (2*im = 1536/6144/7168/34816, and the pack's 4864), so act_w == 2*im. The first container with im % 256 != 0 takes the split branch and gets shifted rows plus an out-of-bounds read. Fixed by giving both branches the same layout instead of teaching the read two strides: the split branch now sizes its packed buffer at act_w (zero-filled, so the pad columns match what assemble_B leaves in the fused case) and indexes it at act_w. The comment says which layout it is describing rather than pointing at O/D, whose reason does not hold for a swapped buffer.

…nels) — #855's launcher ABI The OGA pack's kernels are a different launch family from the int8 per-tile one I8Ctx was written for: their runtime sequence declares (arg0 = the tensor's whole q4_1 tile stream, arg1 = K bf16 activations, arg2 = N f32 outputs), and its .dynsym symbols are per core, so arg0 and arg2 are 1/cores of the stream and of the output while arg1 is the whole activation row. Driving them with the int8 launcher is what made every array run of this pack either fault (IO_PAGE_FAULT bursts, 2026-11-02 20:35) or come back degenerate. The family is detected from the artifact, not from a config knob: q4nx_gemv_family() decides from the three declared widths (the same artifact-level information #869's guard reads), and I8Ctx then * sizes the activation BO K*2 (bf16) and the output BO N*4 (f32), * takes the weight BO as the tensor's own 5,120-byte q4_1 tiles ((K/256)*(N/32) of them), * launches (weights, activations, outputs), * converts the activation row f32 -> bf16 (round-to-nearest-even) and reads the result back as f32, skipping the int32 canary (its sentinel would land in an f32 buffer). The forward builds that weight image by copying the container's chunks -- row-tile-major, k-chunk inner, the order the vendor packer and the bit-exact 2026-11-01 bench both use -- so every tile keeps its own bf16 scale and zero; nothing is re-quantised. Fused designs (QKV, gate|up) concatenate their parts in the design's N order and pad the tail with zero row-tiles. The head uses its own tile tensor; a tied bf16 table has no tile stream and is refused by name rather than quantised here. The family has no M dimension (its _m16 ELFs are byte-identical to the one-row design), so npu_gemm_rows/run_lm_head_rows fall back to per-row launches: correct, and slower for prefill -- that is #671 item 2's m128 work, stated rather than hidden. Device-free verification, because the rule is where the first attempt went wrong: * q4nx_gemv_family() unit cases in tests/npu_elf_args_test.c: the five pack shapes classified from their measured declarations (QKV 245,760/2,048/1,536 at K=1024 N=1536 ... head 24,330,240/2,048/ 152,064), plus six near misses that must not match -- the int8 per-tile family (512/1,024/4,096), a full-width stream, a byte-count output, int8-width activations, K off the 256 grid, a zero stream. 11 declarations, all classified correctly. * the same rule run over the real artifacts (a harness calling the exact function init_elf uses): all five pack ELFs -> gemv, the 27B's int8 per-tile ELF -> not gemv. * ctest 5/5 (npu_elf_args, npu_design_grid, npu_forward_loader, npu_pack_import, npu_q4nx), copyright.py --check rc 0, and the host-GEMM path unchanged on the corrected container ("The capital of France is Paris.", AMD's own oracle string). Depends on #885 for the batched row strides; stacked on it rather than on main because the per-row fallback fills the same M*N buffers #885 fixed the callers to read.

bong-water-water-bong · 2026-11-03T10:48:11Z

Fixed at 591c51d3, taking your first option: both branches now give ACT the same layout, so there is one stride and the read cannot depend on which branch filled it.

split branch: the packed buffer is sized at act_w = pad_n(2*im) and zero-filled, written at av[m*act_w + …] from gv[m*gg->N] / uv[m*gu->N] — so its rows sit exactly where the fused branch's do, and the pad columns are 0 (the same thing assemble_B leaves in the fused case, so nothing new is read);
act_w is now computed once, before the if, and the comment says what it is: the stride both branches leave ACT at, with only the first 2*im columns of a row real — instead of pointing at O/D, whose reason indeed does not hold for a swapped buffer.

Your latent-only read is accepted as stated: every intermediate_size on this box is 512-aligned (2*im = 1536/6144/7168/34816, pack 4864), so act_w == 2*im and nothing existing could have shown it; the case that would have is im % 256 != 0 taking the split branch, and that is now the branch that packs at the padded width rather than the one that reads past its buffer.

Re-verified at this head: ctest -R "npu_elf_args|npu_design_grid|npu_forward_loader|npu_pack_import|npu_q4nx" 5/5, copyright.py --check rc 0, host-GEMM text unchanged (The capital of France is Paris.). #891 (the gemv launcher, stacked on this branch) was rebased onto the new tip so its diff stays clean — head a273cfe4.

bong-water-water-bong

VERDICT: READY

At 591c51d3. The item I blocked on is fixed the right way — one layout, one stride, rather than a per-branch rule:

3348  const size_t act_w = (size_t)q4nx_pad_n(2 * im);          // declared before BOTH branches
3359  std::vector<float> gv, uv, av((size_t)M * act_w, 0.0f);    // zero-filled at the design width
3379  av[(size_t)m * act_w + i]      = gv[(size_t)m * gg->N + i];
3380  av[(size_t)m * act_w + im + i] = uv[(size_t)m * gu->N + i];
3392  act_row.assign(ACT.begin() + m * act_w, … + 2 * im);

So the split branch now hands ACT rows at exactly the stride the fused branch produces (npu_gemm_rows writing M rows of
the design's N), the single read below is correct for both, and the OOB I measured is gone by construction: av is sized
M * act_w, every write is inside it, and the pad columns are zero rather than uninitialised. The rest of the PR is
unchanged and was already verified: O/D at q4nx_pad_n(H), gv/uv at gg->N/gu->N, qh0/qh1/kvk/kvv at
gq->N/gk->N with the size checks to match.

Built at this head and ran the lane's tests: ctest -R "npu_design_grid|npu_forward_loader|npu_pack_import|npu_q4nx|npu_elf_args" → 5/5 passed. That is the same set your
run covers, and it is the identity case (pad_n(2*im) == 2*im for every container here), so the value of the fix is in the
im % 256 != 0 case that no container on this box exercises — worth saying in the commit message, since the test suite
cannot distinguish the two layouts.

Two follow-ups, neither blocking this PR:

#891 is stacked on this branch (base = 1bit/855-batched-nstride), so after this merges it needs git rebase origin/main before it can land — same trap as #862/#872 earlier tonight.
The ACT comment now explains both branches, which is what the previous version was missing; if the fused branch's pad
columns are ever non-zero (assemble_B with a padded weight), the "matching what assemble_B leaves" sentence will be the
place to update.

github-actions · 2026-11-03T10:53:43Z

Persistent review updated to latest commit 6f9a995

bong-water-water-bong

VERDICT: READY

At 6f9a99562 (merge of main into 591c51d3d; 1 of 1 files blob-identical). Build success at this head.

The fix is unchanged and is the one-layout form I asked for: act_w = q4nx_pad_n(2 * im) declared before both branches,
the split gate/up branch's av sized M * act_w and zero-filled with writes at m * act_w, so the single read below is
correct for the fused and the split branch alike. Verified by building this tree: ctest -R "npu_design_grid|npu_forward_loader|npu_pack_import|npu_q4nx|npu_elf_args" 5/5.

Please merge this before #891, which is based on this branch (1bit/855-batched-nstride): once it lands, #891 needs
git rebase origin/main (it carries this branch's commits) and I re-verdict it at the rebased head — my check there is a
real-ELF classification harness plus npu_elf_args_test, both seconds.

…nels) — #855's launcher ABI The OGA pack's kernels are a different launch family from the int8 per-tile one I8Ctx was written for: their runtime sequence declares (arg0 = the tensor's whole q4_1 tile stream, arg1 = K bf16 activations, arg2 = N f32 outputs), and its .dynsym symbols are per core, so arg0 and arg2 are 1/cores of the stream and of the output while arg1 is the whole activation row. Driving them with the int8 launcher is what made every array run of this pack either fault (IO_PAGE_FAULT bursts, 2026-11-02 20:35) or come back degenerate. The family is detected from the artifact, not from a config knob: q4nx_gemv_family() decides from the three declared widths (the same artifact-level information #869's guard reads), and I8Ctx then * sizes the activation BO K*2 (bf16) and the output BO N*4 (f32), * takes the weight BO as the tensor's own 5,120-byte q4_1 tiles ((K/256)*(N/32) of them), * launches (weights, activations, outputs), * converts the activation row f32 -> bf16 (round-to-nearest-even) and reads the result back as f32, skipping the int32 canary (its sentinel would land in an f32 buffer). The forward builds that weight image by copying the container's chunks -- row-tile-major, k-chunk inner, the order the vendor packer and the bit-exact 2026-11-01 bench both use -- so every tile keeps its own bf16 scale and zero; nothing is re-quantised. Fused designs (QKV, gate|up) concatenate their parts in the design's N order and pad the tail with zero row-tiles. The head uses its own tile tensor; a tied bf16 table has no tile stream and is refused by name rather than quantised here. The family has no M dimension (its _m16 ELFs are byte-identical to the one-row design), so npu_gemm_rows/run_lm_head_rows fall back to per-row launches: correct, and slower for prefill -- that is #671 item 2's m128 work, stated rather than hidden. Device-free verification, because the rule is where the first attempt went wrong: * q4nx_gemv_family() unit cases in tests/npu_elf_args_test.c: the five pack shapes classified from their measured declarations (QKV 245,760/2,048/1,536 at K=1024 N=1536 ... head 24,330,240/2,048/ 152,064), plus six near misses that must not match -- the int8 per-tile family (512/1,024/4,096), a full-width stream, a byte-count output, int8-width activations, K off the 256 grid, a zero stream. 11 declarations, all classified correctly. * the same rule run over the real artifacts (a harness calling the exact function init_elf uses): all five pack ELFs -> gemv, the 27B's int8 per-tile ELF -> not gemv. * ctest 5/5 (npu_elf_args, npu_design_grid, npu_forward_loader, npu_pack_import, npu_q4nx), copyright.py --check rc 0, and the host-GEMM path unchanged on the corrected container ("The capital of France is Paris.", AMD's own oracle string). Depends on #885 for the batched row strides; stacked on it rather than on main because the per-row fallback fills the same M*N buffers #885 fixed the callers to read.

github-actions · 2026-11-03T12:18:09Z

Persistent review updated to latest commit e8bf018

github-actions · 2026-11-03T12:24:05Z

Persistent review updated to latest commit d5b3992

…nels) — #855's launcher ABI (#891) The OGA pack's kernels are a different launch family from the int8 per-tile one I8Ctx was written for: their runtime sequence declares (arg0 = the tensor's whole q4_1 tile stream, arg1 = K bf16 activations, arg2 = N f32 outputs), and its .dynsym symbols are per core, so arg0 and arg2 are 1/cores of the stream and of the output while arg1 is the whole activation row. Driving them with the int8 launcher is what made every array run of this pack either fault (IO_PAGE_FAULT bursts, 2026-11-02 20:35) or come back degenerate. The family is detected from the artifact, not from a config knob: q4nx_gemv_family() decides from the three declared widths (the same artifact-level information #869's guard reads), and I8Ctx then * sizes the activation BO K*2 (bf16) and the output BO N*4 (f32), * takes the weight BO as the tensor's own 5,120-byte q4_1 tiles ((K/256)*(N/32) of them), * launches (weights, activations, outputs), * converts the activation row f32 -> bf16 (round-to-nearest-even) and reads the result back as f32, skipping the int32 canary (its sentinel would land in an f32 buffer). The forward builds that weight image by copying the container's chunks -- row-tile-major, k-chunk inner, the order the vendor packer and the bit-exact 2026-11-01 bench both use -- so every tile keeps its own bf16 scale and zero; nothing is re-quantised. Fused designs (QKV, gate|up) concatenate their parts in the design's N order and pad the tail with zero row-tiles. The head uses its own tile tensor; a tied bf16 table has no tile stream and is refused by name rather than quantised here. The family has no M dimension (its _m16 ELFs are byte-identical to the one-row design), so npu_gemm_rows/run_lm_head_rows fall back to per-row launches: correct, and slower for prefill -- that is #671 item 2's m128 work, stated rather than hidden. Device-free verification, because the rule is where the first attempt went wrong: * q4nx_gemv_family() unit cases in tests/npu_elf_args_test.c: the five pack shapes classified from their measured declarations (QKV 245,760/2,048/1,536 at K=1024 N=1536 ... head 24,330,240/2,048/ 152,064), plus six near misses that must not match -- the int8 per-tile family (512/1,024/4,096), a full-width stream, a byte-count output, int8-width activations, K off the 256 grid, a zero stream. 11 declarations, all classified correctly. * the same rule run over the real artifacts (a harness calling the exact function init_elf uses): all five pack ELFs -> gemv, the 27B's int8 per-tile ELF -> not gemv. * ctest 5/5 (npu_elf_args, npu_design_grid, npu_forward_loader, npu_pack_import, npu_q4nx), copyright.py --check rc 0, and the host-GEMM path unchanged on the corrected container ("The capital of France is Paris.", AMD's own oracle string). Depends on #885 for the batched row strides; stacked on it rather than on main because the per-row fallback fills the same M*N buffers #885 fixed the callers to read. Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg>

github-actions · 2026-11-03T12:27:04Z

Persistent review updated to latest commit 8cc5f1d

… in the rebase The forge q4_1 gemv family landed in npu_gemm/npu_gemm_rows after this branch started, so the phase that claims to move those bodies has to say where it put them: the tile image is model state, the launch is the target's, and GemvArgs::tiles is the one seam between them. Doc only; no code.

…ixes #803) Same content as the 13-commit stack on npu/plan-p4-target, re-applied onto origin/main 7153bed as ONE commit: the branch had accumulated fifteen "merge origin/main" commits, one per re-gate, and the repo squashes on merge anyway. npu(p4): extract the XDNA launch layer behind NpuTarget (engine#799 P4 / #803) docs(npu-p4): the Apache-2.0 notice tools/copyright.py requires npu(p4): consume P1's plan loader in NpuTarget::open instead of duplicating the derivation npu(p4): the LM head falls back through ctx_for(), so a set with no M=16 twins still runs npu(p4): the pb16i role split is the target's state, so setup asks it instead of assigning npu(p4): the kernel family is the forward's to name, so open() takes it npu(p4): the rotation is one implementation, so the target calls hadamard.h too npu(p4): the head's ternary flag is head_pb16i_, not a self-reference npu(p4): the rebase put a_k in the wkey/sc slots, so pass all three as main does docs(npu-p4): name the gemv family's boundary in A1, now that #885 is in the rebase npu(p4): the m16 debug comparison takes its difference at int64 width npu(p4): the target headers take npu-relative includes, so CI's syntax target compiles them chore: re-trigger CI Rebased rather than merged this time, because the merge caravan is what has been voiding the gate: each "merge origin/main" moved the head, so fourteen pairings were spent keeping a table honest against a tree that kept moving. The split's own files are byte-identical to the reviewed ones (blob check in the PR): npu/target/target.h, npu/target/xdna/*, npu/forward/include/q4nx_lh.h, docs/npu-p4-target-split.md. The three that differ are the three main changed: CMakeLists.txt (main's test registrations), and q4nx_forward.{h,cpp}, which now carry main's newer forward work (#1243's AVX2/FMA attention, #1266's MoE container) *on top of* the split -- verified by grepping the moved-in symbols in the rebased file, not by trusting the clean merge. Nothing in here changes numerics: the device and launch layer moved, the 38 launch sites became role-based calls, and the model math stayed where it was.

…ixes #803) Same content as the 13-commit stack on npu/plan-p4-target, re-applied onto origin/main 7153bed as ONE commit: the branch had accumulated fifteen "merge origin/main" commits, one per re-gate, and the repo squashes on merge anyway. npu(p4): extract the XDNA launch layer behind NpuTarget (engine#799 P4 / #803) docs(npu-p4): the Apache-2.0 notice tools/copyright.py requires npu(p4): consume P1's plan loader in NpuTarget::open instead of duplicating the derivation npu(p4): the LM head falls back through ctx_for(), so a set with no M=16 twins still runs npu(p4): the pb16i role split is the target's state, so setup asks it instead of assigning npu(p4): the kernel family is the forward's to name, so open() takes it npu(p4): the rotation is one implementation, so the target calls hadamard.h too npu(p4): the head's ternary flag is head_pb16i_, not a self-reference npu(p4): the rebase put a_k in the wkey/sc slots, so pass all three as main does docs(npu-p4): name the gemv family's boundary in A1, now that #885 is in the rebase npu(p4): the m16 debug comparison takes its difference at int64 width npu(p4): the target headers take npu-relative includes, so CI's syntax target compiles them chore: re-trigger CI Rebased rather than merged this time, because the merge caravan is what has been voiding the gate: each "merge origin/main" moved the head, so fourteen pairings were spent keeping a table honest against a tree that kept moving. The split's own files are byte-identical to the reviewed ones (blob check in the PR): npu/target/target.h, npu/target/xdna/*, npu/forward/include/q4nx_lh.h, docs/npu-p4-target-split.md. The three that differ are the three main changed: CMakeLists.txt (main's test registrations), and q4nx_forward.{h,cpp}, which now carry main's newer forward work (#1243's AVX2/FMA attention, #1266's MoE container) *on top of* the split -- verified by grepping the moved-in symbols in the rebased file, not by trusting the clean merge. Nothing in here changes numerics: the device and launch layer moved, the 38 launch sites became role-based calls, and the model math stayed where it was. One row of the gate was a citation rather than a check, and is now a check: §4 item 4 asks for `--npu-target xdna` as the default with an unknown name refused by name, and A1 had no such option at all -- the name appeared only in a comment in npu/target/target.h and in the design doc, so the earlier "PASS: from the A1 design row" was a claim about the document. The flag now exists: `forward-serve --npu-target <name>` (defaulting to XdnaTarget::kName), `serve` carries it through to that child, and an unknown name is refused **before the device is opened**, which is what makes the row measurable without a ticket.

bong-water-water-bong mentioned this pull request Nov 3, 2026

npu: the OGA 0.5B pack loads and answers in the engine, but the first greedy token is wrong (flat head over a large-magnitude stream) #855

Closed

github-actions Bot added the Review effort 4/5 label Nov 3, 2026

bong-water-water-bong commented Nov 3, 2026

View reviewed changes

bong-water-water-bong mentioned this pull request Nov 3, 2026

npu forward: drive the forge q4_1 GEMV ELF family (the pack's own kernels) — #855's launcher ABI #891

Merged

bong-water-water-bong commented Nov 3, 2026

View reviewed changes

bong-water-water-bong added the tprs-ready TPRS: review verdict READY posted by the standing reviewer; merge gate satisfied label Nov 3, 2026

Merge branch 'main' into 1bit/855-batched-nstride

6f9a995

bong-water-water-bong commented Nov 3, 2026

View reviewed changes

Merge branch 'main' into 1bit/855-batched-nstride

e8bf018

Merge branch 'main' into 1bit/855-batched-nstride

d5b3992

bong-water-water-bong enabled auto-merge (squash) November 3, 2026 12:22

bong-water-water-bong merged commit 8e8780e into main Nov 3, 2026
8 checks passed

bong-water-water-bong deleted the 1bit/855-batched-nstride branch November 3, 2026 12:30

This was referenced Nov 3, 2026

npu gate: a folded-container row on the container's own taps, and the embedding load it caught on main #886

Merged

os/v3/puppy: init resolves pupsfs=UUID=…, so an installed disk boots (closes #865) #890

Merged

bong-water-water-bong mentioned this pull request Nov 5, 2026

npu: count and time the host's own row copies (NPU_COPY_PROF=1), and remove the two the count exposed (15.38 -> 2.63 MiB per prefill row) #1142

Merged

Navigation Menu

Repository navigation

npu forward: read the batched prefill's GEMM outputs at the kernel's N stride (engine#855 part 2) - #885

npu forward: read the batched prefill's GEMM outputs at the kernel's N stride (engine#855 part 2)#885
bong-water-water-bong merged 7 commits into
mainfrom
1bit/855-batched-nstride

bong-water-water-bong commented Nov 3, 2026

Uh oh!

github-actions Bot commented Nov 3, 2026 •

edited

Loading

Uh oh!

bong-water-water-bong commented Nov 3, 2026

Uh oh!

github-actions Bot commented Nov 3, 2026

Uh oh!

github-actions Bot commented Nov 3, 2026

Uh oh!

bong-water-water-bong left a comment

Uh oh!

bong-water-water-bong commented Nov 3, 2026

Uh oh!

bong-water-water-bong left a comment

Uh oh!

github-actions Bot commented Nov 3, 2026

Uh oh!

bong-water-water-bong left a comment

Uh oh!

github-actions Bot commented Nov 3, 2026

Uh oh!

github-actions Bot commented Nov 3, 2026

Uh oh!

github-actions Bot commented Nov 3, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

bong-water-water-bong commented Nov 3, 2026

Root cause

Measured before/after

Verification

Relation to #855

Uh oh!

github-actions Bot commented Nov 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Reviewer Guide 🔍

(Review updated until commit 8cc5f1d)

Uh oh!

bong-water-water-bong commented Nov 3, 2026

Uh oh!

github-actions Bot commented Nov 3, 2026

Uh oh!

github-actions Bot commented Nov 3, 2026

Uh oh!

bong-water-water-bong left a comment

Choose a reason for hiding this comment

Uh oh!

bong-water-water-bong commented Nov 3, 2026

Uh oh!

bong-water-water-bong left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Nov 3, 2026

Uh oh!

bong-water-water-bong left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Nov 3, 2026

Uh oh!

github-actions Bot commented Nov 3, 2026

Uh oh!

github-actions Bot commented Nov 3, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

github-actions Bot commented Nov 3, 2026 •

edited

Loading

(Review updated until commit `8cc5f1d`)