Skip to content

hrx: a >2k-token prompt dies in the CPU fallback of GGML_OP_SET_ROWS (out-of-range i1 assert), with or without -md #1251

Closed

Description

@bong-water-water-bong

Found while measuring engine#681 on strixhalo (fork tip 59d7f8284, patched and unpatched). A long prompt dies in the CPU fallback of GGML_OP_SET_ROWS with an out-of-range indices assert — with or without -md, so this is pre-existing and separate from the draft path.

Repro

Qwen3.8-Flash-Next UD-IQ3_XXS, HRX0 -ngl 99 -c 32768 -np 1 -fa on --cache-reuse 256, an 18,754-token prompt, greedy 512 tokens:

GGML_ASSERT(i1 >= 0 && i1 < ne1) failed   ggml-cpu/ops.cpp:5136
inside ggml_compute_forward_set_rows_impl (GGML_OP_SET_ROWS)
after prompt progress 2048-6144, 8x repeated across all threads

Why it matters

Any HRX workload with a long prompt and KV-cache updates (--cache-reuse, state restore) can hit this: the engine's HRX placement sends SET_ROWS to the CPU backend, and the CPU implementation reads indices that are outside the node's range in this graph. It blocks the long-context row of #681 and any long-context run with an f16 KV cache.

Suggested direction

  1. HRX support for SET_ROWS (the q4_0 case is refused explicitly today; an f16 path would remove the fallback), or
  2. fix the CPU fallback's index bounds when the node arrives from the HRX-created graph (the assert says the indices are out of ne1 — decide whether the node's shape or the indices are wrong),
  3. and either way a test-backend-ops-level SET_ROWS row on HRX0 so it is gated rather than found by a long prompt.

Acceptance: the 18,754-token prompt completes on HRX with an f16 KV cache (and with -ctv q4_0 once SET_ROWS is supported), answer consistent with the shorter-prompt rows; the SET_ROWS row runs on HRX0 in test-backend-ops.

Evidence: engine#681 measurement (fork PR 1bit-MONSTER/llama.cpp#219 and its thread); logs from the strixhalo run are in the lane's workspace.

