Skip to content

fix(stinkytofu): keep one split-barrier signal in flight per barrier id in the CDNA5 DAG [bkc 20261027] - #14863

Merged
ScottTodd merged 1 commit into
ROCm:release/bkc/therock-10.3-20261027from
akankjai:users/akankjai/cp-26.14-stinkytofu-barrier
Nov 2, 2026
Merged

ScottTodd merged 1 commit into
ROCm:release/bkc/therock-10.3-20261027from
akankjai:users/akankjai/cp-26.14-stinkytofu-barrier

Conversation

@akankjai

@akankjai akankjai commented Nov 2, 2026 •

Copy link
Copy Markdown

Cherry-picks:

Motivation

Cherry-pick request for the BKC 26.14 release branch (release/bkc/therock-10.3-20261027).

gfx1250 split barriers (s_barrier_signal / s_barrier_wait) hold one signal per barrier id in flight: after a signal, the next barrier on that id must be a wait. In the CDNA5 DAG, barrier pairs only get edges through their own LDS tokens, so the scheduler can nest two pairs on different tokens (signal A, signal B, wait B, wait A on the same id), which can race or hang. The fix makes the scheduler track open barrier ids and reject any non-wait barrier on an id that is still open.

Technical Details

Issue Tracking

JIRA ID: AIHPBLAS-5375

Test Plan / Result

Validated on develop in the original PR (#14636): the nested-pair filecheck test fails on the old scheduler and passes with the fix, the affected kernels go from 3 nested sites to 0 with unchanged signal/wait counts, and stinkytofu ctest passed 1810/1810. No additional testing was done on the release branch.

Review

@KKyang is the owner of the original change; please review this cherry-pick.

🤖 Generated with Claude Code

…id in the CDNA5 DAG (ROCm#14636)

JIRA ID : AIHPBLAS-5375

gfx1250 split barriers (`s_barrier_signal` / `s_barrier_wait`) hold one
signal per barrier id in flight: after a signal, the next barrier on
that id must be a wait. In the CDNA5 DAG, barrier pairs only get edges
through their own LDS tokens, so two pairs on different tokens have no
edge between them and the scheduler can nest them:

```
s_barrier_signal -1     (pair A)
...
s_barrier_signal -1     (pair B)   <- second signal while A is open
s_barrier_wait   -1     (pair B)
...
s_barrier_wait   -1     (pair A)
```

The two gfx1250 bf16 TN StreamK solutions removed in ROCm#14540
(`MT256x256x64 ... MIWT16_4 ... WG16_8_1`, `MT320x256x64 ... MIWT20_4
... WG16_8_1`) have three such sites each.

- The scheduler tracks the open barrier ids: a signal opens its id, a
wait on it closes it.
- While an id is open, `isBarrierEligibleNow()` rejects every barrier on
that id that is not a wait. The ready wait is that signal's own, since
it shares the signal's tokens. Other ids are free, so `-1` pairs inside
an open `-3` cluster signal stay legal.
- The barrier phase's drain path now uses `isBarrierEligibleNow()` like
the promoted and cluster paths, so no path bypasses the rule.
- If only blocked barriers remain, `pickOne()` stops with an error
instead of emitting the wrong order.

- `raw_asm_token_annotation.s` feeds nested pairs; its checks now
require them to alternate. It fails on the old scheduler.
- The two ROCm#14540 kernels, regenerated from the pre-ROCm#14540 logic file:
nested sites 3 -> 0 in each, with the same 52 signals and 52 waits.
- stinkytofu ctest 1810/1810.

(cherry picked from commit bcf5892)
@akankjai

akankjai commented Nov 2, 2026

Copy link
Copy Markdown
Author

@KKyang, this is the cherry-pick of your #14636 into the 26.14 snap branch (release/bkc/therock-10.3-20261027). Could you please review it? The only difference from the original is the #include block in CDNA5.hpp (only <set> is added; the release branch has no <span>).

@assistant-librarian assistant-librarian Bot added the external contribution Code contribution from users community.. label Nov 2, 2026
@akankjai

akankjai commented Nov 2, 2026

Copy link
Copy Markdown
Author

@KKyang @hcman2, since you are the author and the approving reviewer of the original change (#14636), could one of you please approve this cherry-pick on GitHub? It targets the 26.14 branch (release/bkc/therock-10.3-20261027) and is the only remaining request for the new 26.14 build; DevOps needs an approval here before they can merge it, pull in the submodule and trigger the build. The diff is the same as #14636 except for the #include block in CDNA5.hpp (only <set> is added, the release branch has no <span>).

@ScottTodd
ScottTodd merged commit 9ca7988 into ROCm:release/bkc/therock-10.3-20261027 Nov 2, 2026
32 of 38 checks passed
ScottTodd added a commit to ROCm/TheRock that referenced this pull request Nov 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants