Skip to content

fix(tensilelite): Only defer FUSI next-loop local reads on sub-tile overlap - #12886

Merged
mihnea-chirila merged 7 commits into
developfrom
users/mchirila/bufferload0_regression_fix
Oct 8, 2026
Merged

mihnea-chirila merged 7 commits into
developfrom
users/mchirila/bufferload0_regression_fix

Conversation

@mihnea-chirila

@mihnea-chirila mihnea-chirila commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

JIRA ID : ROCM-31274

Motivation

PR #6944 (4068136, "feat(tensilelite): Added Global Read/Write Support") regressed large FP8 TN GEMMs on gfx950 by ~5.8%. The regression also affects BF16 and F16 ForceUnrollSubIter (FUSI) kernels.

The cause is in _makeSubIterSchedule. #6944 defers all next-loop local reads until after the MFMAs for every non-F32X FUSI kernel with a single VGPR buffer (numVgprBuffer == 1). The deferral is only needed when the reads would overwrite registers that are still in use. For FP8/BF16/F16 kernels they don't, so the scheduler gives up local-read/MFMA overlap for nothing.

Technical Details

With FUSI, each operand's X0 buffer holds two sub-tiles. During the last quadrant, the next-loop local reads refill sub-tile 0 while the MFMAs still read sub-tile 1.

  • Real hazard (SGEMM/CGEMM in bufferLoadStore0.yaml / cgemm.yaml). With TransposeLDS=0 and VectorWidth=4, one ds_read_b128 writes ValuA_X0_I0+0:3, but a sub-tile is only 2 VGPRs wide. The read clobbers sub-tile 1 before the last MFMAs consume it (WAR). Without the deferral these kernels fail validation (NaNs).
  • No hazard (shipped FP8/BF16/F16 FUSI kernels). lrvwTile == 1 (unroll-major LDS or VectorWidth == 1), so each read stays inside one sub-tile. In the FP8 TN kernel, for example, the early reads write A+0..15 / B+0..71 while the last MFMAs read A+16..31 / B+72..143.

This PR gates the deferral on that overlap:

def _nextLoopReadSpansSubTiles(lrvwTile, waveTile):
  return lrvwTile > waveTile // kernel["numSubTiles"]
deferNextLoopReadsWAR = kernel["ForceUnrollSubIter"] and self.states.numVgprBuffer == 1 \
                        and not kernel["UseF32XEmulation"] \
                        and (_nextLoopReadSpansSubTiles(self.states.lrvwTileA, kernel["MIWaveTile"][0]) \
                             or _nextLoopReadSpansSubTiles(self.states.lrvwTileB, kernel["MIWaveTile"][1]))

It also removes the sync1LdsMfmaIndex override that #6944 added to the SIA3 non-scheduled-local-write branch. It produces identical code for the #6944 test and can't apply to the regressed kernels (they all use ScheduleLocalWrite=1 with PGR >= 1).

It also drops the iteration > isBarrier suppression that #6944 added to the SIA3 path. With ForceUnrollSubIter, numItersPLR is 1, so isBarrier is the last iteration unless doFullPackCodePrefetch moves it. That needs F32X emulation, which the gate excludes, so the branch could never be taken.

Scope: of the 1,488 FUSI solutions in asm_full logic (all gfx950), about 1,220 are FP8, 54 BF16/F16 and 215 F32X. None of the non-F32X ones meet the overlap condition, so all of them get the pre-#6944 schedule back.

Test Plan

All runs on one gfx950 GPU (HIP_VISIBLE_DEVICES=2).

  • test_config.py with bufferLoadStore0.yaml and cgemm.yaml (the feat(tensilelite): Added Global Read/Write Support #6944 tests).
  • A minimal SGEMM FUSI config (MT128x128x4, MI16x16x4, DU4, VW4, PLR1, SIA1/SIA3) at 128x128x2x64 and 127x127x2x63, full validation. This config reproduces the WAR hazard.
  • Tensile/Tests/unit, including the new CPU-only codegen test test_fusi_next_loop_read_war.py. It emits SGEMM FUSI kernels (MIWaveTile 4x4, SIA1/SIA3, VectorWidthA/B in {2, 4}) and checks the last sub-iteration of the main loop: no next-loop local read overwrites a register that a later MFMA still reads, and the reads are deferred past the last MFMA exactly when one read spans more than a sub-tile. The test fails with feat(tensilelite): Added Global Read/Write Support #6944's original condition (2 cases) and with a data-type-only gate (12 cases).
  • Benchmarks of FUSI solutions taken from gfx950 logic files. For each, the generated assembly (pre-loop, main loop, post-loop) is diffed against the pre-feat(tensilelite): Added Global Read/Write Support #6944 parent (ed5614bf1c6), and the kernel is timed.

Test Result

  • bufferLoadStore0.yaml and cgemm.yaml pass.
  • The SGEMM FUSI repro: 4/4 pass (a data-type-only gate fails 4/4).
  • Unit: 10260 passed, 0 failed (786 codegen snapshots unchanged).

Performance as relative change in kernel time (negative is faster). Each kernel has a main loop byte-identical to the parent's:

Kernel Problem #6944 vs pre-#6944 This PR vs pre-#6944 This PR vs #6944
FP8 TN (Origami logic, sol 15) 2112x16384x7168 +4.6% +0.3% -4.1%
BF16 TN (Equality logic, sol 2132) 2432x8192x2432 +5.2% -0.5% -5.4%
BF16 NN (Equality logic, sol 743) 3840x5120x3840 +1.8% -2.7% -4.4%
F16 NT (Origami logic, sol 197) 8192x8192x8192 +4.7% +0.1% -4.4%

F16 NT sol 197 in detail:

Separate, unrelated regression found during validation (not fixed here)

On current develop, F16 NT sol 197 is still ~3.4% slower than the pre-#6944 parent. A git bisect (9 steps, the #6944 fix applied where needed) points to b20a30d (#11948, "feat(origami): Update Origami Model to Select Across More Kernel Parameters", AIHPBLAS-1926). The commit is 3.6% slower than its parent 524afdea981, with byte-identical kernel code. The change is in the host-side StreamK grid selection:

  • correct_sk_grid_for_partial_tiles (shared/origami/src/origami/streamk.cpp) now replaces the SK grid of 238 with a DP grid of 832.
  • That problem has 832 tiles, 256 iters/tile and 256 CUs, so DP_eff = 81.25%, just over the 80% threshold.
  • A forced grid shows the cost on current develop + this PR, F16 NT 8192^3. Kernel time relative to the old SK grid (238) in the same run:
TENSILE_STREAMK_FIXED_GRID Run 1 Run 2
238 (old SK choice) baseline baseline
832 (new DP choice) +3.7% +6.0%
default +3.6% +3.8%

This belongs to the Origami heuristic owners and should be tracked separately. This PR doesn't change it.

Submission Checklist

…verlap

PR #6944 deferred all ForceUnrollSubIter next-loop local reads past the
MFMAs for every non-F32X kernel with a single VGPR buffer. The deferral is
only needed when one local read spans more tile elements than a sub-tile
(lrvwTile > MIWaveTile/numSubTiles): that read then overwrites sub-tile 1
registers the last MFMAs still read. FP8/BF16/F16 FUSI kernels use
lrvwTile == 1, so they paid the serialization cost without a hazard
(~3-6% slowdown, e.g. FP8 TN 2112x16384x7168).

Gate the deferral on the actual overlap, which restores the pre-#6944
schedule for those kernels and keeps the deferral for the SGEMM/CGEMM
kernels in bufferLoadStore0.yaml and cgemm.yaml.

Also drop the redundant sync1LdsMfmaIndex override added to SIA3.

Fixes ROCM-31274
@therock-pr-bot

therock-pr-bot Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

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

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@codecov

codecov Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #12886      +/-   ##
===========================================
- Coverage    71.93%   70.92%   -1.01%     
===========================================
  Files         2863     2946      +83     
  Lines       472710   485747   +13037     
  Branches     69829    70150     +321     
===========================================
+ Hits        340010   344489    +4479     
- Misses      109117   117302    +8185     
- Partials     23583    23956     +373     
Flag Coverage Δ *Carryforward flag
TensileLite-CPP 49.32% <ø> (-0.01%) ⬇️
TensileLite-Unit 83.30% <100.00%> (+0.01%) ⬆️
hipBLAS 90.62% <ø> (ø) Carriedforward from 97cff0e
hipBLASLt 38.91% <ø> (ø)
hipCUB 82.68% <ø> (ø) Carriedforward from 97cff0e
hipDNN 86.64% <ø> (+<0.01%) ⬆️ Carriedforward from 97cff0e
hipFFT 54.32% <ø> (+10.89%) ⬆️ Carriedforward from 97cff0e
hipRAND 76.12% <ø> (ø) Carriedforward from 97cff0e
hipSOLVER 68.96% <ø> (ø) Carriedforward from 97cff0e
hipSPARSE 86.99% <ø> (ø) Carriedforward from 97cff0e
rocBLAS 48.24% <ø> (-0.01%) ⬇️ Carriedforward from 97cff0e
rocFFT 47.11% <ø> (-0.51%) ⬇️ Carriedforward from 97cff0e
rocRAND 57.42% <ø> (-0.03%) ⬇️ Carriedforward from 97cff0e
rocSOLVER 76.83% <ø> (+<0.01%) ⬆️ Carriedforward from 97cff0e
rocSPARSE 74.75% <ø> (+0.12%) ⬆️ Carriedforward from 97cff0e
rocThrust 78.45% <ø> (-14.70%) ⬇️ Carriedforward from 97cff0e

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

Files with missing lines Coverage Δ
...ts/hipblaslt/tensilelite/Tensile/Components/SIA.py 91.08% <ø> (+0.22%) ⬆️
...ects/hipblaslt/tensilelite/Tensile/KernelWriter.py 84.62% <100.00%> (+0.10%) ⬆️

... and 171 files with indirect coverage changes

🚀 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.

mihnea-chirila and others added 3 commits October 2, 2026 09:03
Add a CPU-only codegen test that emits SGEMM ForceUnrollSubIter kernels
(MIWaveTile 4x4, SIA1/SIA3, VectorWidthA/B in {2, 4}) and checks the last
sub-iteration of the main loop:
- no next-loop local read overwrites a register a later MFMA still reads;
- the reads are deferred past the last MFMA exactly when one read spans
  more than a sub-tile (VectorWidth 4), and interleaved otherwise.

Drop the `iteration > isBarrier` suppression in the SIA3 path: with
ForceUnrollSubIter numItersPLR is 1, so isBarrier is the last iteration
unless doFullPackCodePrefetch moves it, which requires F32X emulation and
is excluded by the gate. The branch could never be taken.
@mihnea-chirila
mihnea-chirila marked this pull request as ready for review October 2, 2026 20:31
@mihnea-chirila
mihnea-chirila requested a review from a team as a code owner October 2, 2026 20:31
@mihnea-chirila
mihnea-chirila requested a review from b-shi October 5, 2026 19:13

@b-shi b-shi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@mihnea-chirila
mihnea-chirila enabled auto-merge (squash) October 7, 2026 20:10
@mihnea-chirila
mihnea-chirila merged commit 1f0a2ad into develop Oct 8, 2026
248 of 254 checks passed
@mihnea-chirila
mihnea-chirila deleted the users/mchirila/bufferload0_regression_fix branch October 8, 2026 07:46
@assistant-librarian

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

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.

2 participants