[HLSL] Add Wave matrix arithmetic coverage for LinAlg - #8774
Open
Jack Elliott (JoeCitizen) wants to merge 2 commits into
Open
[HLSL] Add Wave matrix arithmetic coverage for LinAlg#8774Jack Elliott (JoeCitizen) wants to merge 2 commits into
Jack Elliott (JoeCitizen) wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds broader LinAlg wave-matrix arithmetic coverage with exact CPU-oracle validation.
Changes:
- Adds rectangular, mixed-type, integer, and B-use arithmetic tests.
- Adds checked integer matrix-product oracle helpers.
- Makes accumulator-layout behavior observable and poison-initialized.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Exercise non-uniform rectangular F16 products, mixed F16-to-F32 accumulation, B-use accumulation, and capability-gated I32 at the exact wave size supported by both matrix construction and WaveMatrixMultiply. The rectangular integer-valued patterns distinguish row, column, and indexing defects while keeping every result bit-exact. Derive expected products with checked signed 64-bit host arithmetic and self-test the oracle against the hand-derived 2x3 by 3x2 products [58, 64, 139, 154] and accumulated values [59, 63, 141, 152]. Make MatrixQueryAccumulatorLayout observable by selecting either the 2+3 A path or 2+7 B path and publishing the queried layout from lane zero only. The existing F16 arithmetic baselines remain unchanged. Assisted-by: GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 83725f5d-8e98-4c1d-91ee-ad47629e007b
Jack Elliott (JoeCitizen)
force-pushed
the
linalg-hlk-wave-matrix-arith
branch
from
August 13, 2026 21:58
927bbab to
081b05b
Compare
Damyan Pepper (damyanp)
left a comment
Damyan Pepper (damyanp)
left a comment
Member
There was a problem hiding this comment.
LGT-my superficial review. This will need to be looked at by someone with some more domain knowledge.
Damyan and Chris both pushed back on the checked arithmetic added here, and on inspection it can go entirely rather than be refactored. Every value it guards is authored by the test. The dimensions are literals of at most M=16, N=16, K=32; MatrixBufferLayout carries literal stride and offset values; nothing reaches these helpers from a driver. The largest integer literal anywhere in the file is 65504, so the widest accumulation the oracle can actually perform is around 1.4e11 against an int64 range of 9.2e18. The overflow branches were unreachable, and an overflow would have meant a bug in the test rather than a driver failing conformance. checkedAddInt64 and checkedMultiplyInt64 are removed along with the self-test assertions that only existed to exercise them. multiplyIntegerMatrices now returns its result directly instead of an optional, and reads as ordinary arithmetic. Its dimension and size preconditions are already established by isWaveMultiplyCaseValid, which every caller passes through, so checking them again here was the cascade Chris objected to. The compound conditions in encodeLogicalMatrixBuffer and isWaveMultiplyCaseValid are split so each test has one readable outcome. Removing these also resolves a collision that neither pull request shows on its own: microsoft#8775 defines the same two helpers in the same namespace, in a different region of the file, so the two would have merged without conflict and left main with a duplicate definition. Verified with the full HLSLExec LinAlg selection on WARP, compared per test rather than by totals: 45 total, 40 passed, 4 failed, 1 skipped, identical to the parent commit. Assisted-by: GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 83725f5d-8e98-4c1d-91ee-ad47629e007b
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (1)
tools/clang/unittests/HLSLExec/LinAlgTests.cpp:4333
isWaveMultiplyCaseValidaccepts any encodable I32 operands, but this unchecked multiply/add can overflow signedint64_t(for example, summing threeINT32_MAX * INT32_MAXterms), invoking undefined behavior before result encoding can reject it. Detect both multiplication and addition overflow and make the oracle fail explicitly so it cannot generate a silently incorrect expected result.
for (MatrixDim Inner = 0; Inner < K; ++Inner)
Sum += MatrixA[static_cast<size_t>(Row) * K + Inner] *
MatrixB[static_cast<size_t>(Inner) * N + Column];
Damyan Pepper (damyanp)
left a comment
Damyan Pepper (damyanp)
left a comment
Member
There was a problem hiding this comment.
Latest updates LGTM, thanks! Deferring to someone with domain expertise to review the test content.
Jack Elliott (JoeCitizen)
enabled auto-merge (squash)
August 15, 2026 00:38
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.
Proposal 0035 specifies Wave-scope matrix multiply and accumulate. Existing coverage was square F16 only, so row, column and indexing defects were not distinguishable.
These six tests exercise non-uniform rectangular F16 products, mixed F16 to F32 accumulation, B-use accumulation and capability-gated I32, at the exact wave size both matrix construction and the multiply advertise. Expected products come from checked signed 64-bit host arithmetic, self-tested against a hand-derived 2x3 by 3x2 case so the oracle cannot silently mirror the implementation.
MatrixQueryAccumulatorLayoutis made observable by selecting a distinct arithmetic path per layout, and the output is poison-initialised so a shader that stores nothing cannot read back as a valid layout. WARP reports the B layout unconditionally, so the A path is compiled and host-oracled here but not executed.