[HLSL] Execute LinAlg element access and wave arithmetic at every supported wave size - #8870
Merged
Jack Elliott (JoeCitizen) merged 1 commit intoSep 2, 2026
Conversation
…ported wave size The MATRIX_CONSTRUCTION and WAVE_MATRIX_MULTIPLY capability queries are answered per wave size, and a device may report several. The selectors scanned 4 through 128 and returned on the first match, so the value baked into FORCED_WAVE_SIZE was always the smallest supported size and every larger one went untested. On a device supporting both wave32 and wave64, a wave64-specific lane distribution, coordinate mapping or collective defect passes the whole suite. This is a defect-detection argument, not a conformance requirement. Proposal 0035 does not define a wave size axis at all, so nothing here claims a driver must support more than one size; the point is only that whatever sizes it does report should all be exercised. collectMatrixConstructionWaveSizes returns the full qualifying set. selectMatrixConstructionWaveSize becomes a wrapper over it that takes the front, so the twenty-six callers that are not part of this change keep first-match behaviour and suite runtime stays bounded. The seven element access cases and the four wave arithmetic cases loop over the full set instead, recompiling and redispatching at each size. Element access is the primitive every other test uses to inspect matrix contents, and the arithmetic cases drain a multiply-produced accumulator, so between them they cover both how elements are placed and how they are computed. selectWaveArithmeticMultiplyWaveSize had exactly one caller and is converted outright rather than kept alongside a sweeping twin. On WARP the 16x16 F16 accumulator tile reports wave 4 and 8 supported and the 4x8 F32 tile reports only wave 4, so eight of the eleven affected cases now execute twice and three still execute once. Total shader executions across those cases go from eleven to nineteen. The 4x8 F32 cases are a useful control in their own right: if the collection logic were wrong they would gain a second run they have no capability for. Worth stating plainly for review: because the sweep happens inside an existing TEST_METHOD, the per-test differential against main is zero added, zero removed and zero outcome changes. That proves no regression but proves nothing about the added coverage. The coverage is evidenced by the log showing each affected case compiling at both FORCED_WAVE_SIZE=4 and FORCED_WAVE_SIZE=8, and by a negative control that corrupts output only under FORCED_WAVE_SIZE == 8: it fails the 16x16 F16 cases on this change while leaving both the 4x8 F32 cases and the pre-change build green. No wave size above 8 is exercised anywhere, because WARP reports none. Assisted-by: GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 83725f5d-8e98-4c1d-91ee-ad47629e007b
Copilot started reviewing on behalf of
Jack Elliott (JoeCitizen)
September 1, 2026 19:26
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Expands HLSL LinAlg execution coverage across every supported wave size while preserving first-match behavior elsewhere.
Changes:
- Collects all supported matrix-construction wave sizes.
- Sweeps seven element-access and four wave-arithmetic cases.
- Improves capability-match logging.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| bool &Supported, UINT &SelectedWaveSize) { | ||
| Supported = false; | ||
| SelectedWaveSize = 0; | ||
| static HRESULT collectMatrixConstructionWaveSizes( |
Joshua Batista (bob80905)
approved these changes
Sep 1, 2026
Joshua Batista (bob80905)
left a comment
Joshua Batista (bob80905)
left a comment
Collaborator
There was a problem hiding this comment.
a nit, otherwise LGTM
| return S_OK; | ||
| } | ||
|
|
||
| for (UINT WaveSize = 4; WaveSize <= 128; WaveSize *= 2) { |
Collaborator
There was a problem hiding this comment.
I do wonder if there are global variables that store Min/Max wave size, should this maximum change and 256 sized waves become a thing.
Alex Sepkowski (alsepkow)
approved these changes
Sep 2, 2026
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.
The MATRIX_CONSTRUCTION and WAVE_MATRIX_MULTIPLY capability queries are answered per wave size and a device may report several, but the selectors returned on the first match, so every case compiled at the smallest supported size and no larger one was ever executed. The seven element access cases and the four wave arithmetic cases now sweep the full reported set; the other twenty-six callers keep first-match behaviour to bound suite runtime.
Assisted-by: GitHub Copilot