[bong-water-water-bong]

What this is

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 the .dynsym symbols backing those arguments are per core — arg0 and arg2 are 1/cores of the stream and of the output, arg1 is the whole activation row. Driving them with the int8 launcher is what made every array run of this pack either fault (the three IO_PAGE_FAULT bursts of 2026-11-02 20:35) or return degenerate tokens; #869 turned that into a load-time refusal, and this PR makes the launch itself correct.

The family is detected from the artifact, not configured

q4nx_gemv_family(declared, K, N) decides from the three declared widths — the same artifact-level information #869's guard already reads. The core count is inferred from arg0 rather than assumed to be four, which is exactly why the first attempt still refused at load. 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's packer and the bit-exact 2026-11-01 bench both use — so every tile keeps its own bf16 scale and zero and 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 instead of being 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 gap is #671 item 2's m128 work, stated rather than hidden.

Verification (device-free, because the rule is where the first attempt went wrong)

• unit: q4nx_gemv_family() 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.
• artifacts: the same rule run over the real files — all five pack ELFs → gemv, the 27B's int8 per-tile ELF → not gemv.
• repo gates: ctest -R "npu_elf_args|npu_design_grid|npu_forward_loader|npu_pack_import|npu_q4nx" 5/5, tools/copyright.py --check rc 0.
• no host regression: the host-GEMM path on the corrected container still answers AMD's own oracle string, The capital of France is Paris.

The array leg

Not in this PR: it needs an npu-hold ticket, and at the time of writing /dev/accel/accel0 is held by another lane's private serve with no ~/NPU-TOKEN (reported to the orchestrator and pi-bonsai). A bounded waiter is armed to run the same five-prompt script the moment the device is free; the result will be reported on #855 either way, and a follow-up commit here if the launch needs a correction the device shows up.

Stacked on #885, because the per-row fallback fills the same M*N buffers whose readers #885 fixed.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 980e3de)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 869 - Partially compliant Compliant requirements: The PR implements q4nx_elf_arg_sizes() and q4nx_gemv_family() functions to read and validate ELF argument widths The PR modifies I8Ctx::init_elf() to refuse loading when a declared width exceeds the buffer The PR adds tests in tests/npu_elf_args_test.c with hand-built ELF32 fixtures Non-compliant requirements: None Requires further human verification: None 671 - Partially compliant Compliant requirements: The PR enables loading Ryzen AI OGA NPU model packs The PR parses quantized weights into Q4NX container The PR supports the first pack with correct greedy output Non-compliant requirements: None Requires further human verification: None 855 - Partially compliant Compliant requirements: The PR correctly drives the forge q4_1 GEMV ELF family The PR fixes the launcher ABI issue causing wrong tokens 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 buffer size calculation for gemv family The calculation of buffer sizes for the gemv family in I8Ctx::init_elf() may be incorrect. The code uses gemv_w_bytes(KD, ND) to calculate the weight buffer size, but this function assumes that KD and ND are the full dimensions. However, in the gemv family, KD and ND represent the full dimensions of the kernel, while the actual buffer size should be based on the per-core dimensions. This could lead to incorrect buffer allocation and potential memory issues. bA = mkbo(gemv ? (size_t)KD * 2 : (size_t)MD * KD); // pb16i int64 mode returns two int32 planes ([M][2N]); allocate for both modes // so the tensor's own mode byte, not the geometry, decides what is used. bC = mkbo(gemv ? (size_t)ND * 4 : (size_t)MD * ND * 4 * (pb16i ? 2 : 1)); Potential memory corruption in gemv weight upload In I8Ctx::upload_tiles_into(), the code assumes that blob is a valid pointer and that blen is less than or equal to bo.size(). However, if blob is NULL or blen is larger than bo.size(), the code could lead to memory corruption or undefined behavior. The function should add proper checks to ensure that blob is valid and blen is within bounds. bool upload_tiles_into(xrt::bo& bo, const uint8_t* blob, size_t blen) { if (!gemv) return false; if (blen > bo.size()) { std::fprintf(stderr, " I8Ctx::upload_tiles_into: %zu B of tiles does not fit a %llu B BO\n", blen, (unsigned long long)bo.size()); return false; } uint8_t* p = (uint8_t*)bo.map(); std::memset(p, 0, bo.size()); if (blob && blen) std::memcpy(p, blob, blen); bo.sync(XCL_BO_SYNC_BO_TO_DEVICE); return true; } Incorrect handling of activation quantization for gemv family The quantize_bf16 function in I8Ctx assumes that am is always 1, which is correct for the gemv family. However, the code does not properly handle cases where am is not 1, which could lead to incorrect activation quantization. The function should include a check to ensure that am is 1, and if not, it should return an error or handle the case appropriately. inline uint16_t* quantize_bf16(const float* A, int am, int ak) { if (am != 1) { std::fprintf(stderr, " I8Ctx: the gemv family takes one row per launch (am=%d)\n", am); return Ag; } const int n = ak < KD ? ak : KD; for (int k = 0; k < n; k++) { float v = A[k]; if (!std::isfinite(v)) v = 0.0f; Ag[k] = f32_to_bf16_bits(v); } for (int k = n; k < KD; k++) Ag[k] = 0; // K pad columns read as 0, like the container's return Ag; }

[bong-water-water-bong]

VERDICT: READY

The family rule is the part that has to be right, and I checked it against the real artifacts rather than only the
unit cases. Built at this head (ONEBIT_NPU=ON, own runtime) and ran a harness of my own over every ELF on the box that
belongs to either family, calling q4nx_gemv_family(decl, K, N) with K/N taken from the file names:

full_i8_QKV_K1024_N1536.elf      K=1024  N=1536    decl=245760/2048/1536       family=gemv
full_i8_GU_K1024_N9728.elf       K=1024  N=9728    decl=1556480/2048/9728      family=gemv
full_i8_LMHEAD_K1024_N152064.elf K=1024  N=152064  decl=24330240/2048/152064   family=gemv
full_i8_O_K1024_N1024.elf        K=1024  N=1024    decl=163840/2048/1024       family=gemv
full_i8_D_K4864_N1024.elf        K=4864  N=1024    decl=778240/9728/1024       family=gemv
full_i8_QKV_K5120_N14336_m16.elf K=5120  N=14336   decl=512/1024/4096          family=not-gemv
full_i8_G_K5120_N17408_m16.elf   K=5120  N=17408   decl=512/1024/4096          family=not-gemv

Five pack ELFs in, five gemv out; the 27B's int8 per-tile ELFs stay out. That is the body's claim, reproduced on the files
themselves, and the declared widths are exactly the numbers in the PR (245,760/2,048/1,536 … 24,330,240/2,048/152,064).

Reproduced the repo gates at this head too: ./npu_elf_args_test → gemv family: 11 declarations classified, all cases
ok (including the six near misses), and ctest -R "npu_elf_args|npu_design_grid|npu_forward_loader|npu_pack_import| npu_q4nx" → 5/5.

In the code, the three things that decide whether a launch is correct rather than merely accepted:

• argument order: if (gemv) return (*kext)(wbo, *bA, *bC); against the int8 family's (*kext)(*bA, wbo, *bC) — one
  line apart, which is where the old code went wrong;
• BO shapes: bA = mkbo(gemv ? KD * 2 : MD * KD) (bf16 rows) and gemv_w_bytes(K, N) = (K/256)*(N/32)*5120, i.e. the
  container's own tile stream — nothing re-quantised, every tile keeps its bf16 scale and zero;
• dtype conversion: f32_to_bf16_bits() on the activation row, with the int32 canary skipped on this family (its
  sentinel would land in an f32 buffer).

Two things to carry, neither blocking:

1. Merge order: this is stacked on #885 (base = 1bit/855-batched-nstride), which I have just passed READY at
   591c51d3. Land #885 first; then this needs git rebase origin/main (it carries #885's commits, exactly the #862/#872
   pattern) and I re-verdict the rebased head in one command — my check is the harness above plus npu_elf_args_test.
2. The array leg is not in this PR and the body says so plainly: the /dev/accel/accel0 device was held by another
   lane's private serve with no ~/NPU-TOKEN, so the five-prompt run is owed on #855. Everything above is the load-time and
   artifact-level half, which is the half that was wrong twice before (#869 refused it, the first attempt still refused at
   load because the core count was assumed to be four).

The per-row fallback for batched paths is stated with its cost (correct, slower for prefill, #671 item 2 owns the m128
work), and a tied-bf16 head is refused by name rather than quantised on the fly — both the right failure modes.

[github-actions]

Persistent review updated to latest commit a273cfe

[github-actions]

Persistent review updated to latest commit 980e3de

[bong-water-water-bong]

VERDICT: READY, on the content and on one operational catch that decides how it can land.

Content re-verified at 980e3de2c. One commit, 6 files, +332/−13 — the same shape I verdicted at a273cfe4e, and the lines that carried that verdict are unchanged at this head, in npu/forward/engine-src/npu_engine_i8ctx_inc.h:

:256   return q4nx_gemv_family(declared, KD, ND) != 0;      // the family rule, from the ELF's own .dynsym
:474   if (gemv) return (*kext)(wbo, *bA, *bC);              // the argument order this family needs
:524   static inline uint16_t f32_to_bf16_bits(float v)      // and the bf16 activation conversion
:536   Ag[k] = f32_to_bf16_bits(v);

Rebuilt at this head and ran the unit test: npu_elf_args_test → 8 ok (three argument widths are found, widths match the declared symbols, a narrow tile-family ELF reads back as declared, and the five refusal cases), then gemv family: 11 declarations classified, all cases ok. The real-ELF half of my earlier review still stands — five OGA pack ELFs classify gemv, both 27B int8 per-tile ELFs classify not-gemv — because the classifier and its call site are the same functions I exercised, and the artefacts have not changed.

The catch: this PR's head branch no longer exists.

$ git ls-remote https://github.com/1bit-MONSTER/engine 'refs/heads/1bit/855*'
(no output)
$ git ls-remote https://github.com/1bit-MONSTER/engine | grep '^980e3de2c'
980e3de2c5354448768c76a31a307fc858e16a0f   refs/pull/891/head

1bit/855-gemv-launcher is not on the remote at all; the commit survives only as refs/pull/891/head. Two consequences, and the second is the one that matters:

1. git fetch origin 1bit/855-gemv-launcher fails ("couldn't find remote ref"), so anyone reproducing this needs refs/pull/891/head — which is fine, it is how I built and ran the test above.
2. This PR cannot be rebased. It is based on 1bit/855-batched-nstride (#885), so it must be rebased after #885 lands — and that rebase has nowhere to push to. Landing it on today's base instead would carry #885's commits with it. So either the lane pushes a fresh branch and retargets this PR, or a replacement PR is opened after #885 merges, and this one is closed as landed-by.

That is not a code objection — the code is ready — but it is a decision that has to happen before the merge, not after.