hrx: do not fuse away a value that something outside the programme reads (engine #533)

ggml/src/ggml-hrx/dispatch_registration/common/dispatch-mul-mat-id.cpp

@@ -244,15 +244,22 @@ static bool match_mul_mat_id_postops_dispatch(const DispatchMatchContext & conte
     dispatch.bindings.push_back({ routing_bundle.expert_table, 0, routing_bundle.expert_table_byte_count });
     dispatch.bindings.push_back({ routing_bundle.partition_table, 0, routing_bundle.partition_table_byte_count });
     dispatch.bindings.push_back({ match.root.weight->id, 0, match.root.weight->byte_count });
+    // The exports declare the bias and residual slots with read access whether or not the kernel uses
+    // them; the kernel only loads them when has_bias / has_residual is set, so an absent slot is
+    // filled with a read-only value the programme reads anyway. Binding the output transient here
+    // instead made the command-program verifier reject the plan ("reads transient value N before
+    // write", engine #533): the slot is declared read, and nothing writes the output before this
+    // command does.
+    const Value & absent_slot = *match.root.weight;
     if (match.has_bias) {
         dispatch.bindings.push_back({ match.bias->id, 0, match.bias->byte_count });
     } else {
-        dispatch.bindings.push_back({ match.residual_output->id, 0, match.residual_output->byte_count });
+        dispatch.bindings.push_back({ absent_slot.id, 0, absent_slot.byte_count });
     }
     if (match.has_residual) {
         dispatch.bindings.push_back({ match.residual_input->id, 0, match.residual_input->byte_count });
     } else {
-        dispatch.bindings.push_back({ match.residual_output->id, 0, match.residual_output->byte_count });
+        dispatch.bindings.push_back({ absent_slot.id, 0, absent_slot.byte_count });
     }
     dispatch.bindings.push_back({ match.residual_output->id, 0, match.residual_output->byte_count });
     if (match.has_rmsnorm) {

ggml/src/ggml-hrx/dispatch_registration/llm/dispatch-gated-delta-net.cpp

@@ -1036,6 +1036,26 @@ static bool match_gated_delta_net_rmsnorm_q8(const DispatchMatchContext & contex
     return true;
 }
 
+// engine #533: a value may only be taken over by a fused kernel when nothing outside the fused programme
+// reads it. A side reader (the copy that materialises the snapshot plane, a scale on the gate output) would
+// otherwise be scheduled before the fused kernel that writes it, and the command-program verifier rejects the
+// plan for reading a transient nothing has written yet.
+static bool consumers_covered_by(const Graph & graph, ValueId id, const std::vector<size_t> & first,
+                                 const std::vector<size_t> & second) {
+    for (const GraphNode * consumer : graph.index().consumers(id)) {
+        size_t index = 0;
+        if (!graph.index().node_index(consumer, index)) {
+            return false;
+        }
+        const bool covered = std::find(first.begin(), first.end(), index) != first.end() ||
+                             std::find(second.begin(), second.end(), index) != second.end();
+        if (!covered) {
+            return false;
+        }
+    }
+    return true;
+}
+
 static bool match_selected_gated_delta_net_dispatch(const DispatchMatchContext & context, DispatchMatch & result) {
     const Graph & graph = context.graph;
     const GraphNode * gather = context.root_node;
@@ -1159,6 +1179,14 @@ static bool match_selected_gated_delta_net_dispatch(const DispatchMatchContext &
         return false;
     }
     if (fuse_rms_q8) {
+        // The fused kernel writes the selected-snapshot plane and the cache; if a node outside this
+        // programme reads either, fusion would order that reader before the write (engine #533).
+        const Dispatch & candidate = selected.dispatches.back();
+        const DispatchBinding snapshot_binding = candidate.bindings[match.token_count == 1 ? 7 : 8];
+        if (!consumers_covered_by(graph, snapshot_binding.value, selected.covered_nodes, rms_match.covered_nodes) ||
+            !consumers_covered_by(graph, cache->id, selected.covered_nodes, rms_match.covered_nodes)) {
+            return false;
+        }
         Dispatch dispatch = std::move(selected.dispatches.back());
         const Dispatch & norm = rms_match.dispatches.front();
         const auto snapshot = dispatch.bindings[match.token_count == 1 ? 7 : 8];
@@ -1178,8 +1206,11 @@ static bool match_selected_gated_delta_net_dispatch(const DispatchMatchContext &
         dispatch.bindings.push_back(norm.bindings[1]);
         dispatch.bindings.push_back(norm.bindings[2]);
         dispatch.bindings.push_back(norm.bindings[4]);
-        selected.dispatches.clear();
-        selected.dispatches.push_back(std::move(dispatch));
+        // Replace only the dispatch this kernel supersedes: the earlier ones (the projections / gate
+        // producer) stay in the programme *before* the fused GDN. Clearing the list dropped them while
+        // their nodes stayed covered, so the value the fused kernel reads as raw_gate was written by a
+        // later command and the command-program verifier rejected the plan (engine #533).
+        selected.dispatches.back() = std::move(dispatch);
         selected.covered_nodes.insert(selected.covered_nodes.end(), rms_match.covered_nodes.begin(), rms_match.covered_nodes.end());
         selected.transients = std::move(rms_match.transients);
         if (!selected.metadata.append(std::move(rms_match.metadata), selected.status)) {

tests/hrx-backend-test.cpp

@@ -10232,11 +10232,23 @@ static void run_gdn_selected_rms_q8_dispatch_checks() {
             const size_t bytes = static_cast<size_t>(test.tokens * heads) * ggml_row_size(GGML_TYPE_Q8_1, width);
             const auto * alternate = plan.metadata.find_alternate_value(output->id, GGML_TYPE_Q8_1, bytes);
             REQUIRE(alternate != nullptr && alternate->alternate_value == fused->bindings.back().value);
+            // engine #533: the fused programme legitimately carries a copy for the selected-snapshot
+            // plane - it runs *before* the fused kernel and feeds one of its bindings - so the case checks
+            // that ordering instead of forbidding the copy. The fallback kernels stay forbidden.
             REQUIRE(std::none_of(plan.dispatches.begin(), plan.dispatches.end(), [](const auto & dispatch) {
                 const auto name = kernel_name_for_id(dispatch.kernel.kernel_id);
-                return name == "loom_libs:ggml_get_rows_f32" || name == "loom_libs:ggml_copy_f32" ||
-                       name == "loom_libs:ggml_rmsnorm_gate_f32_q8_1_x4";
+                return name == "loom_libs:ggml_get_rows_f32" || name == "loom_libs:ggml_rmsnorm_gate_f32_q8_1_x4";
             }));
+            const auto snapshot_copy =
+                std::find_if(plan.dispatches.begin(), plan.dispatches.end(), [](const auto & dispatch) {
+                    return kernel_name_for_id(dispatch.kernel.kernel_id) == "loom_libs:ggml_copy_f32";
+                });
+            if (snapshot_copy != plan.dispatches.end()) {
+                REQUIRE(snapshot_copy < fused);
+                const auto copied = snapshot_copy->bindings.back().value;
+                REQUIRE(std::any_of(fused->bindings.begin(), fused->bindings.end(),
+                                    [&](const auto & binding) { return binding.value == copied; }));
+            }
         }
         const auto commands = ggml::hrx::build_command_program(imported.graph, plan, ggml::hrx::get_qwen_kernel_corpus(), "gfx1151");
         REQUIRE(commands.valid());
@@ -11296,7 +11308,7 @@ int main() {
         { "gdn_rmsnorm_gate_dispatch", [] { run_gdn_rmsnorm_gate_dispatch_checks(); } },
         { "gdn_native_projection_pair_dispatch", [] { run_gdn_native_projection_pair_dispatch_checks(); } },
         { "gdn_selected_snapshot_dispatch", [] { run_gdn_selected_snapshot_dispatch_checks(); } },
-        { "gdn_selected_rms_q8_dispatch", [] { run_gdn_selected_rms_q8_dispatch_checks(); }, "#533" },
+        { "gdn_selected_rms_q8_dispatch", [] { run_gdn_selected_rms_q8_dispatch_checks(); } },
         { "llama_attention_matmul_dispatch", [] { run_llama_attention_matmul_dispatch_checks(); } },
         { "quantized_value_projection_dispatch", [] { run_quantized_value_projection_dispatch_checks(); } },
         { "schedule_qwen_terminal_q6k_q8_command.tokens1", [] { schedule_qwen_terminal_q6k_q8_command(1); } },
@@ -11306,7 +11318,7 @@ int main() {
         { "qwen_router_top8_dispatch", [] { run_qwen_router_top8_dispatch_checks(); }, "#445" },
         { "common_mul_mat_id_swiglu_dispatch", [] { run_common_mul_mat_id_swiglu_dispatch_checks(); } },
         { "qwen_routed_gate_up_dispatch", [] { run_qwen_routed_gate_up_dispatch_checks(); }, "#533" },
-        { "common_mul_mat_id_postops_dispatch", [] { run_common_mul_mat_id_postops_dispatch_checks(); }, "#533" },
+        { "common_mul_mat_id_postops_dispatch", [] { run_common_mul_mat_id_postops_dispatch_checks(); } },
         { "alias_value_import", [] { run_alias_value_import_checks(); } },
         { "multi_dispatch", [] { run_multi_dispatch_checks(); } },
         { "layout_alias_scheduler_elision", [] { run_layout_alias_scheduler_elision_checks(); } },