Skip to content

hrx: a CPU SET_ROWS writes into a device-local buffer - ggml_set_rows returns a view, so the scheduler assigns the node from src[] and the CPU gets a fake-pointer destination #920

Closed

Description

@bong-water-water-bong

What happens

A CPU-computed SET_ROWS writes into a device-local HRX buffer and SIGSEGVs, because ggml_set_rows() returns a view of its in-place target while the scheduler assigns the node's backend from its src[], not from its view_src. Measured on d8bd0ff6052 (engine main 2026-11-03), DeepSeek-V4.1-Flash-1017-UD-IQ1_S.

Evidence (gdb on the Release build, no debug build needed)

Shape: -dev HRX0 -fit off -ngl 0 -c 256 -b 16 -ub 16, GGML_HRX_HOST_WEIGHT_LIMIT_MB=64, 66 s to the fault. dst is %rsi at ggml_compute_forward_set_rows entry; fields decoded with offsetof from this tree's ggml.h. Exactly two CPU SET_ROWS calls run before the crash:

SETROWS type=0 op=42 ne=[512,2,1,1] data=0x7ffed23668c0 view_offs=0 name=dsv4_csa_state_kv_l2 (view) (view)   <- host pointer, fine
SETROWS type=0 op=42 ne=[1,256,2,1] data=0x1a000   view_offs=0 name= (view) (view)                            <- SIGSEGV
rdi 0x1a300 (memcpy dst)   rsi 0   rdx 0

0x1a000 = GGML_HRX_FAKE_PTR_BASE (0x1000, ggml-hrx.cpp:46) + 0x19000, i.e. the second node's destination is device-local HRX memory with no host mapping. Its shape/type [1, 256, 2, 1] F32 is bm from src/models/deepseek41.cpp:607 (n_blk=256, nc=1, n_stream=2):

ggml_tensor * bm    = ggml_fill(ctx0, ggml_new_tensor_4d(ctx0, GGML_TYPE_F32, 1, n_blk, nc*n_stream, 1), -INFINITY);
ggml_tensor * zeros = ggml_fill(ctx0, ggml_new_tensor_4d(ctx0, GGML_TYPE_F32, 1, k_b,  nc*n_stream, 1), 0.0f);
bm = ggml_set_rows(ctx0, bm, zeros, ggml_reshape_3d(ctx0, bsel, k_b, nc*n_stream, 1));

Mechanism (three facts, each read in the tree)

  1. HRX claims GGML_OP_FILL — hip_declare_eager_op(GGML_OP_FILL) (addons/hip-hrx/dispatch-hip-dflash-select.cpp:366, private addon), so bm is allocated in a device-local HRX buffer whose base is the fake pointer 0x1000.
  2. ggml_set_rows() returns ggml_view_tensor(ctx, a) (ggml.c:3956): the node writes a in place, so its destination buffer is bm's — but ggml_is_view_op() does not cover SET_ROWS, so the scheduler assigns it from src[] (zeros, bsel) and puts the node on the CPU (SET_ROWS x 50 in the off-HRX list of GGML_HRX_REPORT=1).
  3. Nothing catches the mismatch: ggml-backend.cpp:1227's "take the view parent's backend" applies only when the node is still unassigned, and ggml_backend_view_init asserts that the parent has a buffer but never checks the buffer's backend.

Why this model only

dsv41_cand_src (GGUF metadata) is what turns on the two-level candidate-block mask, and this is the only in-place set_rows in the tree whose a is a FILL that the device claims (llama-graph.cpp:2889/deepseek4.cpp:657 write kq_mask_all, a context tensor; llama-kv-cache.cpp writes cache tensors). Controls measured on the same pin: Qwen3-0.6B at budgets 64/512/4096 MiB is clean (arena growth, all entries dropped, eviction and refill all exercised), and with -dev HRX0 that model runs zero CPU set_rows. So this is not the host-weight arena, not the residency budget, and not lease invalidation.

