Skip to content

metal : fix moe_reduce fusion race when dst aliases its inputs - #30795

Merged
ggerganov merged 1 commit into
masterfrom
gg/metal-moe-reduce-alias
Oct 19, 2026
Merged

ggerganov merged 1 commit into
masterfrom
gg/metal-moe-reduce-alias

Conversation

@ggerganov

Copy link
Copy Markdown
Member

Overview

The fused MOE_REDUCE Metal kernel (kernel_moe_reduce_f32) computes dst = Σ experts[e]·weights[e] in a single dispatch, reading experts and weights while writing dst. The graph allocator computes tensor liveness on the unfused graph, where the inputs are fully consumed by earlier nodes of the fused group, so it can legally recycle the input memory as dst — in the fused kernel that becomes a threadgroup-vs-threadgroup race inside a single dispatch, producing rare non-deterministic wrong results.

Fix: reject the fusion at encode time when dst overlaps either input, using the existing ggml_metal_fusion_overlap helper — the same guard pattern as the MUL_MAT_ADD fusion. The unfused MUL + ADD path is always correct.

Requirements

The fused moe_reduce kernel computes dst = sum(experts[e] * weights[e]) in a
single dispatch, reading experts and weights while writing dst. The graph
allocator computes tensor liveness on the unfused graph, where the inputs are
fully consumed by earlier nodes of the fused group, so it can legally recycle
the input memory as dst. In the fused kernel this becomes a
threadgroup-vs-threadgroup race that produces rare non-deterministic wrong
results (observed with test-llama-archs -a kolibri on the meta backend).

Reject the fusion when dst overlaps either input, same guard as the
mul_mat_add fusion. The unfused MUL + ADD path is always correct.

Assisted-by: pi:llama.cpp/ggml-org/GLM-5.3-Flash-GGUF (2x DGX Spark (RDMA, TP=2))
@ggerganov ggerganov closed this Oct 19, 2026
@ggerganov
ggerganov deleted the gg/metal-moe-reduce-alias branch October 19, 2026 07:20
@ggerganov
ggerganov restored the gg/metal-moe-reduce-alias branch October 19, 2026 07:24
@ggerganov ggerganov reopened this Oct 19, 2026
@ggerganov
ggerganov marked this pull request as ready for review October 19, 2026 07:24
@ggerganov
ggerganov requested a review from a team as a code owner October 19, 2026 07:24
@am17an

am17an commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

Should this happen with add_alloc_deps? Or is it the graph optimize step is not called in this case (i.e. over RPC?)

@ggerganov

Copy link
Copy Markdown
Member Author

Exactly, the graph_optimize is not called with RPC and Meta backend - the failing test is using the latter path. I've tried before to enable the graph_optimize in the Meta backend, but IIRC there were some other non-trivial issues, so not an option for now.

@ggerganov
ggerganov merged commit 098ce52 into master Oct 19, 2026
32 of 52 checks passed
@ggerganov
ggerganov deleted the gg/metal-moe-reduce-alias branch October 19, 2026 08:55
@github-actions github-actions Bot added ggml changes relating to the ggml tensor library for machine learning Apple Metal https://en.wikipedia.org/wiki/Metal_(API) labels Oct 19, 2026
edwardyoon pushed a commit to edwardyoon/focus-llama that referenced this pull request Oct 24, 2026
…org#30795)

The fused moe_reduce kernel computes dst = sum(experts[e] * weights[e]) in a
single dispatch, reading experts and weights while writing dst. The graph
allocator computes tensor liveness on the unfused graph, where the inputs are
fully consumed by earlier nodes of the fused group, so it can legally recycle
the input memory as dst. In the fused kernel this becomes a
threadgroup-vs-threadgroup race that produces rare non-deterministic wrong
results (observed with test-llama-archs -a kolibri on the meta backend).

Reject the fusion when dst overlaps either input, same guard as the
mul_mat_add fusion. The unfused MUL + ADD path is always correct.

Assisted-by: pi:llama.cpp/ggml-org/GLM-5.3-Flash-GGUF (2x DGX Spark (RDMA, TP=2))
auRose94 pushed a commit to auRose94/llama.cpp that referenced this pull request Oct 28, 2026
…org#30795)

The fused moe_reduce kernel computes dst = sum(experts[e] * weights[e]) in a
single dispatch, reading experts and weights while writing dst. The graph
allocator computes tensor liveness on the unfused graph, where the inputs are
fully consumed by earlier nodes of the fused group, so it can legally recycle
the input memory as dst. In the fused kernel this becomes a
threadgroup-vs-threadgroup race that produces rare non-deterministic wrong
results (observed with test-llama-archs -a kolibri on the meta backend).

Reject the fusion when dst overlaps either input, same guard as the
mul_mat_add fusion. The unfused MUL + ADD path is always correct.

Assisted-by: pi:llama.cpp/ggml-org/GLM-5.3-Flash-GGUF (2x DGX Spark (RDMA, TP=2))
@BrewTestBot BrewTestBot mentioned this pull request Oct 28, 2026
1 task done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Apple Metal https://en.wikipedia.org/wiki/Metal_(API) ggml changes relating to the ggml tensor library for machine learning

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants