Repository navigation
glm5-next: fix indexer scatter data race in the sparse mask - #29745
Merged
Merged
Conversation
The sparse indexer mask is built with a set_rows scatter. Padded pools, absent sequences and missing tail cells all pointed to the same n_kv sentinel row, and invisible pools picked by top_k to fill the selection overlap the tail cells of the token, so several CPU threads wrote the same element (ThreadSanitizer data race in the sanitize CI). Allocate the slot mask for both selection paths and route every dead slot to its own dump row n_kv + slot. Live slots address disjoint cells, so the scatter indices of a token are unique.
Member
|
Is the double cast necessary? As in, you cast to F32, perform arithmetic (mul/add), then cast back to I32 - can't the arithmetic be done in I32 from the start? |
Member
That might be useful, but nope, not supported. |
Contributor
Author
|
Yes and the round trip is exact since the indices stay far below 2^24, the same function already does it for the top_k reordering. |
am17an
approved these changes
Sep 30, 2026
pwilkin
approved these changes
Sep 30, 2026
Merged
edwardyoon
pushed a commit
to edwardyoon/focus-llama
that referenced
this pull request
Oct 7, 2026
The sparse indexer mask is built with a set_rows scatter. Padded pools, absent sequences and missing tail cells all pointed to the same n_kv sentinel row, and invisible pools picked by top_k to fill the selection overlap the tail cells of the token, so several CPU threads wrote the same element (ThreadSanitizer data race in the sanitize CI). Allocate the slot mask for both selection paths and route every dead slot to its own dump row n_kv + slot. Live slots address disjoint cells, so the scatter indices of a token are unique. (cherry picked from commit 05af0d2)
gagallo7
pushed a commit
to tetherto/qvac-fabric-llm.cpp
that referenced
this pull request
Oct 14, 2026
The sparse indexer mask is built with a set_rows scatter. Padded pools, absent sequences and missing tail cells all pointed to the same n_kv sentinel row, and invisible pools picked by top_k to fill the selection overlap the tail cells of the token, so several CPU threads wrote the same element (ThreadSanitizer data race in the sanitize CI). Allocate the slot mask for both selection paths and route every dead slot to its own dump row n_kv + slot. Live slots address disjoint cells, so the scatter indices of a token are unique. (cherry picked from commit 05af0d2) Assisted-by: Claude Opus 5.5
JenySadadia
pushed a commit
to tetherto/qvac-fabric-llm.cpp
that referenced
this pull request
Oct 22, 2026
ggml-backend-meta: mark ARANGE as mirrored ARANGE has no sources, so handle_generic returned UNKNOWN and aborted once glm5-next (ggml-org#29745 dump-row indices) ran under tensor split. Its output depends only on op params, so it is the same on every device. Assisted-by: Claude Opus 5.5 Signed-off-by: Jeny Sadadia <jeny.sadadia@collabora.com>
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.
Overview
The sparse indexer mask is built by a set_rows scatter that the CPU backend splits across threads, so the indices of a token must be unique. Here they were not: padded pools, absent sequences and missing tail cells all shared the n_kv sentinel row, and invisible -inf pools picked by top_k overlapped the tail cells of the token. Same value written, but still a real race, and with multiple streams those pools also unmasked cells of the wrong stream.
Fix: reuse the host slot mask of the gather path in the scatter path too, every dead slot goes to its own dump row n_kv + slot. No host change, the mask only grows by n_sel rows.
Tested with the same CPU only TSan build as the CI: 36 races before, 0 after, full ctest -L main green, CUDA passes too.
Additional information
Follow-up to #27773, fixes a ThreadSanitizer data race on the glm5-next dummy model in the sanitize CI (CPU only build): https://github.com/ggml-org/llama.cpp/actions/runs/36695129904/job/109821149292
Requirements