Repository navigation
refactor(tensilelite): separate persistent execution policies - #12537
Merged
Merged
Conversation
1 task
ThanHenderson
changed the title
✅ 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. |
jaopaulolc
added
the
rocm:gemm algos
AlexBrownAMD
left a comment
AlexBrownAMD
left a comment
Contributor
There was a problem hiding this comment.
Nice refactor! It'd be good to also run a quick performance test just to verify modes like stream-k/persistent kernel aren't accidentally being disabled somewhere that we didn't catch in review.
jaopaulolc
reviewed
Sep 25, 2026
jaopaulolc
left a comment
jaopaulolc
left a comment
Contributor
There was a problem hiding this comment.
LGTM so far. I am roughly 1/3 through it and will add more comments as I work through the changes.
ThanHenderson
force-pushed
the
users/nhenders/persistent-loop-01-policy
branch
3 times, most recently
from
September 25, 2026 22:01
e7cfc93 to
6a3250c
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #12537 +/- ##
===========================================
+ Coverage 70.69% 70.72% +0.03%
===========================================
Files 2941 2944 +3
Lines 484449 484876 +427
Branches 69855 69936 +81
===========================================
+ Hits 342475 342916 +441
+ Misses 118081 118056 -25
- Partials 23893 23904 +11
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
ThanHenderson
force-pushed
the
users/nhenders/persistent-loop-01-policy
branch
from
September 28, 2026 15:36
6a3250c to
72668fd
Compare
This was referenced Sep 28, 2026
jaopaulolc
reviewed
Sep 28, 2026
jaopaulolc
reviewed
Sep 28, 2026
ThanHenderson
force-pushed
the
users/nhenders/persistent-loop-01-policy
branch
from
September 28, 2026 18:21
72668fd to
5a40ad1
Compare
jaopaulolc
reviewed
Sep 28, 2026
jaopaulolc
left a comment
jaopaulolc
left a comment
Contributor
There was a problem hiding this comment.
LGTM. If a perf run does not show any unintended regressions, I am hapy to approve this.
ThanHenderson
force-pushed
the
users/nhenders/persistent-loop-01-policy
branch
from
September 29, 2026 15:46
5a40ad1 to
52c2933
Compare
ThanHenderson
marked this pull request as ready for review
September 29, 2026 16:11
ThanHenderson
force-pushed
the
users/nhenders/persistent-loop-01-policy
branch
from
September 29, 2026 17:34
52c2933 to
25fbd4e
Compare
AlexBrownAMD
approved these changes
Sep 29, 2026
AlexBrownAMD
left a comment
AlexBrownAMD
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the updates!
Alex-Vasile
approved these changes
Sep 29, 2026
Alex-Vasile
left a comment
Alex-Vasile
left a comment
Contributor
There was a problem hiding this comment.
Approving CMake changes
jaopaulolc
approved these changes
Sep 29, 2026
ThanHenderson
force-pushed
the
users/nhenders/persistent-loop-01-policy
branch
2 times, most recently
from
October 1, 2026 14:40
81556c9 to
80ba953
Compare
Normalize legacy selectors into tile processing and work assignment policies. Select persistent DataParallel independently while retaining the version-zero six-word scheduling layout.
ThanHenderson
force-pushed
the
users/nhenders/persistent-loop-01-policy
branch
from
October 1, 2026 19:28
80ba953 to
46145ef
Compare
Contributor
|
All important tests passed before tests were restarted. |
minsukim-amd
added a commit
that referenced
this pull request
Oct 2, 2026
…20260901 Resolves three conflicts with develop's persistent execution-policy work (#12537, #12539): - LibraryIO.py: develop replaced parseLibraryLogicData's DefaultSolution merge with normalize_execution_policy_with_defaults(), which translates legacy StreamK selectors before Tensile's global defaults are filled in. This branch had moved that fill into the shared fillSolutionDefaults(), so give the helper develop's semantics -- normalize with the file's defaults, then fill defaultSolution -- and have it return the new dict, as the normalizer does. The converter's readers take its result too. - TensileLibLogicToYaml.py: keep develop's normalize_execution_policy() as the first step of formForkParams, ahead of this branch's generic carry, so legacy selectors reach it as the canonical TileProcessingStrategy and WorkAssignment. - DECISIONS.md: develop added its own D44 (and a second D30). Keep develop's entries as-is and renumber this branch's D44-D47 to D45-D48, updating the references in ADR 0030 and the three test files. This branch's ADRs (0029-0031) do not collide with develop's new 0014/0015 files. The normalizer now always completes InternalSupportParams (it adds PersistentLoopArgsVersion), and Tensile's reader hands that completed block to a handwritten custom kernel, so the two custom-kernel tests expect the normalized block rather than the raw input. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 task
caio96
added a commit
that referenced
this pull request
Oct 2, 2026
…ration SizeMapping no longer carries streamK or streamKForceDPOnly after #12537. Gate the check on requiresPartialReduction(), which already excludes atomic StreamK and persistent DataParallel, and migrate sk_queue_selfclean.yaml to the named policy parameters. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Contributor
TheRock Submodule Bump ActivityNewest first
|
dsaffars
added a commit
that referenced
this pull request
Oct 28, 2026
…dump (#14195) JIRA ID: AIHPBLAS-5250 ## Motivation Dict-format library logic keeps a stable solution key order after a Tensile merge and after geko writes a library. `SolutionIndex`, `SolutionUID` when present, `KernelNameMin`, and `SolutionNameMin` come first. The remaining keys are alphabetical. `InternalSupportParams` is sorted, and `KernArgsVersion` is omitted. The Tensile merge reorder is required after `812bcc660b` (`refactor(tensilelite): separate persistent execution policies`, #12537). That commit makes `reNameSolutions` rebuild every solution with `normalize_execution_policy_with_defaults`, `sol.clear()`, and `sol.update()`. The rebuilt insertion order is what the merge writer saves. geko also assigns keys in place, such as `StaggerUMapping` when `StaggerU` is 0 and `ActivationFuncCall`, and its writer emits that insertion order, so those keys were landing at the end of the solution. ## Technical Details - `TensileMergeLibrary.avoidRegressions` reorders every merged solution with `LibraryIO.reorderSolutionDictForDictMerge` after `removeDefaultInitParams` and before the YAML write. - geko `Library.dump` applies the same layout through a local helper, `_reorder_dict_solution`, so `libs`, `final_libs`, and `full_libs` match merge output. Dump does not import `LibraryIO`. ## Test Plan - `test_merge_writes_dict_solutions_in_canonical_order` merges shuffled dict solutions and checks the lead keys, the alphabetical tail, and sorted `InternalSupportParams` without `KernArgsVersion`. - `test_dict_dump_orders_appended_solution_keys` checks a shuffled geko solution that includes `SolutionUID`. - `test_dict_library_supports_property_access_and_dump` checks the lead order when `SolutionUID` is absent. - Merge a real gfx950 logic file and confirm each solution starts with the naming keys and the rest is alphabetical. ## Test Result The new unit tests passed. A merged gfx950 library with 382 solutions was checked separately: every solution had the naming keys first and the remaining keys alphabetical.
1 task
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-4950
Motivation
TensileLite generates matrix-multiplication kernels and controls how GPU workgroups receive output tiles. Persistent workgroups process multiple tiles, but selecting whole-tile DataParallel work currently requires StreamK settings. Expose tile processing and work assignment separately so DataParallel can be selected directly.
Technical Details
TileProcessingStrategyandWorkAssignmentto configuration, solution identity, serialization, and host launch selection.Noneselects ordinary execution;DataParallelselects persistent whole tiles;StreamKallows splitting tiles across workgroups.isPersistentDataParallelto distinguish persistent whole-tile execution from ordinary matrix multiplication. Preserve both ordinary custom-kernel grid forms and the persistent launch grid.DefaultSolutionvalues before global defaults, with explicit solution values taking precedence. Library merging uses the same rule for policy and ordinary tuning fields.--setoverrides to every size group. Tests cover both policy overrides and an ordinaryDepthU=32override across multiple groups.LoadLibraryFileentry point using temporary files. Both MessagePack and LLVM YAML serialization remain supported.skBackupleft by the policy migration. Add 20 generation-and-assembly regression cases across gfx90a, gfx942, gfx950, and gfx1250.Stack 1/5, targeting
develop. Followed by PR #12538.Test Plan
Run the Python unit suite, including policy ingestion, CLI overrides, generation-and-assembly regressions, and saved-result comparisons. Run the focused experimental-library extras tests. Build the complete C++ test executable with
HIPBLASLT_ENABLE_YAML=OFFand run policy, serialization, argument-layout, custom-grid, launch-bound, range, and prediction-library tests. Use MessagePack for this validation; the existing LLVM YAML shared-test link failure is outside this PR. No performance measurements.Test Result
At
5a40ad1816b, after rebasing ontodevelopat7affebf2fef, the full Python unit suite passed 8,969 tests, with 47 skips, 2 expected failures, 1 unexpected pass, and all 786 snapshots passing. This includes all 20 adaptive-GSU generation-and-assembly cases. The focused policy-ingestion, CLI, and experimental-library run passed 139 tests before the rebase.The rebuilt MessagePack host test executable passed 118 selected tests, including the new upstream prediction-library tests. One existing RangeLibrary test was skipped for AIHPBLAS-3506. Pre-commit passed. No snapshots were updated and no performance measurements were taken.
Automated checks for the updated head are pending. Previously reported Math profiling aborts/timeouts, ASAN runner failures, the C++ coverage shortfall, and other external gfx1250 failures are not claimed fixed; gfx1250 device execution was not validated locally.
Submission Checklist
Risk level
Medium (3/5): changes policy interpretation across generation, serialization, naming, and host dispatch while retaining existing input conversion and scheduling arguments. Tests cover policy conflicts, custom grids, serialization, launch bounds, and sampled emitted instructions.