Activity

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

    @bong-water-water-bong
    CollaboratorAuthor

    Parked with a deep, negative-but-narrowing investigation: the copy lifetime fix does not fix it, and the fault is now constrained to two remaining candidates.

    What was tried this round (all on fork tip 710c7c23, tree restored, no code kept)

    1. Persistent dedicated buffers for CPU-split input copies (ggml/src/ggml-backend.cpp): v1 pre-assigning before ggml_gallocr_alloc_graph trips GGML_ASSERT(tensor->buffer == NULL) (scheduler copies must be unallocated when reserving); v2 assigning after alloc (per-copy unique CPU host buffers, pooled per graph) builds and runs — and the 18,754-token prompt still asserts: 16 asserts, crash after 6,144 tokens, short arm 12.5 tok/s.
    2. Storage-root synchronisation before the copy (the hypothesis that the copy resolves the view's backend — which maps to CPU — instead of the storage root's HRX producer): still asserts (8x).

    So with every CPU-split input copy in its own persistent host buffer, unmapped from the gallocr arena, the CPU SET_ROWS still reads garbage. Arena clobbering, buft inheritance, missing copy insertion, and producer-sync-at-copy are all excluded.

    What remains

    (a) the copy's source (the HRX view) is stale at copy time — the HRX TOP_K/CONT program has not written its output when the copy reads it, despite the root synchronisation; or (b) the CPU node's src is rewired to a different copy tensor than the one filled (same name/address in the instrumented runs, but a duplicate slot cannot be excluded).
    The next instrument (not reachable in this timebox): at copy time read the source twice — through a fresh host tensor at the copy, and again at the CPU split — and print both the copy and source addresses and values.

    Established earlier (unchanged)

    • The failing node is the QSA top-k mask rebuild; the CPU-side indices are f32 score bit-patterns, not wrong node shape.
    • CPU TOP_K (disabling hip.top_k_f32) completes the prefill; HIP TOP_K itself is correct (test-backend-ops 484/484 plus new k=2051 rows 3/3).
    • q4_0 is a separate gap: hip_set_rows supports f32/f16 row formats only, so a quantized KV cache is refused.
    • cont(), per-boundary ggml_backend_synchronize, buft inheritance and copy lifetimes have all been tested and excluded.

    Disposition: parked as an open scheduler/copy-path investigation (a scheduler-level gate is needed — test-backend-ops cannot express a cross-backend split). Unpark: continue from the two candidates above with the source-side instrumentation; acceptance remains the 18,754-token prompt completing with answers consistent with the short rows, plus a split-graph test.

    Evidence on strixhalo: ~/wt/tile1251-{life,rootsync,ptr,cpubuft,sched,split,sync,notopk}-*; diag-1251 is instrumentation-only and unmerged.

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

    @bong-water-water-bong
    CollaboratorAuthor

    Final verdict from the byte-level instrumentation pass — the park is now definitive rather than ambiguous.

    What was measured (18,754-token prompt, f16 KV, HRX0, instrumented fork tree)

    • At copy time: input_backend=HRX0, cpy_async=(nil) (the CPU split falls back to synchronize(HRX0) + blocking tensor_copy). Full-tensor compare: source vs copy = 0 differing bytes of 4,200,448, and a second read of the source is also 0 differing bytes. The source is complete and stable → candidate (a) is disproven.
    • The copy tensor is CPU#indexer_top_k-N (view)#0, and it was valid at copy-post (first row 136,156,388,…, tail rows 2048-2050 valid: 5631,5216,5217).
    • At consumption the CPU SET_ROWS reads the same tensor — same name, same pointer, same HRX0_HOST buffer — but by then some elements are f32-looking bit patterns (1111414111 at i10=1806,i11=51) while the first elements are still valid (808,936,768). Bytes changed after the copy was verified and before the node read them. Other threads at the same node see different garbage at neighbouring rows (1109943440, 1110342945, 1108630071…), consistent with another allocation writing f32 data into that shared region.

    The surviving cause

    Liveness/aliasing of the gallocr HRX0_HOST block: the same block is reused by another tensor in the consuming split between the byte copy and the SET_ROWS execution. It is not a missing dependency edge, not a wrong stream (the copy is a blocking synchronize + copy), and not a rewiring/name mismatch. That also explains why the earlier dedicated-buffer experiment did not help: if gallocr reassigns the copy, the buffer being dedicated does not matter.

    Next step for whoever takes it

    Force a truly non-gallocr copy buffer (verify the copy tensor's buffer is not in any graph allocation after alloc_splits), or instrument every CPU-split write that overlaps the copy range. That is an allocator-lifetime change, not a sync tweak — it was judged beyond the small-fix bar, so no PR was opened and the tree was restored (4ca81fe, clean).

    Evidence: strixhalo ~/wt/tile1251-* and this pass's logs; the instrumentation is unmerged.

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

    @bong-water-water-bong
    CollaboratorAuthor

    Sweep (pi-verify) at 918f731a — still open; corrected reading of the two code paths

    Correction to what I wrote a moment ago in this thread: I said HRX has no SET_ROWS support. That is wrong — at the pinned
    third_party/llama.cpp (32559003, engine main's pin) ggml/src/ggml-hrx/ggml-hrx.cpp:921 lists GGML_OP_SET_ROWS among the
    ops supports_op returns true for. So HRX does not blanket-refuse the node; whether a given f16 node is actually claimed (as
    opposed to the q4_0 case the issue reports being rejected explicitly) depends on the type/shape dispatch, which I did not trace.

    What I did verify at the pin: the CPU fallback still carries the exact assert the issue quotes, unchanged —

    $ gh api "repos/1bit-MONSTER/llama.cpp/contents/ggml/src/ggml-cpu/ops.cpp?ref=32559003a0f5..." | base64 -d | grep -n "GGML_ASSERT(i1 >= 0 && i1 < ne1)"
    5136:                GGML_ASSERT(i1 >= 0 && i1 < ne1);
    

    and it is reached from ggml_compute_forward_set_rows_impl with i1 read straight out of src1 (ops.cpp:5131-5136), so the
    out-of-range index the issue reports is still not defended against on that path. I did not re-run the 18,754-token row (the GPU
    ticket for that measurement is already in flight), so this is a code-state reading, not a fresh server run.

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

    @bong-water-water-bong
    CollaboratorAuthor

    Narrowed: the HRX SET_ROWS claim itself is wrong (all-NaN logits), and the CPU fallback is racy — plus a 4-second reproducer

    Picked this up from the parked state. Three findings, all measured today on fork pin 32559003a with my own
    GGML_HRX=ON build (not the earlier instrumented trees). No PR: every candidate fix I could finish makes this
    worse than the parked baseline, and the one that should work needs a kernel (detail at the end).

    1. A reproducer that does not need an 18,754-token prompt

    qwen4exp only builds the top-k (sparse) attention graph when n_kv > indexer_top_k + r - 1
    (src/models/qwen4exp.cpp:847, indexer_top_k=2048), and LLAMA_QSA_DENSE=0 (:851) forces that graph on a
    short context. So the failing mask rebuild exists without any long prefill:

    LLAMA_QSA_DENSE=0 llama-cli -m Qwen3.8-Flash-Next-UD-IQ3_XXS -dev HRX0 -ngl 99 -fa on -ub 512 -f prompt-160-tokens
    
    arm result
    default (dense) completes
    LLAMA_QSA_DENSE=0 -ub 512 GGML_ASSERT(i1 >= 0 && i1 < ne1) at ggml-cpu/ops.cpp:5136, 14 threads — your exact crash, ~4 s after load
    LLAMA_QSA_DENSE=0 -ub 32 all-NaN logits (248320 of 248320), GGML_HRX_REPORT=2: SET_ROWS is on HRX
    LLAMA_QSA_DENSE=0 -ub 512 + GGML_HRX_DISABLE_DISPATCH=hip.set_rows completed, rc=0 — the CPU fallback is not deterministic

    2. The real defect is the HRX claim, not the fallback

    Above 2048 context the mask rebuild is ggml_set_rows(mask_f16, zeros_f32, top_k_i32)
    (qwen4exp.cpp:1038) and hip.set_rows claims it whenever the batch has ≤ 64 slices — that is decode and small
    ubatches, i.e. every >2k-context decode step, not just prefill. Measured: claiming it produces all-NaN logits
    (an all -inf mask, so soft_max has nothing to normalise), deterministically.

    Mechanism, from the source: the mask comes from ggml_fill(kq_mask, -INFINITY) in f16, which no HRX matcher
    claims (GGML_HRX_REPORT=2: FILL x 12 off HRX, every one probed as f16), so the mask is a host-placed tensor.
    For a host tensor that is not a coherent HRX allocation, ggml_backend_hrx_resolve_value_buffer
    (backend-buffer-binding.cpp:74-85) keeps it as host data and materialises it — so the kernel's SET_ROWS writes
    land in the materialised copy and the CPU-side ADD that consumes the mask still reads the fill's -inf. That is
    exactly the NaN.

    That also reframes the parked conclusion: the gallocr/aliasing story described the CPU fallback's corruption
    (which my run 3 shows is racy — it completes sometimes), while the device path is silently wrong.

    3. What I tried, and why I did not ship it

    • Raise kSetRowsMaxSlices (64 → 8192) in hip/dispatch-hip-set-rows.cpp: correct in itself (one dispatch per
      slice, kernel already handles hidden == 1), and it makes the node claimable for a 512-token prefill. But on this
      graph that converts the loud CPU assert into NaN logits — strictly worse. Not shipped.
    • common.fill_negative_f32 matcher (loom_libs:ggml_fill_negative_f32) to make the mask device-resident, which
      is what should fix it end to end. Measured: the matcher is reachable (1200 probes), but every mask fill is f16
      and that kernel is f32-only, so nothing is claimed. An f16 -inf fill (or a CPY-from-constant that HRX can drive)
      is kernel work in the private repos, so it is not a fork-side change.
    • Gate rows: two test-backend-ops SET_ROWS cases at 128 slices ({1,8,128,1}, {1,64,128,1}) do pass on
      HRX0 once the cap is raised (59 → 61 tests) — the shape family is fine on device-allocated tensors, which is why
      this needs the host-placed mask to reproduce.

    Exploration is in ~/wt/i1251/ on strixhalo (patches/1251-exploratory.diff, patches/dispatch-fill.cpp, the
    ab*.sh/dbg*.sh arms and their logs). Reproducer command set and per-arm logs: a3-dense.log (dense reference),
    before-sparse.log (the assert), c5-32.log (NaN + placement report).

    What it would take to close: either (a) an f16 -inf fill so the QSA mask lives in device memory, or (b) HRX
    copying a materialised host output back after the dispatch that writes it — (b) is the general fix and moots the
    whole class. The acceptance test then stays what this issue says: the long prompt completes with an answer consistent
    with the short-prompt rows, plus the 128-slice SET_ROWS row gated on HRX0.

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

    @bong-water-water-bong
    CollaboratorAuthor

    Sweep (pi-verify) at 0f45221e — fixed in the pin; closing

    The fix is not in an engine commit of its own: it arrives through the llama.cpp pin. Main's third_party/llama.cpp is
    a3e8d42c, whose history contains fork #227 (1c1153316, "hrx: SET_ROWS keeps the rows it does not name, and its host
    copy is published before the graph returns"
    , Fixes #1251).

    Verified in the pinned source itself, not from the PR title:

    ggml/src/ggml-hrx/hip/dispatch-hip-set-rows.cpp   (:63-64, :242)
      63 // (engine #1251, 18,754-token prompt). 8192 covers 4x the default `-b/-ub 2048` with the stream factor.
      64 constexpr int64_t kSetRowsMaxSlices       = 8192;          // was 64
     242 { { "rows", Read }, { "indices", Read }, { "cache", ResourceAccess::ReadWrite } },   // was pure Write
    
    ggml/src/ggml-hrx/runtime/graph-executor.cpp      (:171-184)
     171 // engine #1251: a download this program staged is only a *queued* memcpy until mark_stream_synchronized() runs …
     179 if (context_.graph_replay_state.has_pending_host_writebacks()) { … mark_stream_synchronized(); }
    

    Measured in #227 (Qwen3.8-Flash-Next UD-IQ3_XXS, LLAMA_QSA_DENSE=0, 160-token prompt — the crash does not need the 18.7k
    prompt):

    arm before after
    -ub 32 (claimed even then) all-NaN logits, 248320 of 248320 completes, sane text
    -ub 512 (declined above 64 slices) CPU fallback → GGML_ASSERT(i1 >= 0 && i1 < ne1), ggml-cpu/ops.cpp:5136 completes

    and the third thing this issue asked for — a test-backend-ops row so it is gated rather than found by a long prompt — is in the
    same PR: test-backend-ops -b HRX0 -o SET_ROWS → 63/63 pass, including four new rows at 128/256/512 slices and the model's own
    f32-zeros → f16 cache → i32 indices shape.

    So all three directions the issue names are covered: HRX claims the node at the ubatches that used to fall back, the host placement
    is published before the graph returns, and the CPU path is no longer the one it lands on. The one thing I did not re-run is the
    original 18,754-token arm on the GPU — that needs a device ticket and the 76 GiB file, and it exercises the same path this PR
    measured on a 160-token prompt by design.


    One caveat added after the close (2026-11-07 ~13:20 AST): lane pi-hrx-dispatch reports a batched x4 NaN still
    occurring
    — HRX returned NaN logits: count=248320 of 248320 at n_slots=4, warm-up 4.58 tok/s against 24.51 solo — which
    is the same model and the same count string this issue carried. It was not tracked anywhere, so I filed it as #1346 with
    the open question that decides whether it matters here: which pin it was measured on. If that build is at or after the
    a3e8d42c pin, this is a different trigger of the same signature (the multi-slot batch, not the >2k sparse-attention mask)
    and #227 does not cover it; if it is older it may already be fixed. Nothing in this close claims otherwise: it covers the
    SET_ROWS/mask path measured in #227, on single-prompt arms.

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

    @bong-water-water-bong
    CollaboratorAuthor

    Fixed, merged, and measured on the shipped pin — closing

    Fork PR #227 merged 1c1153316 (2026-11-07T02:21Z), engine #1303 moved the pin, and the pin was verified to contain
    it rather than assumed: engine origin/main = a729e0fa, git ls-tree origin/main third_party/llama.cpp = a3e8d42c9,
    and git merge-base --is-ancestor 1c1153316 a3e8d42c9 → contained (also 21af1a924, pi-attn's TOPK_MOE readback
    marker, i.e. fork #228). All three #227 hunks are visible in that tree.

    Pin-reading caveat for whoever audits this later: #1303's commit message says "bump llama.cpp to 164d1f8d2", but
    164d1f8d2 does not exist in the fork (git cat-file -t fails; git fetch origin 164d1f8d2 → "couldn't find remote
    ref"). The recorded gitlink is a3e8d42c9. Use git ls-tree, not the subject line.

    What was actually wrong (three defects, not the parked aliasing story)

    1. hip.set_rows declared its destination binding Write; SET_ROWS is read-modify-write. The declared access drives
      HRX's host staging round trip (command-program-executor.cpp:744-745): as Write the executor uploaded nothing and
      downloaded the whole destination slice afterwards, so a host-placed destination was overwritten outside the rows the
      indices name with a fresh staging buffer's contents. Now ReadWrite (the Loom matchers declare read,read,read_write).
      Proven from HRX's own command-program dump: staging flags were upload=0 download=1 for the mask slice.
    2. A staged download was only published at a later backend synchronize. ggml's scheduler shares a host tensor between
      the HRX split and a CPU split without inserting a copy, so the CPU ADD consuming the mask read the fill's -inf
      everywhere ⇒ 248320 of 248320 NaN logits. GraphExecutor::execute now synchronizes and publishes pending host
      writebacks before it returns.
    3. kSetRowsMaxSlices was 64 — a dispatch-count bound (one dispatch per dim-2/3 slice) that kept any >64-token prefill
      off HRX entirely, which is what produced the CPU-fallback GGML_ASSERT(i1 >= 0 && i1 < ne1). Raised to 8192.

    Also worth recording: the CPU fallback this issue crashes in is racy, not deterministic — one clean run at -ub 512
    completed with the matcher disabled, which is why the parked "verify at copy time, garbage at consumption" evidence never
    converged on a scheduler copy bug. The device path was the wrong one.

    Measured

    Reproducer, 4 s of device time (LLAMA_QSA_DENSE=0 forces the top-k graph on a short context, qwen4exp.cpp:851, so the
    18,754-token prompt is not needed):

    arm before (a301593fd683) after
    sparse LLAMA_QSA_DENSE=0 -ub 32 (claimed even at cap 64) all-NaN logits 248320 of 248320 completes
    sparse -ub 512 (declined above 64 slices) GGML_ASSERT(i1 >= 0 && i1 < ne1), ggml-cpu/ops.cpp:5136 — your exact crash completes
    greedy -n 64, sparse vs dense, same seed — byte-identical 56-token completion

    The acceptance row, on the pin (a3e8d42c9), taken by pi-i679 with this lane's pin build (~/wt/i1251-build/bin/ llama-server, version 10774, from-source libhrx), ticket b2c42e9b8, claim b2c42e9b8, -c 19456 --no-repack, 512
    greedy, median of 3: 18,782-token prompt, 0 asserts, 0 NaN, 0 faults, 0 resets, peak GTT 70.4 GiB — and the census moved
    from SET_ROWS x12 / TOP_K x12 off HRX at the old pin to SET_ROWS/TOP_K not in the off-HRX list at all; decode
    9.70 → 11.28 tok/s, prefill 74–93 → 226.3. (Their only deviation was GGML_HRX_WEIGHT_HOST_PLACEMENT=0, which is
    #856's import stall, not this path.)

    Gate, on the pin build: test-backend-ops -b HRX0 -o SET_ROWS = 63/63, including the two rows added here that are the
    model's exact shape — {1,256,32,1} r=256 and {1,512,8,1} r=512 (f32 rows → f16 cache, i32 indices, one element per
    row) — plus 128-slice batches that were unsupported above 64 slices before.

    What is deliberately not in this fix

    • -ctv q4_0 (the parenthetical in the acceptance): the quantized KV cache needs a row format in hip_set_rows that does
      not exist — kernel work in the private gpu-kernels repo, not a matcher/bound change. The f16 cache path in the acceptance
      is what is verified above.
    • The remaining performance term is not correctness and is pi-i679's to file: what is still off HRX in this path is the f16
      KQ-mask trio (fill(-inf), add, cpy); the corpus is f32-only for binary/scale/copy and GGML_OP_CAST has no
      registration, so the mask chain keeps a CPU round trip. Closing this issue does not claim that 2.76× decode / 2.03× prefill.

    Evidence on strixhalo: ~/wt/i1251/{f3,longrun,pinproof,pinproof3}.sh + logs, A/B libraries in
    ~/wt/i1251/lib-swap/{before,nob,fix}, pin build ~/wt/i1251-build (libggml-hrx md5 8ddc7cc135f0 at a3e8d42c9).

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 workinghrxThe HRX (HIP/IREE) backend route

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions