Repository navigation
hrx: name the two backend-ops suite failures that are not the backend's, and refuse the router launch the kernel cannot take (engine #1259) - #221
Merged
bong-water-water-bong merged 1 commit intoNov 6, 2026
Conversation
…'s, and refuse the 32-expert router launch (engine ggml-org#1259) Full suite at 710c7c2, twice: 4268/4592 both runs, the same 324 failures (suite grew from 4374 since cffae5c4). 1) 318 TOPK_MOE cases fail only on selected_experts, an internal non-contiguous VIEW of the ARGSORT output: the HRX read of that view is garbage while CPU has the ids; comparing the routed weights output (the operation result) passes every shape and gating variant. The real graph consumes the ids inside GET_ROWS on device, so this is a comparison-node artefact, named in a waiver line and still compared on every other backend. 2) two TOPK_MOE with_norm=1 cases are the Loom range failure: router_top8_f32 computes experts_per_lane0 = expert_count / wave_size and assumes [1,16], so 32 experts give 0 and the launch is rejected. The issue attributed this to 31 experts, but the JIT config says expert_count=32. The matcher now requires >=64 experts and a multiple of the 64-lane wave (288 = 4*64+32 left a remainder unscanned, measured 1e-6 in the routed weights); our real MoEs are 128/256. Smaller test graphs run the standalone dispatches. 3) 4 DSV4_HC_POST identity=0 cases: hip.hc_post is a private HIP add-on dispatch, not in this fork, and its non-identity path is wrong there (identity=1 passes). Named waiver, tracked separately. Waiver lines are explicit and printed once per class, e.g. 'WAIVER [HRX0] TOPK_MOE: ...'. Assisted-by: DeepSeek Harness
This was referenced Nov 6, 2026
bong-water-water-bong
added a commit
that referenced
this pull request
Nov 6, 2026
…T when no dispatch covers it (engine ggml-org#1259) (#222) Two bugs of the same shape as the 32-expert router reject that #221 fixed. 1. The matchers accepted the kernel's own upper bound. Every kernel in kernel-corpus/kernels/loom-libs/ops/small_rows_f32.loom carries `index.assume %column_count [range(%column_count, 1, LIMIT)]`, and a Loom range is exclusive of its upper bound, so the kernel refuses LIMIT itself: common.softmax_rows_f32 matcher `<= 4096` kernel range [1, 4096) common.sum_rows_f32 matcher `<= 4096` kernel range [1, 4096) common.argsort_rows_f32 matcher `<= 1024` kernel range [1, 1024), and range(%rank, 0, 1023) common.norm_rows_f32 matcher `<= 65536` kernel range [1, 65536) common.get_rows_small_f32 matcher `<= 65536` kernel range [1, 65536) x4 A shape at exactly the limit was therefore claimed and then rejected at prepare time instead of falling back. The bounds are now exclusive, and get_rows_small also bounds id_stride, which its kernel assumes is < 1048576 and the matcher did not bound at all. 2. ARGSORT was claimed unconditionally by device_supports_op, but the only registration that produces ids is common.argsort_rows_f32. So any ARGSORT shape that matcher declines - and, in particular, the one switched off with GGML_HRX_DISABLE_DISPATCH for a measurement - stayed claimed with no dispatch and failed the WHOLE graph: graph_compute: unsupported HRX node 2: ARGSORT output=3:i32[31,22,1,1] <-ARGSORT inputs=[1:f32[31,22,1,1]<-SOFT_MAX] instead of splitting to the CPU. device_supports_op now asks small_rows_argsort_tensor_supported(), the same bounded-claim pattern TOP_K already uses (hip_top_k_tensor_supported). This also repairs every ARGSORT with more than 1024 columns, which was claimed, declined and failing the graph before. Effect on the suite (pin ced49c6, full `test-backend-ops test -b HRX0`, the TOPK_MOE waiver switched off so the ids are actually compared, one claim): before fix, nothing disabled : 4279/4599, [VIEW] 320, every other node OK after fix, nothing disabled : 4275/4595, [VIEW] 320, every other node OK (no regression) after fix, argsort disabled : 4233/4233 PASS, [VIEW] 0, 0 mismatch dumps The totals move by 4 because four ARGSORT cases with ne[0] >= 1024 used to be claimed and are now honestly declined, so the harness reports them as not supported instead of running them. The second line is the point: with the argsort registration switched off, the ids now fall back to the CPU and the whole suite passes, where before the same arm died on "unsupported HRX node ARGSORT" and only looked like 40 failures - which is what made the debug switch unusable for measurements like the one in engine ggml-org#1259. The kernels themselves are untouched: the ids they produce on device are correct (the routed weights gathered FROM those ids match the CPU in all 320 cases), so nothing here "fixes" a store. The remaining TOPK_MOE difference is the host readback of a non-contiguous VIEW of the argsort output, which is what #221's waiver names.
Author
|
Adds a commit removing the DSV4_HC_POST waiver: the add-on fix is https://github.com/1bit-MONSTER/gpu-kernels/pull/60 (comb matrix read in ggml layout; identity=0 was the transposed read). With that kernel, all 6 DSV4_HC_POST cases pass and only the TOPK_MOE view waiver remains. NOTE: this branch is red on the current engine pin until the gpu-kernels pin is bumped to the PR #60 merge. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
engine ggml-org#1259: the HRX backend-ops suite is green again at tip 710c7c2, with the two non-backend classes named explicitly.
Reproduction at tip: test-backend-ops test -b HRX0 twice -> 4268/4592 both runs, the same 324 failures (issue pin cffae5c4: 4050/4374; the suite grew by 218 tests, the failing set did not change).
After: run1 4592/4592 OK, run3 4592/4592 OK; run2 4591/4592 with one intermittent MUL_MAT(f32,m=1,n=1,k=2048,per=[0,1,2,3],o=1), not in the deterministic 324 and 5/5 filtered - noted as a separate flake, not waived.
Remains: hip.hc_post identity path in the private add-on; optionally materialise non-contiguous view outputs in HRX to remove the first waiver. Fork-only; no engine pin edits.