Skip to content

feat(stinkytofu): wmma reorder pass with cross-iteration ds_load prefetch migration - #11478

Draft
KKyang wants to merge 5 commits into
developfrom
users/kkyang/wmma-reorder
Draft

feat(stinkytofu): wmma reorder pass with cross-iteration ds_load prefetch migration#11478
KKyang wants to merge 5 commits into
developfrom
users/kkyang/wmma-reorder

Conversation

@KKyang

@KKyang KKyang commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Summary

A new mechanism for changing the wmma issue pattern of an unrolled GEMM loop, and repairing the LDS software pipeline that the new pattern invalidates.

The same wmma are issued either way — only the order changes — but that reorder moves which ds_loads need a head start. Some loads must now be issued an iteration early and some no longer need to be, so prefetches appear in and disappear from the loop preheader.

Default off. No kernel changes without EnableWmmaReorder.

Design

The order comes from a swappable mode. IWmmaOrderProvider is the extension point; each reordering method is one implementation over shared rewrite machinery. Three ship:

mode order
VgprAnalysisOrderProvider (default) from analyzeWmmaVgprReorder — minimize VGPR pressure
ExplicitOrderProvider a caller-supplied permutation, for tuning sweeps
ReverseOrderProvider reversed; exercises the machinery with no tuner attached

Two invariants keep the rewrite honest.

  1. The loop body is only ever permuted — no instruction is created or destroyed there, so the kernel still issues exactly the same work per iteration.
  2. Everything added or removed happens in the preheader, where the pipeline's iteration-0 prefetches live. A ds_load whose new slot falls before the top of the body gains a preheader clone and has its body copy retuned to the next iteration; one that no longer needs the head start loses its preheader copy and is de-rotated.

The per-iteration LDS stride is never invented. It is read off the preheader/body ds_load pairs the kernel already has (same dest VGPR group, same address operands, offset difference). With no such pair the pass cannot know how the kernel rotates buffers, so cross-iteration migration is disabled and the body is permuted in place.

One hard safety rule. A wrapped load issues for the next iteration, so it must sit after every wmma that reads its registers this iteration — not just the first. The requested prefetch distance yields to that. A loop where an already-prefetched load has no legal slot is left untouched rather than silently corrupted.

Worked example

Four wmma over A0..A3, arriving pipelined for order (A0,A1,A2,A3) at distance 2, so A0/A1 are prefetched. Reversing to (A3,A2,A1,A0):

before after
preheader A0, A1 prefetches A2, A3 prefetches
A0/A1 body offsets 512, 528, 544, 560 0, 16, 32, 48 (de-rotated)
A2/A3 body offsets 64, 80, 96, 112 576, 592, 608, 624 (rotated)

prefetch +4/-4, with the 512-byte stride discovered from the input. This is tests/filecheck/wmma_reorder_cross_iter.stir.

Why this slot in the pipeline

WMMA is the scheduling anchor, so the pattern has to be final before the DAG runs. scheduleInDAG builds wmmaIndex by walking the block in program order, and the dsReadPriority pre-scan gives every ds_read the index of its earliest wmma consumer. Moving a wmma after the DAG has run would leave those priorities describing an order that no longer exists.

Running immediately before StinkyBuildImplicitDependencyPass also puts the pass after StinkyRemoveWaitCntPass, so no leftover s_waitcnt chops the window the reorder is allowed to move in.

The in-body ds_load placement this pass produces is therefore a seed order — the DAG re-places local reads against its own model. What survives to the kernel is the wmma order plus the preheader add/remove, which the DAG cannot do at all since it never crosses the back-edge. The seed is safe under the DAG because buildRegisterDependencyDAG emits WAR edges, so a wrapped load stays after the wmma reading its registers.

Commits

  1. refactor — lift the wmma vgpr reorder analysis out of its pass into analyzeWmmaVgprReorder(), so a mode can compute its own input instead of depending on a pipeline neighbour and a file-static map. No behaviour change.
  2. feat — the pass, its modes and its tests. Also adds mod.wmma_pool serialization (WmmaPoolData had no .stir round-trip, so the vgpr-analysis mode was untestable) and a key=value argument helper for stinkytofu-opt.
  3. feat — the EnableWmmaReorder flag and the gfx1250 wiring.

Testing

ctest 1138/1138 on gfx1250. New coverage:

  • tests/filecheck/wmma_reorder_cross_iter.stir — the worked example above, checking both the preheader swap and the rewritten body.
  • tests/filecheck/wmma_reorder_no_pipeline.stir — a loop with no existing prefetch: no stride is available, so nothing crosses the back-edge and the body is permuted in place.
  • tests/unit/asm/StinkyWmmaReorderPassTest.cpp — 7 tests over the mode ABI, cross-iteration migration, the body-is-a-permutation invariant, and the refusals.
./build/bin/stinkytofu-opt --arch gfx1250 kernel.stir \
  --StinkyWmmaReorderPass=reverse,distance=2 --print-output

Not in this PR

  • No distance knob. prefetchDistance is derived from ds_load latency over wmma issue rate. Worth flagging: it is the value that decides which loads wrap, so it is the one worth sweeping. One more X(...) row when wanted.
  • Enabling the flag is a no-op on today's kernels. The default mode declines any block whose wmma carry no WmmaPoolData, which TensileLite does not yet emit. The wiring is ready for when it does, or for a sweep driving ExplicitOrderProvider.
  • No hard wmma-order constraint in the DAG. The order is honoured through DAG-id tie-breaking, but the scheduler may still hoist a later wmma past one stalled on data. Left alone deliberately until we measure whether that matters.

🤖 Generated with Claude Code

KKyang added 3 commits August 30, 2026 20:16
…tion

The analysis lived only inside StinkyWmmaVgprReorderPass and published its
results through a file-static map, so any consumer had to depend on that pass
having run earlier in the same pipeline.

Lift the body into analyzeWmmaVgprReorder(bb[, liveness, algorithm]). The pass
becomes a thin wrapper over it, keeping getWmmaReorderResult() and its unit
tests unchanged, while callers that only need one block's order can compute it
themselves.

No behaviour change.
Rewrites an unrolled-loop body to a new wmma issue order and re-places the
ds_loads that feed it, including across the loop back-edge.

The order comes from a swappable mode (IWmmaOrderProvider) so each reordering
method is one implementation over shared rewrite machinery: VgprAnalysisOrder
(minimize VGPR pressure), ExplicitOrder (a permutation, for tuning sweeps) and
ReverseOrder (exercises the machinery without a tuner).

Two invariants keep the rewrite honest. The loop body is only permuted, so the
kernel still issues the same work per iteration. Everything added or removed
happens in the preheader, where the software pipeline's iteration-0 prefetches
live: a ds_load whose new slot falls before the top of the body gains a
preheader clone and has its body copy retuned to the next iteration, and one
that no longer needs the head start loses its preheader copy and is de-rotated.

The per-iteration LDS stride is never invented. It is read off the
preheader/body ds_load pairs the kernel already has; with no such pair,
cross-iteration migration is disabled and the body is permuted in place.

A wrapped load must issue after every wmma that reads its registers this
iteration, not just the first, or it clobbers them. The requested prefetch
distance yields to that rule, and a loop where an already-prefetched load has
no legal slot is left untouched rather than silently corrupted.

Also adds mod.wmma_pool serialization, without which WmmaPoolData had no .stir
round-trip and the vgpr-analysis mode could not be tested, and a key=value
argument helper for stinkytofu-opt.
…mmaReorder

Adds the ModuleOptions flag, default false, and runs the pass in
addGfx1250RegionPasses immediately before StinkyBuildImplicitDependencyPass.

That slot is required, not incidental. WMMA is the scheduling anchor:
scheduleInDAG builds wmmaIndex by walking the block in program order and the
dsReadPriority pre-scan gives every ds_read the index of its earliest wmma
consumer, so moving a wmma after the DAG has run would leave those priorities
describing an order that no longer exists. Running here also puts the pass
after StinkyRemoveWaitCntPass, so no leftover s_waitcnt chops the window the
reorder is allowed to move in.

The flag stays off, so no kernel changes. With it on, today's kernels are still
unaffected: the default mode declines any block whose wmma carry no
WmmaPoolData, which TensileLite does not yet emit.
@therock-pr-bot

Copy link
Copy Markdown

❌ PR Check — Action Required

Check Status Details
📝 PR Description ❌ Fail Error: PR description must reference a JIRA ID, ISSUE ID, or a GitHub closing keyword.
Expected: include a JIRA ID / ISSUE ID line (separator : or -, or omitted; value may be a JIRA key, a number with/without #, or a link), OR a closing keyword + issue reference. Accepted examples:
JIRA ID : TESTAUTO-6039
JIRA ID - #330
JIRA ID #330
JIRA ID (on separate line)
ROCM-25757
ISSUE ID : TESTUTO-3334
ISSUE ID #3334
ISSUE ID - TESTAUTO-3433
ISSUE ID (on separate line)
AIRUNTIME-2352
ISSUE ID : https://github.com/<org_name>/<repo_name>/issues/1234
Closes #10
Fixes octo-org/octo-repo#100
Resolves: #123
#123
https://github.com/<org_name>/<repo_name>/issues/123
Current: no valid JIRA/ISSUE/closing-keyword reference found
Forbidden Files ✅ Pass
🧪 Unit Test ⚠️ Warning Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: shared/stinkytofu/include/stinkytofu/bindings/python/Module.hpp, shared/stinkytofu/include/stinkytofu/transforms/asm/StinkyWmmaReorderPass.hpp, shared/stinkytofu/include/stinkytofu/transforms/asm/StinkyWmmaVgprReorderPass.hpp, shared/stinkytofu/src/pipeline/backend/Gfx1250Backend.cpp, shared/stinkytofu/src/serialization/asm/ModifierSerializer.cpp (+4 more); no test file found
🔎 pre-commit ✅ Pass
🚫 Draft PR 🔜 To Be Enabled
🚩 Feature Flag 🔜 To Be Enabled
📊 Code Coverage 🔜 To Be Enabled

⚠️ 1 policy check(s) failed. Please address the issues above before this PR can be Reviewed.

🚫 Please fix the failed policies

  • ❌ PR Description

The Not ready to Review label was added to this PR. Once all policies pass, the label is removed automatically.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ PR Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

@codecov

codecov Bot commented Aug 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

❌ Your project check has failed because the head coverage (38.33%) 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   #11478   +/-   ##
========================================
  Coverage    69.33%   69.33%           
========================================
  Files         2810     2810           
  Lines       465293   465293           
  Branches     68653    68653           
========================================
  Hits        322565   322565           
  Misses      119189   119189           
  Partials     23539    23539           
Flag Coverage Δ *Carryforward flag
TensileLite-CPP 38.33% <ø> (ø)
TensileLite-Unit 76.15% <ø> (ø)
hipBLAS 90.62% <ø> (ø) Carriedforward from bcdb9f8
hipBLASLt 35.22% <ø> (ø)
hipCUB 82.68% <ø> (ø) Carriedforward from bcdb9f8
hipDNN 86.85% <ø> (ø) Carriedforward from bcdb9f8
hipFFT 43.78% <ø> (ø) Carriedforward from bcdb9f8
hipRAND 76.12% <ø> (ø) Carriedforward from bcdb9f8
hipSOLVER 69.03% <ø> (ø) Carriedforward from bcdb9f8
hipSPARSE 86.99% <ø> (ø) Carriedforward from bcdb9f8
rocBLAS 48.29% <ø> (ø) Carriedforward from bcdb9f8
rocFFT 46.60% <ø> (ø) Carriedforward from bcdb9f8
rocRAND 56.91% <ø> (ø) Carriedforward from bcdb9f8
rocSOLVER 76.83% <ø> (ø) Carriedforward from bcdb9f8
rocSPARSE 74.60% <ø> (ø) Carriedforward from bcdb9f8
rocThrust 91.60% <ø> (ø) Carriedforward from bcdb9f8

*This pull request uses carry forward flags. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

KKyang added 2 commits August 31, 2026 14:54
getTerminator() returns null once every existing prefetch has been
de-rotated out of a preheader, and dyn_cast doesn't tolerate a null
argument -- classof dereferences it directly. Reordering a loop whose
wmma pattern fully swaps which loads are prefetched (e.g. reverse on a
kernel where the preheader holds only prefetches) hits this and
segfaults intermittently, since which garbage classof reads back
depends on heap layout.

Found while stress-testing StinkyWmmaReorderPass under AddressSanitizer.
A wmma order is free to place any two ds_loads in any relative order,
but it can't invent extra physical registers: when a segment
double-buffers one RegGroup across more than one producer (ordinary
software pipelining, not just the back-edge case), the new order can
make two producers' live windows overlap on the same physical register.

Detect this generally -- not just the existing wrapped-load-vs-its-own-
twin check -- by tracking each ds_load's exact consumer set and original
body position, scoping consumer attribution per producer so a shared
register's reuses aren't all attributed to every producer indiscriminately.
When two producers on one RegGroup collide, the later one's def, matched
consumers, and preheader twin are retargeted together to a placeholder
register (StinkyRegister::Virtual) instead of silently clobbering the
earlier value or bailing the whole loop. Final physical assignment is
left to register allocation.
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.

1 participant