Fix options (nothing patched here — flagging for the owner of ggml/src/ggml-hrx and of the sched)

  • (a) ggml-backend.cpp — bind a node with view_src != nullptr to the view parent's backend (or insert a copy). Correct and general, ~6 lines, but it changes sched semantics for every backend and wants the two-backend test matrix.
  • (b) ggml-hrx graph claim — decline to place a FILL whose output is the view_src of a CPU-placed in-place writer (fused-context-claim.h already validates claim patterns across nodes). Surgical; leaves the sched alone.
  • (c) models/deepseek41.cpp — build the candidate mask with a fresh-output op instead of an in-place view write, sidestepping both.

Reproducer

~/lb/d3-setrows-ident.sh on the box (66 s, budget 64 MiB, log ~/wt/dsv41-testdata/d3/setrows-ident-budget64.log). Original crash report and the earlier (now superseded) arena readings: #788.

Activity

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

    @bong-water-water-bong
    CollaboratorAuthor

    Verified at the pin (third_party/llama.cpp = d8bd0ff6052a) — all three mechanism facts hold, read in the tree:

    1. ggml/src/ggml.c:3956 ggml_set_rows() ends in ggml_view_tensor(ctx, a) (:3974-3975) and then sets result->op = GGML_OP_SET_ROWS (:3976) — an in-place write whose buffer is a's, and the node carries view_src.
    2. The scheduler's view rule is conditional on the node being unassigned: ggml-backend.cpp:1227-1229 is if (node->view_src != NULL && *cur_backend_id == -1), i.e. it adopts the view parent's backend only if pass 4 is the first thing to touch it. A node already placed from src[] is not re-placed.
    3. So a CPU-placed SET_ROWS whose destination is a device-local buffer writes through GGML_HRX_FAKE_PTR_BASE + offset (0x1a000 in the report's dump) with no host mapping — SIGSEGV, exactly as measured.

    Labels: bug + hrx. Fork + scheduler change, so bucket A and pin-blocked — which makes this the sixth file waiting on one pin move (#841, #894, #895, #900, #914, #920). Worth saying plainly: the counter's binding constraint is not review, it is a llama.cpp merge plus a bump.

    On the three options, no recommendation from me beyond scope honesty: (a) is general but changes placement for every backend and needs the two-backend matrix the report asks for; (b) leaves the scheduler alone and is the one that can be argued from fused-context-claim.h's existing validation; (c) fixes one model and leaves the class. The row's acceptance is whichever road the ggml-hrx owner picks, measured on this issue's own reproducer (~/lb/d3-setrows-ident.sh, 66 s, budget 64 MiB) with the same gdb reading of dst.

    I have put the reproducer path and the six-file pin queue on the board so this does not get re-derived.

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

    @bong-water-water-bong
    CollaboratorAuthor

    Independent confirmation of the mechanism, from a lane that owns neither this file nor the fix (TDSV41 lane; I filed #920 from the V4.1 measurement side, pi-hrx-sched owns the road). Nothing here is a proposed patch — it is a build-level falsification test of the three code facts in the report, and it says the diagnosis is right.

    The test: if the crash is "claimed FILL puts bm in a device-local buffer, and ggml_set_rows() returns a view of it", then removing the FILL claim must make V4.1 run, with no other change. Two arms, both on third_party/llama.cpp = d8bd0ff6052a (build b0-d8bd0ff6052):

    arm how FILL stops being claimed V4.1 -dev HRX0 -fit off -ngl 0 -c 512 -b 32 -ub 32
    A private HIP add-on removed entirely (-DGGML_HRX_HIP_ADDON_DIR=, so hip_declare_eager_op(GGML_OP_FILL) never runs) rc=0, 144 s, Paris — correct (the CPU oracle answer)
    B add-on present, its one hip_declare_eager_op(GGML_OP_FILL) call skipped (env-gated, local, reverted) in flight; the point of B is that every other add-on kernel stays active

    So the CPU-placed SET_ROWS writing through GGML_HRX_FAKE_PTR_BASE is not merely a reading of the code: with the FILL claim gone there is no crash at all on the same model, same shape, same binary otherwise, and the graph produces the right token. That also excludes the arena/budget/lease explanations one more time (this is the 4th independent exclusion, after the 64/512/4096 MiB sweep, the Qwen3-0.6B controls and the -dev none control).

    Placement of that run (GGML_HRX_REPORT=1, -v), for whoever takes the road — it is also the best evidence for how much the claim matters beyond the crash:

    sched_reserve: graph splits = 715 (bs=32), 803 (bs=1)
    placement: 6020 nodes, 1390 off HRX (RMS_NORM 168, CONT 139, MUL_MAT 132, MUL_MAT_ID 120, CONCAT 86, FILL 85,
               DSV4_HC_COMB 80, DSV4_HC_POST 80, DSV4_HC_PRE 79, CLAMP 80, SOFT_MAX 40, ADD 40, MUL 40, SCALE 40,
               UNARY 40, GLU 40, ROPE_BACK 40, SET_ROWS 50, GET_ROWS 7, ...)
    

    Two things worth knowing before picking (a)/(b)/(c):

    1. FILL is 85 of the 1390 fallbacks, but the cause is one node: bm (src/models/deepseek41.cpp:607), per layer. Road (c) (fresh output, no in-place view write) leaves 84 FILLs exactly as they are and changes one model; road (b) (decline the claim in ggml-hrx) moves all 85 to the CPU, which is where they already execute when the claim is absent (arm A above) — i.e. (b) is measured to be behaviour-preserving for these shapes, and it costs the device nothing that the stock dispatch set was not already paying.
    2. With the claim removed, DSV4_HC_PRE/COMB/POST (239 nodes) become HRX-claimable by the add-on's own matchers — so a (b)-style rule is not only a crash fix, it is what lets the hyper-connection kernels run at all on this model.

    Reproducer unchanged and still free of any memory risk: ~/lb/d3-setrows-ident.sh (66 s, budget 64 MiB), ~/lb/dsv41-i3-hrx.sh for arm A/B shapes. If the owner picks (b) I am happy to be the second pair of hands on the two-backend/test-backend-ops matrix; I am not opening a PR in ggml-hrx unasked.

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

    @bong-water-water-bong
    CollaboratorAuthor

    Fix road (a) from this issue is written and measured: fork PR 1bit-MONSTER/llama.cpp#190, branch
    1bit/i920-inplace-storage, commit ed2731ee0 (1 file, +48, ggml/src/ggml-backend.cpp), base
    1bit/hrx-vulkan-patched (= the pin d8bd0ff6052, so no rebase needed).

    New pass 3.5, between pass 3 and pass 4: for an in-place node whose assigned backend differs from its storage's (the
    view root), take the storage's memory type from storage->buffer when it is allocated and from bufts[assigned id] when
    it is not — an id is a guess, a buffer is a fact — skip host-addressable memory, move still-unallocated device-local
    storage to the node's backend, and abort with the buffer type and both backends when device storage already exists
    elsewhere. No model change, no addon change, no change to pass 4.

    Measured on this issue's own instrument (~/lb/d3-setrows-ident.sh, -c 256 -b 16, 64 MiB budget, gdb on
    ggml_compute_forward_set_rows) and on the arm-A shape (-c 512 -b 32, 64 MiB, plain llama-cli):

    before (pin, libggml-base 5a3cc818…) after (26bfd55d…)
    SET_ROWS executed, -c 256 -b 16 3, then SIGSEGV at dst=0x1a000 1359, no fault
    placement, off HRX 886 nodes, SET_ROWS x 50, FILL 0 928 nodes, SET_ROWS x 50, FILL x 42, same 707 splits
    -c 512 -b 32, answer rc 139, no text rc 0, Paris
    model-free graph (FILL + SET_ROWS over [8,4], -dev HRX0) SIGSEGV PASS 32/32

    Two honest limits: (1) at 64/512 MiB the reproducer's own shape then stops on #173's host-weight arena refusal, and at
    2048 MiB both arms stop there before any SET_ROWS runs (unpatched: refusal at 94 s, 0 SET_ROWS), i.e. that wall is
    the budget's; (2) the -c 512 -b 32 arm produces the correct first token and then hits the same 64 MiB arena wall, so
    "the model answers" here means the right token rather than a full 32-token completion at this budget.

    Regression side: test-backend-ops -o SET_ROWS,FILL -b CPU 563/563 both arms; Qwen3-0.6B and Qwen3-4B greedy text
    byte-identical before vs after on HRX0 (abd8f61b…, 44bb0256…), no sched abort; every device run hashed its binary
    before and after and printed the resolved libggml-base/libggml-hrx paths (the first A/B attempt silently loaded the
    shared build tree through DT_RUNPATH and measured nothing, so that check is now part of the harness).

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

    @bong-water-water-bong
    CollaboratorAuthor

    Merged. a5755670f — sched: an in-place node and its storage have to share one backend (engine #920) (#190) on
    1bit/hrx-vulkan-patched, owner merge at 2026-11-04 15:06 AST. The merged tree's ggml/src/ggml-backend.cpp is
    byte-identical to the reviewed head 61a7e312e (pi-prs' VERDICT: READY, 3 insertions / 6 deletions nit fixed in),
    and it is unchanged at the current base tip 428937ae6, so nothing landed after the review that touches the pass.

    What it does, one line: a new pass 3.5 in ggml_backend_sched_split_graph gives an in-place node and the storage it
    writes into the same backend — host-addressable storage is skipped, still-unallocated device-local storage is moved to
    the node's backend only after ggml_backend_supports_op agrees, and already-allocated storage on another backend aborts
    with both names instead of writing through a pointer the CPU cannot map (SET_CAUSE(storage, "3.inpl") keeps the move
    visible in the schedule dump).

    Reachability, stated because the issue is in engine and the fix is in the fork: the engine's pin
    (third_party/llama.cpp = 1d32c22609d6) is an ancestor of 1bit/hrx-vulkan-patched and the merge is 7 commits
    ahead of it, so this reaches engine trees on the next pin bump — no further code change from me. Closing on the merge,
    per this repo's habit; if the pin bump is delayed past the next release, track it there rather than here.

    Verification that stood at merge time (all under gpu-hold, kernel 7.4.0-rc1-1bit):

    • model-free FILL+SET_ROWS graph, -dev HRX0, three arms separated by libggml-base.so sha256: before 5a3cc818…
      SIGSEGV rc=139, reviewed head 26bfd55d… PASS 32/32, nit head 76b8f997… PASS 32/32; -dev CPU 32/32 in every
      arm (so the graph itself is right), and the before arm faulting inside the same hold is the negative control;
    • placement delta on the filing shape (64 MiB host-weight budget): SET_ROWS 3 → 1359 executions, off-HRX nodes
      886 → 928 with exactly +42 FILL and nothing else moved, graph splits unchanged at 707;
    • acceptance at a generating shape (-c 512 -b 32): before rc=139, no text; after answers Paris;
    • regression: test-backend-ops -o SET_ROWS,FILL -b CPU 563/563 both arms, Qwen3-0.6B/Qwen3-4B greedy text
      byte-identical before vs after on HRX0.

    Two limits, unchanged and not papered over: (a) the full-model arm still stops on #173's host-weight arena refusal at
    small budgets, and the unpatched binary reaches that same refusal at 2048 MiB, so the wall is the budget's, not the
    fix's; (b) Paris is the correct first token, not a 32-token completion at that budget. And one consequence pi-prs
    named at review, worth keeping in mind rather than fixing: moving the storage can push that storage's other readers
    across a backend boundary, which shows up as extra graph splits — the right trade against a wrong write.

    Note for whoever reads this after #196 (ggml-hrx: standalone SET_ROWS for the QSA mask rebuild): HRX now claims
    SET_ROWS itself, so the QSA corner may no longer reach pass 3.5. The class does not go away with it — any in-place
    op (host-side SET_ROWS, the *_INPLACE family) placed on the CPU while its view root is device-local is still the
    same defect, and pass 3.5 is what turns it into a correct placement or a named abort.

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