From 636150d4acd099299b03e93689b78fd02cf22ca2 Mon Sep 17 00:00:00 2001 From: bcloud Date: Mon, 9 Nov 2026 04:39:19 -0400 Subject: [PATCH 1/2] hrx: claim a head-row ROPE only when a fused matcher accepts its parameters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `fused_context_claim` accepted a per-head ROPE on SHAPE alone (`is_head_row(op) && fed_by_op(op)`), while the two registrations that execute rotations accept different PARAMETER sets: * llm/dispatch-attention-qkv.cpp: `n_dims == head_size && mode == NORMAL`, ext_factor 0, attn_factor 1; * common/dispatch-rope-set-rows.cpp: `4 <= n_dims <= head_size`, multiple of 4, mode NORMAL / NEOX / collapsible IMROPE. A vision projector's rotation (`n_dims = 32` on a 64-wide head, `mode = GGML_ROPE_TYPE_VISION`) satisfies neither, but it is a head row fed by an op, so it was claimed and then refused when the dispatch was built: E graph_compute: unsupported HRX node 11: ROPE output=f32[64,16,784,1] <- ROPE inputs=[f32[64,16,784,1] <- VIEW] E graph_compute: also: ROPE matcher rejected node: ROPE params or positions shape is unsupported params={n_dims=32, mode=24, …} E clip_image_batch_encode: ggml_backend_sched_graph_compute failed with error -1 i.e. the whole CLIP graph failed instead of that one node running on the CPU, which is why engine #1467's projector cannot run with HRX0 in the graph at all. The condition now lives in one place, `ggml/src/ggml-hrx/rope-claim-params.h`: the two matcher predicates plus their union. Both matchers forward to it (no behaviour change) and the claim requires the union, so the claim cannot drift away from the matchers again - which is exactly how two earlier attempts at this fix broke text decode: each restated the condition and used only the SET_ROWS predicate, which text rotations do not satisfy because they run inside the attention fusion. Verified without a device (the GPU was held by another lane's bench for this whole window): `tests/test-hrx-rope-claim-params.cpp` (ctest `test-hrx-rope-claim-params`) asserts the predicates directly and drives `fused_context_claim` on a graph shaped like the failing node (3-D, VIEW over a produced tensor, so the head-row and fed-by-an-op rules both hold): * text rotation (n_dims == head width, NORMAL) -> claimed (unchanged by this commit) * vision rotation (n_dims 32, mode VISION) -> NOT claimed <- the fix and, with ONLY this claim change reverted and everything else in place, the test aborts with `claim for vision rotation: got 1, wanted 0` - so it fails on the defect and passes with the fix. Still blocking the rest of #1467: with the projector on HRX the next refused node is a pure-alias VIEW (`unsupported HRX node 20: VIEW f32[2048,2,1,1] <- VIEW`), which is claimed by the standalone path rather than by a fused claim. That is a separate change and is not in this commit. Refs #1467 --- .../common/dispatch-rope-set-rows.cpp | 9 +- .../llm/dispatch-attention-qkv.cpp | 7 +- ggml/src/ggml-hrx/fused-context-claim.h | 19 +- ggml/src/ggml-hrx/rope-claim-params.h | 69 +++++++ tests/CMakeLists.txt | 7 + tests/test-hrx-rope-claim-params.cpp | 173 ++++++++++++++++++ 6 files changed, 275 insertions(+), 9 deletions(-) create mode 100644 ggml/src/ggml-hrx/rope-claim-params.h create mode 100644 tests/test-hrx-rope-claim-params.cpp diff --git a/ggml/src/ggml-hrx/dispatch_registration/common/dispatch-rope-set-rows.cpp b/ggml/src/ggml-hrx/dispatch_registration/common/dispatch-rope-set-rows.cpp index 5595f0e90297..7360dafc65ed 100644 --- a/ggml/src/ggml-hrx/dispatch_registration/common/dispatch-rope-set-rows.cpp +++ b/ggml/src/ggml-hrx/dispatch_registration/common/dispatch-rope-set-rows.cpp @@ -6,6 +6,7 @@ #include "dispatch-layout-utils.h" #include "dispatch-rope-utils.h" +#include "rope-claim-params.h" #include "ggml.h" #include "qwen4exp-hrx-support.h" #include "graph/graph-matcher.h" @@ -129,12 +130,10 @@ static bool is_supported_rope_input_layout(const Value & value, size_t & span_el return required <= span_elements; } +// The condition itself lives in ggml/src/ggml-hrx/rope-claim-params.h, so the claim in +// fused-context-claim.h cannot drift away from what this matcher accepts (engine #1467). static bool is_supported_rope_params(const RopeParams & params, int64_t head_size) { - const bool supported_mode = params.mode == GGML_ROPE_TYPE_NORMAL || params.mode == GGML_ROPE_TYPE_NEOX || - rope_mode_is_collapsible_imrope(params); - return params.n_dims >= 4 && params.n_dims <= head_size && params.n_dims % 4 == 0 && supported_mode && - std::isfinite(params.freq_base) && params.freq_base > 0.0f && std::isfinite(params.freq_scale) && - params.freq_scale > 0.0f && std::isfinite(params.ext_factor) && std::isfinite(params.attn_factor); + return rope_params_supported_by_rope_set_rows(params, head_size); } static bool build_rope_theta_table(const GraphNode & rope, diff --git a/ggml/src/ggml-hrx/dispatch_registration/llm/dispatch-attention-qkv.cpp b/ggml/src/ggml-hrx/dispatch_registration/llm/dispatch-attention-qkv.cpp index ea81bda320f3..03462b793aed 100644 --- a/ggml/src/ggml-hrx/dispatch_registration/llm/dispatch-attention-qkv.cpp +++ b/ggml/src/ggml-hrx/dispatch_registration/llm/dispatch-attention-qkv.cpp @@ -4,6 +4,7 @@ #include "ggml.h" #include "graph/graph-matcher.h" #include "kernel-corpus/kernel-corpus-catalog-verify.h" +#include "rope-claim-params.h" #include #include @@ -104,10 +105,10 @@ static std::string to_config_value(int64_t value) { return std::to_string(value); } +// The condition itself lives in ggml/src/ggml-hrx/rope-claim-params.h, because the claim in +// fused-context-claim.h has to assert exactly what this matcher accepts (engine #1467). static bool is_supported_attention_rope_params(const RopeParams & params, int64_t head_size) { - return params.n_dims == head_size && params.mode == GGML_ROPE_TYPE_NORMAL && std::isfinite(params.freq_base) && - params.freq_base > 0.0f && std::isfinite(params.freq_scale) && params.freq_scale > 0.0f && - params.ext_factor == 0.0f && params.attn_factor == 1.0f; + return rope_params_supported_by_attention_fusion(params, head_size); } static bool build_rope_theta_table(const GraphNode & rope, int64_t head_size, std::vector & data) { diff --git a/ggml/src/ggml-hrx/fused-context-claim.h b/ggml/src/ggml-hrx/fused-context-claim.h index f098887a6089..9ab4f23a810a 100644 --- a/ggml/src/ggml-hrx/fused-context-claim.h +++ b/ggml/src/ggml-hrx/fused-context-claim.h @@ -39,6 +39,7 @@ // standalone test. #include "ggml.h" +#include "rope-claim-params.h" #include "ubatch-token-limits.h" #include @@ -155,8 +156,24 @@ inline bool fused_context_claim(const ggml_tensor * op) { op->ne[2] <= 3 && op->ne[0] >= 32 && op->ne[0] <= 65536 && op->ne[0] % 32 == 0; // per-head attention normalisation and rotation case GGML_OP_RMS_NORM: - case GGML_OP_ROPE: return fused_context_is_head_row(op) && fused_context_fed_by_op(op); + case GGML_OP_ROPE: { + // A head-row rotation is claimed only when some fused registration accepts its PARAMETERS as well as + // its shape: a claim that is broader than the matchers leaves the node with no dispatch, and the + // whole graph fails instead of the node falling back to the CPU. engine #1467 is that failure: the + // Qwen3-VL projector's ROPE (n_dims 32 against a 64-wide head, mode GGML_ROPE_TYPE_VISION) is a + // head row fed by an op, so it was claimed, and both matchers then rejected it - "ROPE matcher + // rejected node: ROPE params or positions shape is unsupported" - which made clip_image_batch_encode + // fail on HRX0 at chunk 25 instead of running that node on the CPU. The predicates are shared with the + // matchers (rope-claim-params.h) rather than restated here, because restating them is what broke text + // decode twice: text rotations satisfy the attention fusion's set, not the ROPE+SET_ROWS one. + if (!fused_context_is_head_row(op) || !fused_context_fed_by_op(op)) { + return false; + } + const OpParams imported = import_op_params(*op); + const RopeParams * params = op_params_as(imported); + return params != nullptr && rope_params_supported_by_any_fusion(*params, op->ne[0]); + } default: return false; } diff --git a/ggml/src/ggml-hrx/rope-claim-params.h b/ggml/src/ggml-hrx/rope-claim-params.h new file mode 100644 index 000000000000..b2517de047bb --- /dev/null +++ b/ggml/src/ggml-hrx/rope-claim-params.h @@ -0,0 +1,69 @@ +// Copyright 2026 bong-water-water-bong +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#pragma once + +// Which ROPE parameter sets can a fused registration actually execute? +// +// The claim in `fused-context-claim.h` says "this node will run inside a fused dispatch", and +// `device_supports_op` uses that to keep the node on this backend. If the claim is broader than the +// matchers, the node is claimed and then refused when the dispatch is built, and the whole graph +// fails instead of the node falling back to the CPU - engine #1467: a vision projector's ROPE +// (`n_dims = 32`, `mode = GGML_ROPE_TYPE_VISION`) was claimed by the head-row rule and then rejected +// by both matchers, so `clip_image_batch_encode` could not compute at all on HRX0. +// +// Two matchers accept rotations and their parameter sets differ: +// +// * the attention fusion (llm/dispatch-attention-qkv.cpp): n_dims == head_size, mode NORMAL; +// * the ROPE + SET_ROWS registration (common/dispatch-rope-set-rows.cpp): 4 <= n_dims <= head_size, +// n_dims a multiple of 4, mode NORMAL / NEOX / a collapsible IMROPE. +// +// Their union is what the claim may assert. Both predicates live here, the two matchers call them and +// the claim calls the union, so a matcher can be narrowed without the claim silently going stale - +// which is how the two earlier attempts at this fix broke text decode: they re-derived the condition +// and used only the SET_ROWS one, which text rotations do not satisfy (they run inside the attention +// fusion). + +#include "graph/op-params.h" +#include "qwen4exp-hrx-support.h" // rope_mode_is_collapsible_imrope + +#include + +namespace ggml::hrx { + +// llm/dispatch-attention-qkv.cpp: the per-head rotation consumed by the attention fusions. +inline bool rope_params_supported_by_attention_fusion(const RopeParams & params, int64_t head_size) { + return params.n_dims == head_size && params.mode == GGML_ROPE_TYPE_NORMAL && std::isfinite(params.freq_base) && + params.freq_base > 0.0f && std::isfinite(params.freq_scale) && params.freq_scale > 0.0f && + params.ext_factor == 0.0f && params.attn_factor == 1.0f; +} + +// common/dispatch-rope-set-rows.cpp: the rotation that registration builds a theta table for. +inline bool rope_params_supported_by_rope_set_rows(const RopeParams & params, int64_t head_size) { + const bool supported_mode = params.mode == GGML_ROPE_TYPE_NORMAL || params.mode == GGML_ROPE_TYPE_NEOX || + rope_mode_is_collapsible_imrope(params); + return params.n_dims >= 4 && params.n_dims <= head_size && params.n_dims % 4 == 0 && supported_mode && + std::isfinite(params.freq_base) && params.freq_base > 0.0f && std::isfinite(params.freq_scale) && + params.freq_scale > 0.0f && std::isfinite(params.ext_factor) && std::isfinite(params.attn_factor); +} + +// What the claim may assert: at least one fused registration accepts these parameters. Anything else +// (a vision rotation, a mode nobody implements) must fall back to the CPU rather than be claimed. +inline bool rope_params_supported_by_any_fusion(const RopeParams & params, int64_t head_size) { + return rope_params_supported_by_attention_fusion(params, head_size) || + rope_params_supported_by_rope_set_rows(params, head_size); +} + +} // namespace ggml::hrx diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index c592e6b41034..972e161370de 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -350,6 +350,13 @@ add_test(NAME test-server-health COMMAND test-server-health) if (TARGET ggml-hrx) # the program cache's byte budget and LRU order: pure policy, no device and no HRX link + # Which rotations the fused registrations accept, i.e. what the claim may assert (engine #1467). + # Header-only: no device, no model, and the vision rotation must be rejected here. + add_executable(test-hrx-rope-claim-params test-hrx-rope-claim-params.cpp) + target_include_directories(test-hrx-rope-claim-params PRIVATE ../ggml/include ../ggml/src ../ggml/src/ggml-hrx) + add_test(NAME test-hrx-rope-claim-params COMMAND test-hrx-rope-claim-params) + target_link_libraries(test-hrx-rope-claim-params PRIVATE ggml) + add_executable(test-hrx-graph-program-budget test-hrx-graph-program-budget.cpp ../ggml/src/ggml-hrx/runtime/graph-program-budget.cpp) target_include_directories(test-hrx-graph-program-budget PRIVATE ../ggml/include ../ggml/src ../ggml/src/ggml-hrx) diff --git a/tests/test-hrx-rope-claim-params.cpp b/tests/test-hrx-rope-claim-params.cpp new file mode 100644 index 000000000000..0471d2225bbd --- /dev/null +++ b/tests/test-hrx-rope-claim-params.cpp @@ -0,0 +1,173 @@ +// Copyright 2026 bong-water-water-bong +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Host test for rope-claim-params.h: which rotations the fused registrations accept, and therefore what +// the claim in fused-context-claim.h may assert. No device and no model - the predicates are pure. +// +// The case this exists for is engine #1467: a vision projector's ROPE carries mode +// GGML_ROPE_TYPE_VISION and n_dims 32 against a 64-wide head, which BOTH matchers reject. The claim used +// to accept it on shape alone, so the node was claimed and then refused when the dispatch was built, and +// the whole CLIP graph failed instead of that node running on the CPU. + +#include "fused-context-claim.h" +#include "rope-claim-params.h" + +#include "ggml.h" + +#include +#include + +#define REQUIRE(condition) \ + do { \ + if (!(condition)) { \ + std::fprintf(stderr, "%s:%d: requirement failed: %s\n", __FILE__, __LINE__, #condition); \ + std::abort(); \ + } \ + } while (0) + +namespace { + +using ggml::hrx::RopeParams; + +RopeParams text_rotation(int n_dims, int head_size) { + RopeParams params; + params.n_dims = n_dims; + params.mode = GGML_ROPE_TYPE_NORMAL; + params.freq_base = 1000000.0f; + params.freq_scale = 1.0f; + params.ext_factor = 0.0f; + params.attn_factor = 1.0f; + (void) head_size; + return params; +} + +// The projector rotation: two-dimensional vision rotary, per ggml.h "GGML_ROPE_TYPE_VISION n_dims = 4 --> +// [yyyyxxxx]". The node #1467 fails on is n_dims 32 against head_size 64 with this mode. +RopeParams vision_rotation() { + RopeParams params; + params.n_dims = 32; + params.mode = GGML_ROPE_TYPE_VISION; + params.freq_base = 10000.0f; + params.freq_scale = 1.0f; + params.ext_factor = 0.0f; + params.attn_factor = 1.0f; + return params; +} + +// The claim itself, on a graph shaped like the one engine #1467 fails on: a ROPE whose source is a VIEW +// (so the head-row and fed-by-an-op rules both hold) over a 64-wide head. Pre-fix the claim accepted the +// vision rotation on shape alone and the node was then refused at dispatch build, which failed the whole +// CLIP graph instead of running that node on the CPU. +static void check_claim_for(bool vision, bool want_claimed) { + ggml_init_params params = { /*mem_size*/ 24 * 1024 * 1024, /*mem_buffer*/ nullptr, /*no_alloc*/ false }; + ggml_context * ctx = ggml_init(params); + REQUIRE(ctx != nullptr); + + // 3-D, as the claim's fused_context_is_3d requires - the real node in #1467 is [64, 16, 784, 1], i.e. ne[3] == 1. + ggml_tensor * base = ggml_new_tensor_3d(ctx, GGML_TYPE_F32, 64, 16, 784); + // fed-by-an-op is resolved through the view chain to its ROOT (fused_context_producer_op), so the view has + // to sit on top of a tensor an op produced - a view of a leaf is GGML_OP_NONE and would make the claim false + // for the wrong reason, which is what an earlier version of this test did. + ggml_tensor * ones = ggml_new_tensor_3d(ctx, GGML_TYPE_F32, 64, 16, 784); + ggml_tensor * mul = ggml_mul(ctx, base, ones); + ggml_tensor * view = ggml_view_3d(ctx, mul, 64, 16, 784, mul->nb[1], mul->nb[2], 0); + ggml_tensor * pos = ggml_new_tensor_1d(ctx, GGML_TYPE_I32, vision ? 3136 : 784); + ggml_tensor * rope = ggml_rope_ext(ctx, view, pos, nullptr, vision ? 32 : 64, + vision ? GGML_ROPE_TYPE_VISION : GGML_ROPE_TYPE_NORMAL, 0, 1000000.0f, 1.0f, + 0.0f, 1.0f, 0.0f, 0.0f); + + // the backend the claim asks about: "is the producer supported" - here, yes for everything, so the + // only thing under test is the ROPE rule itself. + const auto supported = [](const ggml_tensor * t) { return t != nullptr; }; + const bool claimed = ggml::hrx::fused_context_claim(rope, supported); + if (claimed != want_claimed) { + std::fprintf(stderr, "claim for %s rotation: got %d, wanted %d (engine #1467)\n", + vision ? "vision" : "text", (int) claimed, (int) want_claimed); + std::abort(); + } + ggml_free(ctx); +} + +} // namespace + +int main() { + using namespace ggml::hrx; + + { + // A text attention rotation: n_dims equals the head width, mode NORMAL. The attention fusion takes + // it, and it is the case that broke twice when the claim was narrowed with the SET_ROWS predicate + // alone (text rotations do not satisfy that one). + const RopeParams params = text_rotation(64, 64); + REQUIRE(rope_params_supported_by_attention_fusion(params, 64)); + REQUIRE(rope_params_supported_by_any_fusion(params, 64)); + } + { + // A head-width NEOX rotation with fewer dims than the head: the attention fusion declines (mode and + // n_dims), the ROPE+SET_ROWS registration takes it, so the claim may still assert it. + RopeParams params = text_rotation(32, 64); + params.mode = GGML_ROPE_TYPE_NEOX; + REQUIRE(!rope_params_supported_by_attention_fusion(params, 64)); + REQUIRE(rope_params_supported_by_rope_set_rows(params, 64)); + REQUIRE(rope_params_supported_by_any_fusion(params, 64)); + } + { + // THE REGRESSION: the vision rotation. Neither matcher accepts it, so the claim must not either - + // this is the assertion that would have caught engine #1467 before an image request crashed. + const RopeParams params = vision_rotation(); + REQUIRE(!rope_params_supported_by_attention_fusion(params, 64)); + REQUIRE(!rope_params_supported_by_rope_set_rows(params, 64)); + REQUIRE(!rope_params_supported_by_any_fusion(params, 64)); + } + { + // The rejection is on the MODE as well as on n_dims: a vision rotation at head width is still not a + // rotation any fused registration executes. + RopeParams params = vision_rotation(); + params.n_dims = 64; + REQUIRE(!rope_params_supported_by_attention_fusion(params, 64)); + REQUIRE(!rope_params_supported_by_rope_set_rows(params, 64)); + REQUIRE(!rope_params_supported_by_any_fusion(params, 64)); + } + { + // n_dims wider than the head is nobody's case. + RopeParams params = text_rotation(128, 64); + REQUIRE(!rope_params_supported_by_any_fusion(params, 64)); + } + { + // A degenerate freq_base is declined by both, so it cannot be claimed. + RopeParams params = text_rotation(64, 64); + params.freq_base = 0.0f; + REQUIRE(!rope_params_supported_by_any_fusion(params, 64)); + params.freq_base = 1000000.0f; + REQUIRE(rope_params_supported_by_any_fusion(params, 64)); + } + { + // ext_factor is only the attention fusion's condition: the SET_ROWS path builds its own theta table + // and takes it. This is the case where the two matchers genuinely differ, so the union is what the + // claim has to use. + RopeParams params = text_rotation(32, 64); + params.mode = GGML_ROPE_TYPE_NEOX; + params.ext_factor = 1.0f; + REQUIRE(!rope_params_supported_by_attention_fusion(params, 64)); + REQUIRE(rope_params_supported_by_rope_set_rows(params, 64)); + REQUIRE(rope_params_supported_by_any_fusion(params, 64)); + } + + // The regression proper: the claim must refuse the vision rotation and keep the text one. + check_claim_for(/*vision*/ true, /*want_claimed*/ false); + check_claim_for(/*vision*/ false, /*want_claimed*/ true); + + std::printf("test-hrx-rope-claim-params: all requirements met\n"); + return 0; +} From 5cd2181a3344e5a55702d23a6ce8d7e268aafb7a Mon Sep 17 00:00:00 2001 From: bcloud Date: Mon, 9 Nov 2026 06:11:13 -0400 Subject: [PATCH 2/2] hrx: say which relation a refused layout-alias node has, and pin alias elision in a host test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit engine #1467's projector is refused at a node the log names only by shape: E graph_compute: unsupported HRX node 20: VIEW output=f32[2048,2,1,1] <- VIEW inputs=[f32[2048,2,1,1]] consumers=[21:ADD] A layout-alias node is refused there for one of two completely different reasons and the shape cannot tell them apart: it IS an alias (one input, same storage as its input) whose input was not available when the node was visited, or it is not aliasing its input at all and therefore needs a dispatch that no registration provides. The message now says which: ... consumers=[…] alias=same_storage inputs=1 ... consumers=[…] alias=not_aliasing_input inputs=2 `alias=` is `is_layout_alias_node(graph, node)` — the helper the elision path itself calls twelve lines below, not a re-derivation of it. That is deliberate (review point on this PR): a diagnostic that restates the scheduler's rule will eventually describe a scheduler that no longer exists, which is the same argument as sharing rope-claim-params.h between the claim and its matchers in #257. The wording says `not_aliasing_input` rather than "not an alias" because the op here IS a layout-alias op; what is false is the storage relation. Alongside it, `tests/test-hrx-view-alias-elision.cpp` (ctest `test-hrx-view-alias-elision`, links ggml-hrx) builds the graph by hand and pins four storage relations, host-only, no backend: no-op alias, external input : plans <- what can_execute_standalone_op_as_graph builds no-op alias, produced input : plans <- the real graph offset alias, produced input: plans strided view, produced input: REFUSED <- and the refusal lands on the CONSUMER, not the view The first two lines are the useful ones: a lone node's input provenance does NOT decide elision, which refutes the reading that the projector's refusal comes from value_is_available() returning true for a producer-less value (dispatch-scheduler.cpp:102-106). That reading cost this lane a wrong comment on the issue, so it is now an assertion. The last line records that a strided view is elided as well — same_storage() compares the storage ROOT, not the layout — and that the plan is then refused at the consumer; if a consumer ever accepts a strided alias, that assertion is where it becomes visible. Verified: ggml-hrx links clean with -Werror and the test runs green (its status line is the new field through the scheduler's own plan().status.errors()). The device run this diagnostic exists for is on the projector; it printed `alias=not_aliasing_input` for node 20, which is what moved the investigation from "which of two causes" to "ValueMap gave the view a storage of its own". Refs #1467 --- .../ggml-hrx/dispatch/dispatch-scheduler.cpp | 14 ++ tests/CMakeLists.txt | 7 + tests/test-hrx-view-alias-elision.cpp | 142 ++++++++++++++++++ 3 files changed, 163 insertions(+) create mode 100644 tests/test-hrx-view-alias-elision.cpp diff --git a/ggml/src/ggml-hrx/dispatch/dispatch-scheduler.cpp b/ggml/src/ggml-hrx/dispatch/dispatch-scheduler.cpp index a35c773cf260..354369ba2bd6 100644 --- a/ggml/src/ggml-hrx/dispatch/dispatch-scheduler.cpp +++ b/ggml/src/ggml-hrx/dispatch/dispatch-scheduler.cpp @@ -96,6 +96,20 @@ static std::string unsupported_node_message(const Graph & graph, size_t index, c append_node_summary(stream, graph, consumers[i]); } stream << "]"; + // A layout-alias node is refused here for one of two completely different reasons, and the shape cannot tell them + // apart: it is an alias (one input, same storage) whose input was not available yet, or it is NOT an alias at all + // (a strided view of the same shape) and therefore needs a dispatch that no registration provides, because no + // registration names a layout-alias op as its root. engine #1467's projector is refused at + // VIEW output=f32[2048,2,1,1] <- f32[2048,2,1,1] + // and that line alone did not say which case it was, so the next reader had to reproduce it. This costs one line + // in the message and answers it in the log. + if (is_layout_alias_op(node.op)) { + // is_layout_alias_node() is this predicate exactly (graph/graph.cpp): a message that restates the scheduler's rule + // is a message that will describe a scheduler that no longer exists. Same reasoning as rope-claim-params.h sharing + // its predicates with the matchers in #257. + stream << " alias=" << (is_layout_alias_node(graph, node) ? "same_storage" : "not_aliasing_input") + << " inputs=" << node.inputs.size(); + } return stream.str(); } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 972e161370de..1d27cfa1e2af 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -399,6 +399,13 @@ if (TARGET ggml-hrx) target_include_directories(test-hrx-loom-jit PRIVATE ../ggml/include ../ggml/src ../ggml/src/ggml-hrx) add_test(NAME test-hrx-loom-jit COMMAND test-hrx-loom-jit) + # How a layout-alias node is judged when its input is PRODUCED vs EXTERNAL: engine #1467's projector is + # refused at a no-op VIEW that the standalone oracle calls supported because it feeds the view from a leaf. + add_executable(test-hrx-view-alias-elision test-hrx-view-alias-elision.cpp) + target_link_libraries(test-hrx-view-alias-elision PRIVATE ggml-hrx ggml hrx::hrx loomc::loomc) + target_include_directories(test-hrx-view-alias-elision PRIVATE ../ggml/include ../ggml/src ../ggml/src/ggml-hrx) + add_test(NAME test-hrx-view-alias-elision COMMAND test-hrx-view-alias-elision) + add_executable(test-hrx-ops test-hrx-ops.cpp) target_link_libraries(test-hrx-ops PRIVATE ggml-hrx ggml hrx::hrx loomc::loomc) target_include_directories(test-hrx-ops PRIVATE ../ggml/include ../ggml/src ../ggml/src/ggml-hrx) diff --git a/tests/test-hrx-view-alias-elision.cpp b/tests/test-hrx-view-alias-elision.cpp new file mode 100644 index 000000000000..8c1946c8a6d1 --- /dev/null +++ b/tests/test-hrx-view-alias-elision.cpp @@ -0,0 +1,142 @@ +// Copyright 2026 bong-water-water-bong +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Host test for how a layout-alias node is treated when its input is PRODUCED vs EXTERNAL, which is the +// divergence engine #1467's projector hits: with the projector on HRX the graph is refused at +// unsupported HRX node 20: VIEW output=f32[2048,2,1,1] <- VIEW inputs=[f32[2048,2,1,1]] consumers=[21:ADD] +// while `can_execute_standalone_op_as_graph` reports the same node supported, because the single-node graph it +// builds feeds the view from an External leaf and `value_is_available()` returns true when a value has no +// producer (dispatch-scheduler.cpp:102-106). +// +// No device: the scheduler, the registry and the graph are all host-side; only the tensors are real ggml +// objects, and no backend is initialised. + +#include "dispatch/dispatch-scheduler.h" +#include "graph/graph.h" +#include "graph/op-params.h" + +#include "ggml.h" + +#include +#include +#include + +#define REQUIRE(condition) \ + do { \ + if (!(condition)) { \ + std::fprintf(stderr, "%s:%d: requirement failed: %s\n", __FILE__, __LINE__, #condition); \ + std::abort(); \ + } \ + } while (0) + +namespace { + +using namespace ggml::hrx; + +const DispatchTarget kTarget{ "gfx1151" }; + +// A graph shaped like the failing one: an external leaf, one op that produces a value on this backend, and a +// no-op layout alias on top of it. `produced` selects whether the alias's input is that produced value (the +// real graph) or a second external leaf (what can_execute_standalone_op_as_graph builds). +enum class ViewKind { NoOpAlias, OffsetAlias, StridedView }; + +bool plans_with_view(ViewKind kind, bool produced) { + ggml_init_params params = { /*mem_size*/ 32 * 1024 * 1024, /*mem_buffer*/ nullptr, /*no_alloc*/ false }; + ggml_context * ctx = ggml_init(params); + REQUIRE(ctx != nullptr); + + // 3 rows so an offset view stays in bounds + ggml_tensor * a = ggml_new_tensor_2d(ctx, GGML_TYPE_F32, 2048, 3); + ggml_tensor * b = ggml_new_tensor_2d(ctx, GGML_TYPE_F32, 2048, 3); + ggml_tensor * add = ggml_add(ctx, a, b); + ggml_tensor * base = produced ? add : a; + // The three relations that matter: a true alias (same storage), an alias at an offset (same buffers, different + // start - the question is whether ValueMap gives it the same storage), and a genuinely strided view (same shape, + // different nb[1]) which is NOT an alias and therefore needs a real dispatch that no registration provides. + ggml_tensor * view = nullptr; + switch (kind) { + case ViewKind::NoOpAlias: view = ggml_view_2d(ctx, base, 2048, 2, base->nb[1], 0); break; + case ViewKind::OffsetAlias: view = ggml_view_2d(ctx, base, 2048, 2, base->nb[1], base->nb[1]); break; + case ViewKind::StridedView: view = ggml_view_2d(ctx, base, 2048, 2, 2 * base->nb[1], 0); break; + } + REQUIRE(view != nullptr); + ggml_tensor * out = ggml_scale(ctx, view, 0.5f); + + Graph graph; + std::vector no_inputs; + // the leaves + const ValueId va = graph.values().get_or_add_tensor_value(a, ValueKind::External); + const ValueId vb = graph.values().get_or_add_tensor_value(b, ValueKind::External); + // the producer of the alias's input + ValueId vadd = va; + if (produced) { + const ValueId vadd_out = graph.values().get_or_add_tensor_value(add, ValueKind::Transient); + GraphNode & n_add = graph.add_node(GGML_OP_ADD, vadd_out, { va, vb }); + n_add.params = import_op_params(*add); + vadd = vadd_out; + } + // the alias itself + const ValueId vview = graph.values().get_or_add_tensor_value(view, ValueKind::Transient); + GraphNode & n_view = graph.add_node(GGML_OP_VIEW, vview, { vadd }); + n_view.params = import_op_params(*view); + // something that consumes the alias, so the alias is not the graph's output + const ValueId vout = graph.values().get_or_add_tensor_value(out, ValueKind::Transient); + GraphNode & n_scale = graph.add_node(GGML_OP_SCALE, vout, { vview }); + n_scale.params = import_op_params(*out); + (void) no_inputs; + REQUIRE(graph.build_index().success()); + + DispatchScheduler sched; + const bool planned = sched.schedule_graph(graph, kTarget); + if (!planned) { + // print whatever the scheduler recorded, so the diagnostic added for engine #1467 is visible here + for (const std::string & e : sched.plan().status.errors()) { + std::printf(" status: %s" "\n", e.c_str()); + } + } + ggml_free(ctx); + return planned; +} + +} // namespace + +int main() { + // Measured behaviour of the four relations, pinned so a change in the alias rule shows up here and not in an + // image request two days later. + const bool noop_external = plans_with_view(ViewKind::NoOpAlias, false); + const bool noop_produced = plans_with_view(ViewKind::NoOpAlias, true); + const bool offset = plans_with_view(ViewKind::OffsetAlias, true); + const bool strided = plans_with_view(ViewKind::StridedView, true); + + std::printf("no-op alias, external input : %s\n", noop_external ? "plans" : "REFUSED"); + std::printf("no-op alias, produced input : %s\n", noop_produced ? "plans" : "REFUSED"); + std::printf("offset alias, produced input: %s\n", offset ? "plans" : "REFUSED"); + std::printf("strided view, produced input: %s\n", strided ? "plans" : "REFUSED"); + + // A no-op layout alias plans whether its input is an External leaf (the single-node graph that + // can_execute_standalone_op_as_graph builds) or a value this backend produces (the real graph). That REFUTES + // the reading that engine #1467's VIEW refusal comes from value_is_available() returning true for a + // producer-less value: if that shortcut decided it, the produced case would be refused. + REQUIRE(noop_external); + REQUIRE(noop_produced); + // An offset alias is still one storage root, so it is elided too. + REQUIRE(offset); + // A STRIDED view of the same shape is elided as well - same_storage() compares the storage root, not the layout - + // and the refusal then lands on the CONSUMER (see the status line: "unsupported HRX node 2: SCALE"). Pinned as + // the safe outcome it is today: the plan is refused, not silently computed through a wrong layout. If a consumer + // ever accepts a strided alias, THIS assertion is where it should become visible. + REQUIRE(!strided); + return 0; +}