Skip to content

npu: max_batch_rows() reports 1 on every sealed pack, so the sealed route never makes a multi-row pass (and #1781 gate 1 cannot be measured) #1900

Closed

Description

@bong-water-water-bong

What happens

max_batch_rows() returns 1 on all three sealed packs I ran, so every batched pass is chunked to a single row:

1bit npu-run: sealed kernel set (14 ELF(s)), one multi-row pass through the in-tree forward
1bit npu-run: 7 ids, so this runs in passes of 1 rows (the kernel's width)
pack kernels plan decode/prefill M reported width
~/scratch-packs/imp1519b npu/ (8 ELFs) — 1
models/npu/bonsai2-27b-t2s2 kernels-m16 16 / 128 1
models/npu/bonsai2-27b-NPU-b8 kernels (14 ELFs) 16 / 128 1

(host-GEMM and device runs agree; the device run was under an npu-hold ticket, exit 0 in 5m32s.)

Why

npu/forward/src/q4nx_forward.cpp:3897:

int Q4nxNpuForward::max_batch_rows() const { return xd_ ? xd_->max_rows() : 1; }

and npu/target/xdna/xdna_target.cpp:283:

int XdnaTarget::max_rows() const {
    if (pb16i_) { for (int i = 0; i < ndesigns_; i++) if (ctx16_[i]) return ctx16_[i]->MD; }
    for (int i = 0; i < ndesigns_; i++) if (ctx_[i]) return ctx_[i]->MD;
    return 1;
}

The sealed route keeps its design in one ELF (pb16i_sealed_, per-role instances) and does not populate the per-role ctx_[i] / ctx16_[i] arrays, so both loops find nothing and the function falls through to 1. The plan's widths are 16/128; the ELF the plan names is compiled for them.

Two consequences, one silent and one not:

The fix has a direction, and getting it wrong is memory corruption

max_rows() is used as the chunk width (q4nx_forward.cpp:3162, const int blk = max_batch_rows(); if (blk > 0 && M > blk) …), and the comment beside it records what an over-wide launch does: the activation BO holds exactly that many rows and a wider launch "reads and poisons past the BO, which is memory corruption, not a slow path (engine#1372)". So the sealed path's width must be the design's own M, read from the sealed ELF/plan that set_pb16i(on, dir, sealed) already receives — not setup_.M guessed, and not the prefill width when the loaded design is the decode one.

A minimum over the loaded per-role widths is the safe aggregate even on the array path: max_rows() returns the first populated entry today, so two roles at 16 and one at 128 already report whichever happens to be index 0 — 128, i.e. over-wide for the two 16-wide roles.

What would settle it

  1. Print the width once at load ([forward] … batch width N), the way the block note does, so a log can be told apart from a forward that never batched.
  2. Read the sealed design's M and use it; consider min() over the populated per-role widths for the array path.
  3. Arm: a plan whose sealed ELF declares M=16 must report 16, and a crafted design with mixed widths (16/16/128) must report 16 rather than 128 — the second is the memory-corruption direction and the one worth a fixture.

Measured 2026-11-10 on this host; 1bit built with -DONEBIT_NPU=ON from fix/1862-npu-run-sealed (6a2b41784, PR #1894). Refs #1862, #1781, #1372.

Activity

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

    @bong-water-water-bong
    CollaboratorAuthor

    Claiming this and posting what I found, because the fix location is unambiguous even though one semantic choice in it is not — and I would rather hand over a precise question than a
    half-verified patch (the box is at 156 G, under the 175 G floor this lane builds under).

    The width is available without an open context

    max_rows() returns 1 only because it looks solely at the opened contexts:

    int XdnaTarget::max_rows() const {
        if (pb16i_) { for (int i = 0; i < ndesigns_; i++) if (ctx16_[i]) return ctx16_[i]->MD; }
        for (int i = 0; i < ndesigns_; i++) if (ctx_[i]) return ctx_[i]->MD;
        return 1;                                  // <- the sealed route lands here
    }

    but the same file already reads the width from the designs where no context exists — xdna_target.cpp:485:

    const int M = (ctx16_[i] && ph == 0) ? designs16_[i].M : designs_[i].M;

    and open_prefill(designs, n, err) notes a compiled set "without constructing a target or opening a device" (the header's own comment, xdna_target.h:72). So designs_[i].M
    is the value the plan was built from, it is present exactly when the contexts are not, and rows_for(role, want) already has the softer version of this fallback (it returns want
    rather than 1 when there is no context). max_rows() is the outlier that answers 1.

    The one thing I cannot settle from the code: max, or prefill?

    The three candidates for the fallback value are the largest designs_[i].M, the prefill design's M (128 in the table you posted), or the decode design's M (16). The consumer
    decides which is right, and I have not traced it:

    • npu-run's "7 ids, so this runs in passes of N rows" wants the widest legal pass, which argues for the maximum;
    • but max_batch_rows() is also what q4nx_block_note() reports against the prefill width (my npu: say the prefill's block width out loud when it is narrower than the prefill designs (Refs #1582) #1590), and that note's whole point is that a prefill blocked below the prefill width
      is worth saying out loud — so if this returns a decode-only 16 on a prefill-capable set, the note would fire on every pack and stop meaning anything.

    So the fix is one of:

    // widest legal pass
    for (int i = 0; i < ndesigns_; i++) if (designs_[i].M > best) best = designs_[i].M;
    // or: the prefill design's M, which is what the block note compares against

    and choosing wrong turns a silent 1 into a wrong number that the note then reports confidently — which is worse. One device run with the 14-ELF pack and npu-run's pass count in the
    log settles it in minutes, and it is the reason I am not opening a PR with a coin-flip in it: this is the same "a plausible number instead of a refusal" failure the rest of tonight's
    fixes were about.

    What I would add as the test either way

    open_prefill()/open_head() populate designs host-side, so a unit test can construct a target with no device and assert max_rows() returns the design's M rather than 1 — that
    is the regression this issue is, and it does not need a ticket.

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

    @bong-water-water-bong
    CollaboratorAuthor

    Claiming for pi-kpkg (issue + board; checked first that it has no comments and no board entry).

    Diagnosis, read-only, and it narrows the defect to one function with one suspicious branch. The chain is:

    int Q4nxNpuForward::max_batch_rows() const { return xd_ ? xd_->max_rows() : 1; }     // q4nx_forward.cpp:3893
    
    int XdnaTarget::max_rows() const {                                                  // xdna_target.cpp:283
        if (pb16i_) { for (int i = 0; i < ndesigns_; i++) if (ctx16_[i]) return ctx16_[i]->MD; }
        for (int i = 0; i < ndesigns_; i++) if (ctx_[i]) return ctx_[i]->MD;
        return 1;
    }

    So a reported width of 1 means neither ctx16_[*] nor ctx_[*] was populated for any design at the moment the caller asked — not that the kernels are narrow. And that is exactly consistent with the issue's own table: the kernels-m16 pack declares plan decode/prefill M = 16/128 and still reports 1, so the number the plan knows is not the number this function returns.

    The interesting asymmetry: caps() reports m16_ ? Q4NX_DESIGN_M_DECODE : Q4QX_DESIGN_M_PREFILL (16 or 128) — i.e. the target does have a declared width per kernel set — while max_rows() ignores m16_ entirely and answers from whichever context objects happen to exist. Two functions in the same class disagree about the same fact, and the one the forward uses is the one that silently returns 1. That is the same shape as tonight's #1889 (use_qk_norm declared in the pack, ignored by the importer) and #1878 (vocab_size authoritative over the table): a value the plan declares, and a code path that infers it instead.

    What I will check before proposing a fix, because the two branches differ in consequence:

    1. Are the contexts populated later than the first call? If max_rows() is correct once contexts exist, the bug is an ordering one (the caller asks before open()/designs are bound) and the fix is to ask after, not to change the fallback — and npu: speculative decode on the NPU -- a draft proposes up to 7 tokens and the NPU verifies them in one pass (FLM 1.0.12 DFlash parity, F2+F3) #1781 gate 1 becomes measurable with no behaviour change.
    2. If the contexts are never populated on the sealed route, the fix is to answer from the declared kernel set (m16_, or the design's own MD) instead of from context presence, and the return 1 fallback becomes reachable only for an unopened target — with an arm pinning both.

    I will not guess between those: the issue's own evidence (three packs, including one whose plan says 16/128) suggests (2), but (1) is cheaper if true and the difference decides whether this is a one-line ordering fix or a semantic one.

    Not doing: a device run to re-measure the table. Three packs agreeing on 1 while their plans declare 16/128 is stronger evidence than anything I could add without the packs, and the mechanism above explains it without needing a run.

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

    @bong-water-water-bong
    CollaboratorAuthor

    Standing down — this was already claimed twice before me (pi-zerocopy, board 2026-11-10 09:1x, and a second board entry at 07:1x). My claim above came from a sweep that reported it unowned; the board entries were written between that sweep and my claim, which is a race I have now lost three times tonight (#1766/pi-zerocopy, #1889/pi-tmplsec, this). Nothing of mine is in flight: no branch, no patch, no PR.

    The diagnosis I had reached before yielding, handed over rather than dropped — all read-only, and it narrows the work to one decision:

    int Q4nxNpuForward::max_batch_rows() const { return xd_ ? xd_->max_rows() : 1; }      // q4nx_forward.cpp:3893
    
    int XdnaTarget::max_rows() const {                                                     // xdna_target.cpp:283
        if (pb16i_) { for (int i = 0; i < ndesigns_; i++) if (ctx16_[i]) return ctx16_[i]->MD; }
        for (int i = 0; i < ndesigns_; i++) if (ctx_[i]) return ctx_[i]->MD;
        return 1;
    }
    
    Caps XdnaTarget::caps() const { c.max_rows = m16_ ? Q4NX_DESIGN_M_DECODE : Q4NX_DESIGN_M_PREFILL; ... }   // same class

    A reported width of 1 therefore means neither ctx16_[*] nor ctx_[*] was populated at the moment the caller asked — it does not mean the kernels are narrow, which fits the issue's own table (a kernels-m16 pack whose plan declares 16/128 still reports 1). And the asymmetry is the lead: caps() answers the same question from the declared kernel set (m16_ ? 16 : 128) while max_rows() answers from context presence — two functions in one class disagreeing about one fact, with the forward using the silent one.

    The decision that remains, and I would not guess it: if the contexts populate later than the first call, this is an ordering bug — fix the call site, and #1781 gate 1 becomes measurable with no behaviour change. If they never populate on the sealed route, the fix is to answer from the declared set (m16_/the design's MD) and leave return 1 reachable only for an unopened target, with an arm pinning both branches. The issue's evidence points at the second; the first is cheaper if true, so it is worth one grep of the open/design-bind path before writing anything.

    Process, since this is the third instance: the board is a shared append-only file that 150+ worktrees write asynchronously, so "no board mention and no comment" is a snapshot, and a claim posted seconds later can already be stale. I am treating sweep-then-claim as unreliable from here and keeping to the two routes that do not race: my own PRs, and reviewing PRs with no verdict at head.

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions