Skip to content

HRX: decode-split multipass path intermittently faults (HSA_STATUS_ERROR_MEMORY_FAULT) above capacity 2048 #123

Closed

Description

@1bit-traffic-bot

The multi-pass KV-block reduction added by 1bit-MONSTER/llama.cpp#13 (branch feat/hrx-decode-split-multipass, commit f59f6d37a on fa1a4563) makes flash_attention.decode_split work above key_value_token_capacity 2048, but a residual out-of-bounds access intermittently faults the decode instead of declining. That violates the HRX lane's constraint that a dispatch must decline and fall back rather than fail the decode.

Symptom

gfx1151 (Strix Halo), Qwen3-Coder-30B-A3B-Instruct-Q4_K_M, -dev HRX0, llama-bench -p 0 -n 8 -d <D>:

Warning: Queue error - HSA_STATUS_ERROR_MEMORY_FAULT
E graph_compute: ... AMDGPU memory access fault at device address 0x00007f508ea94000 (reason mask 0x00000001)
E llama_decode: failed to decode, ret = -3

5-run GPU-fault counts with the partial transients aligned to 4096:

depth capacity blocks faults
1900 1920 30 0/5
2000 2048 32 0/5
2100 2112 33 0/5
2500 2560 40 0/5
3000 3008 47 1/5
4800 4864 76 0/5

With the SAME code but the partial transients aligned to 256, d2100 is 5/5. GGML_HRX_DISABLE_DISPATCH=flash_attention_decode_split makes every faulting depth pass (0/5), so the fault is in the multipass path, not the fallback.

Excluded (each measured over 5 runs)

  • The reducer - no-op'ing reduce_completed.multipass (inserting template.return as its first statement) still faults 5/5, so it is not the reduction math.
  • The completion counter - replacing its atomic.reduce<addi> -N reset with a plain release-ordered store of 0 changes nothing; so does zero-initialising it via ConstantInitialization.
  • The exact-33 assume bound - relaxing every range(..., 33, 4096) to range(..., 1, 4096) changes nothing.
  • Static sizing - the dispatch's transient bytes are exactly 256 * blocks (partial_max/sum) and 16384 * blocks (partial_output), strictly linear, and the kernel's views match.
  • Every readable access is in bounds - the produce's channel stores (lane_output_base = lane * 4, lane < 32, so the channel is in {0,4,...,124} and a vector<4xf16> store keeps channel + 4 <= value_head_size = 128); the scalar max/sum row stores (guarded by query_head_valid); the reduce's output store (query_head = key_value_head * query_heads_per_key_value_head + query_row < 32, output_channel < value_head_size); the K/V loads (element-guarded by bounded_key_value_token_count); pack_completed_q8's output size (q8_1_x4_byte_count(1, output_hidden_size)).
  • Allocation - alignment 256 -> 4096 helps (d2100 5/5 -> 0/5); padding the transient SIZES makes it worse (d3000 5/5, d2100 4/5); 64K alignment is worse still; padding the block count to whole 64-block waves does not help.

Assessment

Every access is provably in bounds, yet the fault moves with the allocation layout and cannot be removed by adding slack. The best remaining explanation is a wrong address emitted by the JIT-specialised code for this capacity, which allocation cannot fix. Prime suspect: the multipass output pass (flash_attention_decode_split_f32_f16_wmma.loom ~911-920), the only O(blocks)-per-output-element code in the reduce.

Impact

1bit-MONSTER/llama.cpp#13 masks part of this by aligning the partial transients to 4096, but the fault is not eliminated and a decode can still fail - so this is not yet safe to land as-is.

Activity

  1. bong-water-water-bong commented on Sep 26, 2026

    @bong-water-water-bong
    Collaborator

    Possibly related: #140. The ≤2048 decode-split kernel (flash_attention_decode_split_next_q8) is nondeterministic run to run even at short context. Qwen3-0.6B greedy decode gave 2 distinct outputs in 5 identical runs; with GGML_HRX_DISABLE_DISPATCH=decode_split it gave 1. Here the fault survives a no-op reducer, so it isn't in the reduction math. If the two share a cause in the decode-split family, fixing the ≤2048 case first could make this one easier to pin down.

  2. bong-water-water-bong commented on Sep 26, 2026

    @bong-water-water-bong
    Collaborator

    Re-tested this while merging the pin bump (#151). llama.cpp#13's own report was 0/5 faults at depth 2100, 1/5 at depth 3000. Re-running llama-bench -dev HRX0 -p 0 -n 8 -d 2100 here faulted 4/4 times (once inside a combined -d 2100,3000 run, then 3 isolated repeats at 2100 alone) - HSA_STATUS_ERROR_MEMORY_FAULT, res=-3. Ruled out a broader GPU/driver issue: Vulkan0 ran clean immediately before and after, and HRX0 at depth 500 (below the multipass threshold) also ran clean, so this is isolated to the multipass path above 2048, same as originally diagnosed - just reproducing far more readily in this environment than in the original report. Worth knowing before prioritizing the fix: this may not be the rare edge case the 1/5-at-d3000 number suggested.

  3. bong-water-water-bong commented on Sep 26, 2026

    @bong-water-water-bong
    Collaborator

    Possibly related: #140 found the exact same last-workgroup-atomic-counter-then-barrier pattern (shared by every decode-split kernel in this family, including this one) produces measurably wrong results ~19% of the time even on the plain next_q8 short-context path, with no bounds violation possible there (small, fixed block count). That points at the kernel.barrier<global> scope(workgroup) ordering(release/acquire) pair not actually guaranteeing cross-workgroup write visibility on this hardware, rather than anything specific to either kernel's indexing math - which would fit this issue's own conclusion (every access provably in bounds, fault moves with allocation layout) better than a source-level bug does. Worth checking whether strengthening that barrier's scope changes either issue's reproduction rate.

  4. bong-water-water-bong commented on Sep 26, 2026

    @bong-water-water-bong
    Collaborator

    Update: built Loom's own compiler toolchain and directly inspected the barrier lowering (2026-09-26) - this narrows things down, but does not confirm the earlier barrier-visibility hypothesis; if anything it weakens it.

    Built ROCm/hrx-system from source (Python 3.12 + Bazel, dev.py bazel build //loom/src/loom/tools/loom-check ...) and used loom-check's emit source-low mode to compile a minimal standalone kernel reproducing the exact same synchronization idiom used by this kernel family (kernel.barrier<global> scope(workgroup) ordering(release) on the producer side, ordering(acquire) on the consumer side, around a view.atomic.rmw<addi> {ordering = acq_rel, scope = device} completion counter), targeting amdgpu:gfx11-generic.

    Actual emitted GCN/RDNA assembly:

    • Acquire barrier + subsequent global load: lowers to s_barrier (intra-workgroup only) + s_waitcnt {vmcnt=0} + a global_load_b32_saddr ... {glc=1} for the load. No explicit buffer_gl0_inv/buffer_gl1_inv cache-invalidate instructions - the compiler instead uses the GLC ("globally coherent") bit on the load itself to bypass the L0 vector cache. On RDNA this is a standard, documented way to get acquire semantics for a single load and is generally considered correct, not obviously buggy.
    • Release barrier + the atomic RMW itself: lowers to the data store, s_waitcnt {vmcnt=0} + s_waitcnt_vscnt {vscnt=0} (drains outstanding stores), s_barrier, then the buffer_atomic_add_u32_rtn (the counter increment), followed by s_waitcnt {vmcnt=0} and then explicit buffer_gl1_inv + buffer_gl0_inv - because that specific atomic op was declared acq_rel, not just release, so the compiler also emits real invalidate instructions there for the atomic's own acquire half.

    So the compiler clearly is capable of emitting real buffer_gl0_inv/buffer_gl1_inv invalidate instructions when it decides they're needed - it just didn't for the plain acquire-barrier-then-load case, using the cheaper GLC-bypass-load idiom instead. That pairing (writeback+wait on release, GLC-bypass load on acquire) is a standard, plausible-correct AMDGPU idiom, not an obvious hole. I can't fully rule out a subtler issue (e.g. whether GLC bypass on this specific gfx1151 iGPU's cache hierarchy - L0 per-CU, L1 per shader-array, L2 device-wide - really gives full device-scope visibility, vs. only bypassing L0) without lower-level hardware documentation this box doesn't have. But the "the barrier just doesn't provide real cross-workgroup visibility" theory, as stated, is not supported by what's actually emitted.

    Net effect: this rules out the simplest version of the shared hypothesis between this issue and [engine#140]. Root cause for both remains open. Repro scripts kept in case anyone wants to extend them (e.g. compiling the real multipass kernel through the full template/config pipeline and running the actual dynamic --sanitizer=race instrumentation on real hardware, which is a substantially bigger lift than this static check).

  5. bong-water-water-bong commented on Sep 26, 2026

    @bong-water-water-bong
    Collaborator

    Ran the actual empirical test on real hardware (2026-09-26) - built a fresh, correctly-pinned clone of 1bit-MONSTER/llama.cpp @ 8dd75eb48 (the exact commit that shipped the multipass fix), patched the flash-attention decode-split kernel's three reduce_fused variants (direct/cooperative/multipass) to force a genuine cache invalidate before the real partial-buffer reads - a dummy view.atomic.rmw<addi> with {ordering = acq_rel, scope = device} on the completion counter, gated to a single lane, followed by a kernel.barrier<workgroup> to propagate it - based on the confirmed compiler behavior that an acq_rel atomic emits real buffer_gl1_inv/buffer_gl0_inv instructions where a plain ordering(acquire) barrier-then-load does not (see the earlier comment on this issue).

    Important methodology note first: neither #123's fault nor #140's race reproduced at all on a clean, freshly-built unpatched binary under normal (low-contention) conditions - 0/54 combined attempts for #123, 0/300 for #140. Both only reproduced once I deliberately induced GPU contention with a second, concurrent llama-bench workload hammering the same HRX0 device in a loop. This matches this session's own earlier note that the divergence rate is contention-dependent, not a fixed per-request probability - worth remembering before trusting any "looks fixed" result from a quiet box.

    Under matched induced contention:

    • Unpatched baseline: 56/300 (18.7%) divergent logprobs - closely matching this issue's documented 58/300 (19.3%).
    • Patched (forced invalidate before the reads): 208/300 (69.3%), then 170/300 (56.7%) on a repeat run - both roughly 3-3.7x worse, not better.

    Conclusion: this rules out the missing-cache-invalidate hypothesis more decisively than the earlier static check did. Forcing a real, compiler-confirmed cache invalidate immediately before the reads made the race substantially more frequent, not less. The most likely explanation is that the extra synchronization changed wavefront scheduling/occupancy enough to widen whatever the actual race window is, rather than closing a coherence gap - meaning the real bug is probably a genuine scheduling/ordering hazard elsewhere in the reduction (or in how the producer side publishes its partials), not GPU cache visibility. I'm not going to guess further at the exact mechanism without stronger tooling (the dynamic --sanitizer=race instrumentation, still not attempted - would need the kernel's full template/config pipeline resolved against real device facts).

    Patch discarded (this was a diagnostic experiment only, not proposed for merging). Root cause remains open for both this issue and #123.

  6. bong-water-water-bong commented on Sep 26, 2026

    @bong-water-water-bong
    Collaborator

    Attempted the real dynamic race sanitizer against the actual kernel (2026-09-26) - blocked by two genuine upstream gaps, not by anything fixable from this repo.

    Found that AMD's ggml-hrx layer already has plumbing for Loom's compile-time sanitizer passes via GGML_HRX_LOOM_SANITIZER (env var, loom-jit.cpp) - but it only wires up none/access/asan/value/operation/ubsan/all, never race, even though the underlying loomc C API defines LOOMC_SANITIZER_CHECK_RACE and even a ready-made LOOMC_SANITIZER_CHECKS_TSAN_LIKE preset for exactly this. Patched ggml_hrx_loom_jit_parse_sanitizer_checks to add a "race"/"tsan" case (4 lines, following the existing pattern exactly), rebuilt, and set GGML_HRX_LOOM_SANITIZER=race (plus HRX_HAL_SANITIZER=tsan for the HAL-side runtime feature) against a real llama-server serving Qwen3-0.6B on HRX0.

    Two real compile-time failures surfaced immediately:

    1. Missing runtime-global config: generated kernels reference a data symbol iree_tsan_config that never gets emitted - ggml-hrx's ggml_hrx_loom_jit_runtime_globals() only knows how to set LOOMC_AMDGPU_RUNTIME_GLOBAL_ASAN_CONFIG for the ACCESS check; the public C header (loomc/target/amdgpu/emit.h) doesn't define an equivalent TSAN_CONFIG global at all. This is a real gap in the public runtime-globals surface, not something fixable with a local patch.
    2. The harder blocker: sanitizer-insert-race-observations cannot observe vector.fragment.load: local-memory vector operation requires lane-activity-aware race observation. Loom's race sanitizer pass cannot currently instrument WMMA fragment loads at all - and our flash-attention decode-split kernel uses exactly those (vector.fragment.load<lhs>/<rhs> for the P·V product). This affected several other WMMA-based kernels too (mul_mat_swiglu, mul_mat_q6_k_packed, mul_mat_add_f32_f32_wmma), breaking the whole forward pass when the sanitizer is enabled globally. Even with (1) fixed, the actual target kernel would still fail to compile with race instrumentation - this is an unimplemented feature in Loom's sanitizer pass itself.

    Conclusion: the dynamic race sanitizer genuinely cannot be run against this kernel family with the currently pinned hrx-system version. Confirming or ruling out the actual root cause of #123/#140 further needs either an upstream fix to Loom's sanitizer pass (fragment-load support) plus the missing runtime-global plumbing, or a fundamentally different diagnostic approach (e.g. RGP/ISA-level tooling this environment doesn't have). This session tried the static compiler-output check, an empirical barrier-strengthening patch (made the race 3-3.7x worse), and now the dynamic sanitizer (blocked) - all three avenues are now closed off or exhausted with the tooling actually available. The real bug is most likely a scheduling/ordering hazard, not a cache-coherence gap, given the barrier-strengthening result, but pinning it down exactly needs the upstream sanitizer support that doesn't exist yet.

  7. bong-water-water-bong commented on Sep 26, 2026

    @bong-water-water-bong
    Collaborator

    Correction to earlier comments on this issue: got the REAL production kernel to compile and disassembled it directly - the barrier lowering is NOT missing an invalidate after all.

    The earlier static check (comment above) tested a minimal synthetic standalone kernel reproducing the barrier idiom in isolation, and found the acquire barrier lowered to a GLC-bypass load with no explicit invalidate instruction. That result doesn't hold for the real kernel in its full context.

    Using ROCm/hrx-system's loom-compile directly with the actual production config values for this repro (Qwen3-0.6B: query_head_count=16, key_value_head_count=8, qk_head_size=128, value_head_size=128, key_value_token_capacity=2112 for a 2100-token decode - the exact #123 repro depth, landing in the multipass range), I compiled the real flash_attention_decode_split_f32_f16_wmma.loom kernel (via its actual kernel.def entry point, @ggml_flash_attention_decode_split_f32_f16_wmma_next_q8) to a real HSACO artifact, and disassembled it with llvm-objdump (from TheRock's ROCm SDK - roc-obj-extract/llvm-objdump are already present, no extra tooling needed).

    The real, in-context disassembly around the completion counter and the explicit acquire barrier:

    buffer_atomic_add_u32 v2, v1, s[20:23], s2 offen glc   ; completion counter increment (acq_rel)
    s_waitcnt vmcnt(0)
    buffer_gl1_inv
    buffer_gl0_inv                                          ; <- real invalidate, atomic's own acquire half
    ds_store_b32 v1, v2 offset:14592                        ; old_counter -> LDS scratch
    s_barrier                                                ; kernel.barrier<workgroup> (propagate via LDS)
    ds_load_b32 v1, v1 offset:14592                          ; old_counter <- LDS scratch
    v_cmp_eq_i32_e64 s6, v1, 32                              ; is_last_partition? (32 = 33 blocks - 1, matches our capacity)
    s_cbranch_scc0 ...                                       ; skip if not last
    s_barrier                                                ; <- this is the explicit kernel.barrier<global> ordering(acquire)
    s_waitcnt vmcnt(0)
    buffer_gl1_inv
    buffer_gl0_inv                                           ; <- a SECOND real invalidate, right here
    ; ... reduction over partial_max/partial_sum/partial_output begins immediately after ...
    

    Both invalidate pairs are present and correctly placed - one for the counter atomic's own acq_rel, and a distinct second one for the explicit kernel.barrier<global> ordering(acquire) that gates entry into the reduction. The real, deployed kernel already has correct, full L0/L1 cache invalidation before reading the other workgroups' partial buffers. This directly explains the earlier empirical result (posted above): forcing an additional redundant invalidate before the reduction made the race worse rather than better, because there was nothing missing to fix - the extra synchronization was pure overhead that perturbed scheduling without closing any real gap.

    This closes off the cache-coherence hypothesis for good, with ground-truth evidence rather than inference. The actual root cause of both #140's race and #123's fault is very unlikely to be a GPU memory-visibility problem. It's more likely a genuine data/algorithmic hazard - e.g. in how partial values get written by the producer side, an indexing issue that's timing-dependent, or something in the online-softmax combination itself. Confirming that precisely still needs either the (currently blocked - see above) dynamic race sanitizer, or manual disassembly analysis of the producer-side write path, which hasn't been attempted.

  8. bong-water-water-bong commented on Sep 26, 2026

    @bong-water-water-bong
    Collaborator

    Tested a second, independently-motivated hypothesis: allocator-introduced buffer aliasing between dispatches. Also refuted, with the same "any perturbation makes it worse" pattern as the barrier-strengthening test.

    ggml-hrx's transient buffer allocator (ggml/src/ggml-hrx/dispatch/transient-allocator.cpp, TransientAllocator::allocations_can_overlap) decides it's safe to reuse the same physical memory for two different dispatches' transient buffers using command-ordinal-based liveness intervals - a logical, program-order notion, not a guarantee about actual GPU execution-time overlap. This is structurally the same shape as an already-confirmed prior bug in this codebase ([[hrx0-fusion-overlap-bug]]: fused kernels racing on ggml-alloc-aliased buffers because there was no execution-order guarantee behind the reuse decision), so it seemed like a strong, well-motivated hypothesis for #140/#123 too.

    Patched allocations_can_overlap to unconditionally return false, forcing every transient buffer (partial_max, partial_sum, partial_output, completion_counter, etc.) to get its own unique, never-reused memory allocation - eliminating any possibility of allocator-introduced false aliasing between unrelated dispatches. Rebuilt and re-ran the same induced-contention #140 repro (concurrent llama-bench workload + 300-sample logprob probe) used for the earlier barrier-strengthening test.

    Result: divergence got dramatically worse, not better - 160/300 (53.3%) on the first run, 242/300 (80.7%) on a repeat, both far above the 56/300 (18.7%) unpatched-with-reuse-enabled baseline (itself matching the documented 58/300/19.3%). This refutes the buffer-aliasing hypothesis just as clearly as the earlier barrier-strengthening test refuted the cache-coherence hypothesis.

    The pattern across both negative experiments is itself informative: two structurally very different interventions - adding extra synchronization/cache-invalidation, and removing all buffer reuse - both made the race substantially worse rather than better. Neither closed a real correctness gap; both just added overhead/changed memory footprint under contention. Combined with the ground-truth disassembly showing the kernel's own synchronization is textbook-correct on both the release and acquire sides, this points away from both "the kernel's own barriers are wrong" and "the allocator is aliasing buffers unsafely," and toward something more like a genuine hardware/scheduling timing sensitivity specific to this GPU under contention (gfx1151 is a very new Strix Halo APU part; this stack has already turned up other real driver/firmware-level quirks in earlier work: XRT partial-buffer-sync staleness, a from-scratch HRX0 fusion-overlap bug, etc.) - not a straightforwardly fixable software logic bug in either of these two specific places.

    At this point three structurally different tooling approaches (ground-truth disassembly, empirical barrier-strengthening, empirical buffer-isolation) have converged on ruling out the two most plausible software-level hypotheses. Confirming an actual root cause from here would need either the dynamic race sanitizer (blocked upstream, see above) or hardware-level tooling (RGP, an ISA-level execution trace) this environment doesn't have.

  9. bong-water-water-bong commented on Sep 26, 2026

    @bong-water-water-bong
    Collaborator

    Real GPU execution trace (2026-09-26): found continuous, heavy KFD queue eviction/restore churn correlating exactly with the contention that triggers this race - the closest thing to a concrete mechanism this investigation has produced.

    Traced the server process directly under rocprofv3 (/opt/rocm-therock/bin/rocprofv3 --kernel-trace --kfd-queue-trace -- llama-server ...) during the same induced-contention #140 repro used throughout this investigation (a competing llama-bench workload on the same HRX0 device). Two notes on getting there: --attach <PID> (rocprofv3's other mode) requires the target binary to be built with rocprofiler-register attach support, which our build doesn't have - wrapping the launch directly (rocprofv3 -- <app>) works regardless. Also, the HIP/HSA API-level buffers (kernel_dispatch, hsa_api, hip_api) came back completely empty (0 records) - HRX/IREE's dispatch path doesn't go through the API layers rocprofv3 normally intercepts for those buffers, so kernel-level dispatch timing isn't directly observable this way. The KFD-level buffer, however, captured real, substantial data: 704 records over a 30-second capture, sustained at ~23 events/sec for the entire duration the competing workload was running (not a brief spike).

    Decoded via rocprofiler-sdk/kfd/kfd_id.h's enums:

    • 274x ROCPROFILER_KFD_EVENT_QUEUE_RESTORE_RESCHEDULED - "Queue was not restored; will be restored later." The GPU queue is repeatedly failing to restore on the first attempt and having to be rescheduled, continuously, for as long as the competing workload runs.
    • 131x EVICT_SVM and 299x EVICT_USERPTR duration events - the KFD driver evicting SVM-backed and userptr-backed GPU memory (this is what the transient buffers - partial_max/partial_sum/partial_output/completion_counter - are, functionally) under memory/queue pressure from the competing workload.

    Why this is a strong candidate mechanism, not just correlation: it directly explains contention-dependence (eviction pressure only exists when a competing workload creates it - exactly this issue's empirically-established trigger condition), and it sits entirely below the software layers already ruled out this investigation. The kernel's own buffer.gl0_inv/gl1_inv cache-invalidation sequence (confirmed correct via ground-truth disassembly, see above) invalidates caches - it says nothing about what happens if the underlying physical backing memory itself gets migrated/evicted and restored by the driver in the middle of the release/acquire window. If a queue eviction-restore cycle has any gap in how it re-establishes memory mappings or resyncs in-flight buffer state relative to a specific dispatch's execution, that would manifest as exactly the failure mode observed: wrong-but-plausible values, contention-dependent, present regardless of how correct the kernel's own synchronization is - and it also explains why both software-level "fixes" tried earlier (extra cache invalidation, disabling all transient buffer reuse) made things worse: both add more transient memory footprint/synchronization overhead, which would plausibly increase eviction pressure on an already-contended GPU rather than relieve it.

    This is not a confirmed root cause - it's real, decisive evidence of a genuine driver/kernel-level mechanism operating exactly when and where the race occurs, one level below anything checked so far. Confirming it fully would need correlating individual eviction-event timestamps against individual divergent-response timestamps (not attempted - would need request-level tracing correlated with the KFD event stream), or a from-scratch reproduction with queue eviction suppressed (e.g. via KFD sysfs/module parameters, if any exist, to disable eviction and see whether the race disappears entirely) - a natural next experiment for anyone with the time/access to pursue it further, but not attempted here given how much ground this session has already covered.

    This concludes the tooling avenues reasonably available in this environment: ground-truth disassembly (kernel sync is correct), two refuted software-level hypotheses (barrier strengthening, buffer isolation - both made things worse), a blocked dynamic sanitizer (Loom doesn't support instrumenting the WMMA ops this kernel uses), and now a real GPU-level trace pointing at driver-level queue eviction churn as the most concrete remaining lead.

  10. bong-water-water-bong commented on Sep 27, 2026

    @bong-water-water-bong
    Collaborator

    Follow-up to the KFD eviction finding above: request-level correlation test, honest nuanced result - does NOT strengthen the causal claim.

    Ran a second, more precise test: recorded CLOCK_MONOTONIC timestamps immediately before/after each of 150 sequential /completion requests (same clock domain as rocprofv3's KFD trace, both on strixhalo), then checked whether each request's decode window [t0, t1] overlapped a KFD eviction/restore event (RESTORE_RESCHEDULED, EVICT_SVM, EVICT_USERPTR) - both exact-window overlap and a ±20ms margin.

    This run: 87/150 (58%) divergent, 42 total eviction-related events captured (a much shorter/quieter eviction window than the first trace's 704 events/30s - this capture's request burst was only ~4.2s, and eviction activity spanned ~14s, so only part of the eviction activity temporally coincides with when requests were actually firing).

    Result: divergent requests did not show a higher eviction co-occurrence rate than canonical ones - if anything, slightly lower, though counts are small enough that this shouldn't be read as a confirmed refutation either:

    • Exact overlap: divergent 9/87 (10.3%) vs canonical 9/63 (14.3%)
    • ±20ms margin: divergent 12/87 (13.8%) vs canonical 14/63 (22.2%)

    What this means: the aggregate correlation found earlier (eviction/restore churn only exists under the same contention condition that triggers the race, sustained for as long as contention exists) remains real and is still the most concrete lead. But this more precise test doesn't support the stronger, more useful claim that "an eviction event happening during a specific request's dispatch is what corrupts that request's result." Either the causal path is more indirect (e.g. a delayed effect from an earlier incomplete restore, not a same-instant one), the true corrupting window doesn't align with when the KFD event gets logged (eviction detection vs. actual corrupting window may not coincide), or eviction and the data race are both downstream symptoms of the same underlying contention pressure without one directly causing the other. With this small sample (42 eviction events against 150 requests), this test can't distinguish between those possibilities.

    This is a fair, complete stopping point given available tooling: the driver-level eviction/restore churn is real and correlates with contention at the aggregate level, but pinning down precise causation would need either much larger-scale request-level sampling (to get enough eviction-event/divergent-request pairs for a meaningful statistical test) or genuinely different tooling (kernel-level tracing that timestamps the actual corrupting memory access, not just eviction detection).

  11. bong-water-water-bong commented on Sep 27, 2026

    @bong-water-water-bong
    Collaborator

    Re-opening with measurements: b26f194e3's "residual fault was the runtime partial-view stride" is not supported by its own evidence — it was verified on a quiet box, the one condition in which this fault cannot reproduce.

    benchmarks/NOTE-hrx-single-dispatch-verify-2026-09-26.md (committed in ca7820650) concludes the stride fix removed the residual fault, on "d=2100 split 0/3 fallback 0/3 ; d=3000 0/3 0/3 ; d=4800 0/3 0/3 ; d=8192 0/3 0/3". Those runs were on an idle box. Earlier in this same investigation (comment above) the fault was found to need induced contention to appear at all: "0/54 combined attempts for #123 ... under normal (low-contention) conditions", which is why the working repro is a second concurrent llama-bench on HRX0. A fix measured only where the bug is unobservable is not evidence about the bug.

    Re-measured on HEAD (ca7820650, i.e. with b26f194e3), llama-bench -m Qwen3-Coder-30B-A3B-Instruct-Q4_K_M.gguf -dev HRX0 -p 0 -n 8 -d <D> -r 1:

    depth condition multipass faults
    2100 quiet box on 0/1
    4800 quiet box on 0/2
    3000 quiet box on 0/1
    2100 concurrent llama-bench -d 8192 on HRX0 on 2/4
    3000 concurrent on 2/2
    4800 concurrent on 2/2
    2100 concurrent off (GGML_HRX_DISABLE_DISPATCH=flash_attention_decode_split) 0/2
    4800 same session off 0/2

    Load was verified present at the start of each concurrent block (ps count of llama-bench ... HRX0); it is another process's -d 8192 sweep, so contention strength varies run to run — the fallback rows at d4800 were taken as that load was winding down, so the strictest reading of the two is "multipass on faults, multipass off has never been seen to fault", not a perfectly load-matched pair.

    Two things follow.

    1. The fault is contention-gated, and the stride fix did not remove it. Quiet: clean with the fix. Contended: 100% at d3000 and d4800 on the commit that claims "d4800/d8192 now decode 0/5 faults". The NOTE's 0/N numbers are reproducible — they just say nothing about a bug that needs contention to exist.

    2. The stride cannot be this fault's cause, on code grounds. At d2100 the capacity is 2112, so partial_block_capacity = ceil(2112/64) = 33 and producer_block_count = ceil(2100/64) = 33 — numerically equal. Passing the runtime count (pre-fix) or the capacity-derived count (post-fix) as the partial-view stride therefore produces identical addressing, yet d2100 faults 2/4 under contention. Every depth in the table above has producer_block_count == partial_block_capacity, so the whole table is blind to that axis.

    A second avenue is closed: Loom's access sanitizer is unusable for this kernel too, not just race — GGML_HRX_LOOM_SANITIZER=access (and =asan) fails at emission with HRX Loom JIT AMDGPU HSACO emission failed, on both trials. (race is separately blocked by the missing iree_tsan_config global + vector.fragment.load observability.) So neither dynamic sanitizer can instrument the multipass kernel; that route needs an upstream HSACO-emission fix.

    Next discriminating experiment (not run): decouple tokens from capacity, since they are confounded in every row above. A llama-server -c 8192 run with a ~2100-token prompt gives capacity 8192 (128 blocks) with 33 producer blocks — if it faults at a similar rate the stride/block-count axis is excluded and the fault is in something capacity-independent (kernarg/descriptor setup, or the counter-then-barrier handoff); if it is clean, the block-count relationship is implicated after all. That, plus a bounds-instrumented build once the HSACO gap is closed, is what this needs next.

  12. bong-water-water-bong commented on Sep 27, 2026

    @bong-water-water-bong
    Collaborator

    Follow-up: the stride fix cannot have changed addressing in this path at all — code-level proof, plus a retraction of my own last suggestion.

    ggml/src/ggml-hrx/dispatch_registration/common/dispatch-flash-attention.cpp:

    :322   const int64_t key_value_token_count = mask->ne[0];            // the RUNTIME token count
    :349   match.key_value_capacity = ceil_div(key_value_token_count, kDecodeKvTileSize) * kDecodeKvTileSize;
    :23    static constexpr int64_t kDecodeKvTileSize = 64;
    :559   const int64_t key_value_block_count = ceil_div(match.key_value_capacity, kDecodeKvTileSize);
    

    The decode path defines the capacity as the token count rounded up to a 64-token tile. So

    • partial_block_capacity (the kernel's partial-view stride, from the capacity compile parameter) = ceil(token_count/64)
    • producer_block_count (token-derived, the reduce loop bound) = ceil(token_count/64)

    are the same integer by construction, at every depth. Passing producer_block_count (pre-b26f194e3) or partial_block_capacity_ct (post) as the view capacity therefore yields identical address arithmetic. That is why the commit's own depths still fault here — b26f194e3 was a real change to a code path that was already numerically equivalent, so its "residual fault was the runtime partial-view stride" conclusion cannot be right for #123, independently of the fact that its 0/N evidence came from an idle box where this fault never appears.

    Retraction: my previous comment proposed decoupling tokens from capacity as the next experiment (llama-server -c 8192 with a ~2100-token prompt). That is not possible in this path — the capacity is derived from the token count here, so llama-server -c cannot move it. Scratch that experiment.

    Also newly ruled out (so nobody re-derives it): the host-side transient sizing is consistent with the kernel's views. partial_scalar_count = key_value_head_count * key_value_block_count * kDecodeRowCapacity with kDecodeRowCapacity = 16, and the kernel's views are view<[key_value_head_count]x[partial_block_capacity]x16x...> — the third dimension matches at 16, so the buffers are exactly as large as the views claim. And there is no compiled-kernel cache in the HRX dispatch layer (no cache lookup/invalidation anywhere in ggml/src/ggml-hrx/dispatch*), so a stale specialisation being reused for a different capacity is not available as an explanation either.

    Where that leaves it. The transients are sized exactly for ceil(tokens/64) blocks — there is no slack anywhere — while the fault rate is governed by allocation alignment and by contention, and every source-level access is guarded. That combination says the access is one that lands outside the allocation by construction of some address, and that whether it faults is decided by what page it lands in. Two things would settle it, and neither is available here today: a build with an operational bounds sanitizer (Loom access dies at HSACO emission for this kernel, same class of gap as race), or a dump of the dispatched kernarg segment / buffer descriptors next to the device VAs the runtime actually allocated, so an address can be compared rather than argued about. If anyone has a working access-sanitized build of this kernel, that is the single most valuable thing to run against the repro above (/tmp/count_faults.sh <depth> <trials> with a concurrent llama-bench -dev HRX0 -d 8192).

  13. bong-water-water-bong commented on Sep 27, 2026

    @bong-water-water-bong
    Collaborator

    Deterministic repro found — the fault does not need contention after all, and it overshoots a transient by roughly 0.25–1 KB.

    The three partial transients' alignment is a literal in dispatch-flash-attention.cpp (partial_max/partial_sum/partial_output, lines 581/583/585; the next_q8_output one at 587 is separate). Rebuilding ggml-hrx with those three at 256 instead of 4096, and running on a quiet box (no concurrent workload at all — ps count of llama-bench ... HRX0 verified 0), gives:

    partial-transient alignment condition -d 2100 faults
    256 quiet box 5/5
    1024 quiet box 0/2
    2048 quiet box 0/2
    4096 quiet box 0/2

    So:

    1. This is not fundamentally a race and does not need induced contention. 5/5 repeatable on an idle box at 256. The earlier "0/54 without contention" result — and the whole contention-based methodology it forced — was an artefact of the tree being at 4096 alignment, where the fault is masked. That also explains why every fix so far looked effective or ineffective depending on who measured: the observability, not the code, was changing.
    2. The failing access lands 256–1024 bytes past the end of a transient. At 256 alignment the arena has at most 255 bytes of tail slack, so the access leaves the mapping and faults; at 1024+ the arena is padded to the larger alignment and the same overshoot stays inside it. That brackets the overshoot: > 255 B and ≤ 1023 B. For these shapes one block of partial_max/partial_sum is key_value_head_count * 16 * 4 = 256 B (4 KV heads, Qwen3-Coder-30B), i.e. this is consistent with indexing exactly one block past the end of a partial buffer — an off-by-one in the block index, not a wild pointer.
    3. It also means the culprit is reachable deterministically, so this can finally be bisected rather than sampled: set those three alignments to 256, cmake --build <build> --target ggml-hrx, then llama-bench -dev HRX0 -p 0 -n 8 -d 2100 faults every time; set them back to 4096 to make it disappear. No contention harness, no 5-run coin flips.

    Repro counter used here: /tmp/count_faults.sh <depth> <trials> [env...], LD_LIBRARY_PATH=<build>/bin, IREE_HAL_AMDGPU_LIBHSA_PATH=/opt/rocm-therock/lib/python3.14/site-packages/_rocm_sdk_devel/lib/libhsa-runtime64.so.1.

    This does not by itself name the site — the source-level accesses are all guarded, as established earlier — but it converts the search from "somewhere in a capacity-dependent JIT emission, occasionally, under contention" into "248 bytes-to-1 KB past the end of a partial transient, every time", which is a shape a bounds check or a descriptor comparison can actually be aimed at. I am moving this work onto the engine tree now (the engine is where #123 lives and where the llama.cpp pin is set), so the fix lands against the pin rather than only in a loose fork worktree.

  14. 46 remaining items

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 working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions