Skip to content

fix(tensilelite): keep the packed StaggerU across persistent tiles - #14719

Merged
caio96 merged 2 commits into
developfrom
users/csalvado/stagger-persistent-fix
Oct 30, 2026
Merged

caio96 merged 2 commits into
developfrom
users/csalvado/stagger-persistent-fix

Conversation

@caio96

@caio96 caio96 commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Persistent kernels lose their stagger settings after a workgroup's first tile. With uniform summation order on (the default), this gives different results for identical rows of A.

Cause

The host packs the stagger amount, stride and mapping into one register, which the kernel reads once at start. declareStaggerParms decodes it and then overwrites it with just the amount. Persistent kernels run declareStaggerParms again for each tile, so from the second tile on, the mapping and stride read as 0.

Mapping 0 rotates the start of the K loop by the tile's M index, so rows in different M tiles are summed in a different order. Under uniform summation order this breaks row uniformity. It shows up on Hybrid StreamK kernels in the dynamic sub-mode, which give each workgroup many tiles. Example: f32 1536x1536x2048 with HIPBLASLT_MATMUL_DESC_SM_COUNT_TARGET=160 gives 719,328 mismatched elements.

Fix

  • declareStaggerParms decodes the amount into a temporary register and leaves the packed register unchanged.
  • The PGR=2 loop counter decrement checks the decoded amount instead of the whole register.

Kernels that process one tile per workgroup behave the same as before.

Testing

  • New TensileLite unit test: declareStaggerParms never writes the packed register (PGR 1, 2, 3).
  • gfx950, Hybrid dynamic sub-mode, f32, identical rows in A. These went from 238k–2.6M mismatched elements to 0, and results still pass validation:
    • 1536x1536x2048 and 1536x1536x4096 with an SM count target;
    • 1536x1536x4096 and 3072x3072x2048 with dynamic scheduling on.
  • New row-uniformity gtest case HybridDynamicStagger_1536x1536x2048_SmCount160, which fails on develop and passes with this fix.
  • Time: 0 to +3% on the cases above, because the selected stagger now applies to every tile.

Jira ID: AIHPBLAS-5430

caio96 and others added 2 commits October 29, 2026 17:22
declareStaggerParms decoded the stride shift and mapping from the packed
StaggerU sgpr and then overwrote it with the masked amount (and, for
PGR>=3, zeroed it on short loops). StaggerU is only unpacked from the
kernel args at entry, but persistent kernels rerun declareStaggerParms
for every tile, so a workgroup's second and later tiles staggered with
mapping 0 and stride shift 0. Under uniform summation order this gave
non-identical D rows for identical A rows on Hybrid StreamK kernels in
the dynamic work-queue mode (more tiles than workgroups).

Decode the amount into the existing temp instead, and have the PGR=2
closeLoop counter decrement test the masked amount.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ation order

Add a row-uniformity case where an SM count target makes the heuristic
pick the Hybrid dynamic sub-mode, so a workgroup runs several whole
tiles. Every default pick must give identical D rows for identical A
rows, or be refused.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@therock-pr-bot

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

Copy link
Copy Markdown

⏳ Waiting on Required Checks

⏳ Required checks have not all concluded yet. This bot run is exiting without failing so those checks can report independently.

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ⏳ Pending ⏳ Still running…
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

⏳ Required checks are still running or have not started yet.
This PR is not ready for review until they complete.

📖 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 30, 2026 •

Copy link
Copy Markdown

⏳ Waiting for required checks to start or finish:

  • ⏳ pre-commit

@caio96
caio96 marked this pull request as ready for review October 30, 2026 00:03
@caio96
caio96 requested a review from a team as a code owner October 30, 2026 00:03
@codecov

codecov Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...lt/tensilelite/tensilelite/KernelWriterAssembly.py 50.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #14719      +/-   ##
===========================================
- Coverage    77.34%   77.34%   -0.00%     
===========================================
  Files         2893     2893              
  Lines       486262   486261       -1     
  Branches     73502    73502              
===========================================
- Hits        376095   376094       -1     
  Misses       85342    85342              
  Partials     24825    24825              
Flag Coverage Δ *Carryforward flag
TensileLite-CPP 50.82% <ø> (ø)
TensileLite-Unit 84.14% <50.00%> (-<0.01%) ⬇️
hipBLAS 90.62% <ø> (ø) Carriedforward from b0f07f8
hipBLASLt 68.59% <ø> (ø)
hipCUB 82.68% <ø> (ø) Carriedforward from b0f07f8
hipDNN 86.92% <ø> (ø) Carriedforward from b0f07f8
hipFFT 43.18% <ø> (ø) Carriedforward from b0f07f8
hipRAND 76.12% <ø> (ø) Carriedforward from b0f07f8
hipSOLVER 69.67% <ø> (ø) Carriedforward from b0f07f8
hipSPARSE 87.03% <ø> (ø) Carriedforward from b0f07f8
rocBLAS 68.96% <ø> (ø) Carriedforward from b0f07f8
rocFFT 47.36% <ø> (ø) Carriedforward from b0f07f8
rocRAND 57.32% <ø> (ø) Carriedforward from b0f07f8
rocSOLVER 76.74% <ø> (ø) Carriedforward from b0f07f8
rocSPARSE 83.54% <ø> (ø) Carriedforward from b0f07f8
rocThrust 93.15% <ø> (ø) Carriedforward from b0f07f8

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

Files with missing lines Coverage Δ
...lt/tensilelite/tensilelite/KernelWriterAssembly.py 79.48% <50.00%> (-<0.01%) ⬇️
🚀 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.

@caio96
caio96 merged commit d774318 into develop Oct 30, 2026
394 of 420 checks passed
@caio96
caio96 deleted the users/csalvado/stagger-persistent-fix branch October 30, 2026 17:19
@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.

4 participants