Repository navigation
refactor(stinkytofu): derive the WMMA refill line and rank prefetch above the WMMA queue - #14849
Merged
Merged
Conversation
wmmaRefillState() computes the refill state without side effects; wmmaRefillActive() records it. Prepares the refill as a ranked rule. Generated asm is identical on all 21 gfx1250 perf yamls. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…A refill Refill first (tried as an experiment) was 1.77% slower on mxf8_tn_medium, 12 of 12 interleaved reps, and changes no other perf yaml. Comment only. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ment chain Prefetch is L3 and the WMMA queue L4: Barrier, LastDs, Prefetch, WmmaQueue, DsPacer, Filler. A prefetch placed before the refill measured better (refill first: +1.77% on mxf8_tn_medium, 12 of 12 interleaved reps). No rule is registered at the WMMA queue level, so this reorders the enum and AGENTS.md and the rule table's sorted-order check, not the picks. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
WmmaQueueCoverCycles defaults to -1 (automatic): (WmmaQueueDepth - 1) x the WMMA latency, one WMMA short of a full queue, instead of the bare 56. An explicit value still wins and 0 is still off. A negative value used to mean off by accident of max(0, ...); it now means automatic. 56 is 7 x 8 on the 8-cycle f8 WMMAs; on the 4-cycle WMMAs the line is lower and changes no schedule. Generated asm is identical on all 21 gfx1250 perf yamls, ctest 1847 of 1847 (new test: automatic equals the hand-computed value). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (50.95%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #14849 +/- ##
===========================================
- Coverage 77.34% 77.34% -0.00%
===========================================
Files 2894 2894
Lines 487274 487274
Branches 73752 73752
===========================================
- Hits 376860 376859 -1
Misses 85525 85525
- Partials 24889 24890 +1
*This pull request uses carry forward flags. Click here to find out more. 🚀 New features to boost your workflow:
|
…tency The automatic line took the latency of the region's first WMMA, so a region that mixes 4- and 8-cycle WMMAs got 28 where a later 8-cycle WMMA needs 56. wmmaQueueCover() now takes the latency of the WMMA the refill is for, the same value that already sizes the refill target. The on/off tests use wmmaQueueOn() (depth above 1), which is what wmmaQueueCover() > 0 meant. The scheduler's other uses of the region latency (barrier thresholds, WAR gate distance, pacer windows) still assume one WMMA latency per region. All 103 kernels checked (21 perf yamls, 82 stream-K tests) use a single WMMA mnemonic. Generated asm is identical on all 21 perf yamls; ctest 1850 of 1850 with a new test for the line. The production defaults in scheduler-tuning-parameters.md read (8, -1, 6), not (8, 56, 6): 56 is only the 8-cycle value.
KKyang
marked this pull request as ready for review
November 2, 2026 04:39
hcman2
approved these changes
Nov 2, 2026
Contributor
TheRock Submodule Bump ActivityNewest first
|
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.
JIRA ID : AIHPBLAS-5438
Motivation
Three loose ends in how the gfx1250 CDNA5 DAG scheduler places the WMMA refill (the WMMA that tops up the matrix pipe's queue when too little work is queued):
wmmaRefillActive()both read the refill state and changed it, so nothing could ask "is a refill due?" without side effects.AGENTS.mdranked the refill (L3) above prefetch (L5), but the code lets a promoted prefetch go first, and that is the better order (measured below).WmmaQueueCoverCycles = 56, with no stated source.Technical Details
Five commits, each a refactor, a comment or a small fix: the generated gfx1250 asm is identical to the commit before it on all 21 perf yamls.
Split the refill state read from its commit.
wmmaRefillState()computes the state without side effects;wmmaRefillActive()records it.Comment. A promoted prefetch goes before the refill.
Rank prefetch above the WMMA queue.
PlacementLevelis now Barrier, LastDs, Prefetch, WmmaQueue, DsPacer, Filler, and theAGENTS.mdchain table and consequences follow (prefetch L3, WMMA queue L4). No rule is registered at the WMMA queue level, so this changes the enum order and the docs, not the picks.Derive the refill line.
WmmaQueueCoverCyclesdefaults to -1, meaning automatic:(WmmaQueueDepth - 1) xthe latency of the WMMA the refill is for, one WMMA short of a full queue. That is 56 on the 8-cycle f8 WMMAs (the old constant) and 28 on the 4-cycle ones (mxf4, nvf4, bbs), where it changes no schedule. An explicit positive value still wins and 0 is still off. A negative value used to mean off by accident ofmax(0, ...); it now means automatic. A new unit test checks that automatic equals the hand-computed value.Size the line by the next WMMA. The first version took the latency of the region's first WMMA, so a region mixing 4- and 8-cycle WMMAs would get 28 where an 8-cycle WMMA needs 56.
wmmaQueueCover()now takes the latency of the WMMA the refill is for, the value that already sizes the refill target, and a unit test checks 28 and 56. The scheduler's other uses of the region latency (barrier thresholds, WAR gate distance, pacer windows) still assume one WMMA latency per region; all 103 kernels I checked (21 perf yamls, 82 stream-K tests) use one WMMA mnemonic. The production defaults inscheduler-tuning-parameters.mdnow read (8, -1, 6).Not in this PR. The line
WmmaQueueDepth xlatency (64 and 32 cycles) changes the asm on 10 MAF yamls. On eight MAF yamls it measured neutral (every mean between -0.13% and +0.09%, 6 interleaved repetitions), so it is not used. Refill first, ahead of a promoted prefetch, measured 1.77% slower onmxf8_tn_medium(12 of 12 repetitions), the only yaml it changes.Test Plan
StinkyTofu unit and FileCheck suites. Asm comparison against develop on the 21 gfx1250 perf yamls (maf, mab, medium). Hardware A/B for the variants that changed asm (not adopted).
Test Result
Unit and FileCheck tests: 1850 of 1850 pass on the current develop base. Generated asm: identical to develop on all 21 gfx1250 perf yamls (maf, mab, medium), so this PR does not change the generated code. Performance results are recorded in the JIRA ticket for this work, not in this description.
Status
Draft.
Submission Checklist