[HLSL] Add LinAlg runtime capability handling - #8735
Open
JoeCitizen wants to merge 5 commits into
Open
Conversation
Add ABI-checked wrappers for the six D3D12 Linear Algebra capability query categories and explicit applicability classification. Gate the rectangular F32 CopyConvert case using concrete supported wave sizes. Assisted-by: GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 83725f5d-8e98-4c1d-91ee-ad47629e007b
Compile capability-gated CopyConvert coverage at the exact wave size whose MatrixConstruction support was queried. Keep mandatory baseline cases on the existing ranged WaveSize attribute. Assisted-by: GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 83725f5d-8e98-4c1d-91ee-ad47629e007b
Validate multiplication support flags per operation, exhaustively check the preview D3D12 ABI mirrors, and preserve query-backed optional skips in HLK mode. Assisted-by: GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 83725f5d-8e98-4c1d-91ee-ad47629e007b
The hand-rolled D3D12 struct mirrors in HlslExecTestUtils.cpp encoded an
obsolete linear algebra ABI, so every capability query was rejected with
E_INVALIDARG. CheckFeatureSupport validates FeatureSupportDataSize before
reading any field, which is why it failed uniformly for every component type
and every wave size rather than for a particular configuration.
Two of the eight mirrors had drifted:
MATRIX_CONSTRUCTION_SUPPORT
was {ComponentType, WaveSize, MinM, MinK, MinN} 20 bytes
now {ComponentType, WaveSize, Shape, Supported} 24 bytes
WAVE_MATRIX_MULTIPLY_SUPPORT
was {Inputs, SupportFlags, NumShapes, Shapes*} align 8
now {Inputs, Shape, SupportFlags} align 4
The remaining six were verified byte-identical and are unchanged.
The semantics inverted as well, so this is not a rename. The old model asked
the runtime for minimum supported dimensions and read MinM/MinK/MinN out. The
current model supplies a concrete Shape as an input and reads back a single
Supported boolean, with the runtime applying the "positive multiple of a
native tile" rule itself. queryMatrixConstruction and queryWaveMatrixMultiply
are rewritten accordingly, and now match the shape of the existing
queryThreadGroupMatrixMultiply rather than diverging from it.
Both queries stay on D3D12_FEATURE_LINEAR_ALGEBRA_MATRIX_OPERATION_SUPPORT
(feature 78) rather than moving to the new OPERATION_ENUMERATION (feature 80).
Feature 78 is the only form the runtime can always answer: it is zeroed below
_0115_1, translated through the linalgv1compat path on _0115_1, forwarded when
a driver advertises GRANULAR, and synthesized from the enumeration table
otherwise. Feature 80 returns DXGI_ERROR_UNSUPPORTED on _0115_1 preview
drivers, and may legitimately return an empty table for an operation when a
_0115_2 driver advertises GRANULAR only. Since QUERY_FORMS is a per-operation
flag pair, GRANULAR-only is permanently legal, so a conformance test built on
feature 80 could fail on hardware that is behaving correctly.
MatrixRole is removed. Neither MATRIX_CONSTRUCTION_SUPPORT nor its enumeration
entry carries a Role field; both describe a full {M,K,N} shape. The role model
only ever worked because the superseded ABI returned MinM, MinK and MinN as
separate values, so it is an artefact of the old layout rather than something
the current runtime exposes.
WaveMatrixMultiplyInputs is split out of WaveMatrixMultiplyQuery so that the
inputs and the shape are separate, mirroring the runtime's own split between
WAVE_MATRIX_MULTIPLY_INPUTS and the shape field that contains it. This also
removes the oddity of ThreadGroupMatrixMultiplyQuery embedding a query type
that carried a shape it did not use.
A use-A tile pins only M and K, but a granular query requires all three
extents, and the native N is not discoverable from feature 78 alone. The
earlier reasoning that there was therefore no principled value to supply for N
was wrong: the rule is separable per extent, so supportsUseAMatrix probes N
over the power-of-two extents native tiles are built from and accepts on the
first hit. A missed extent skips a test case; it can never report an
unsupported tile as supported. This is load bearing rather than defensive.
WARP's native tile is {w, 4, w}, so at wave 8 it answers TRUE for (8,4,8) but
FALSE for (8,4,4); simply reusing a known extent for N would have produced a
false negative and silently dropped coverage.
Naming
The local copies are named after the header types they describe rather than
carrying a Runtime prefix, so they can be diffed against d3d12.h field by
field and removed mechanically once these types ship in a released Windows
SDK. Declaration order follows the header for the same reason.
They live in a linalg_abi namespace, and that is what keeps the assertions
honest. A type declared in the enclosing anonymous namespace shadows a
same-named declaration at global scope for unqualified lookup, so naming the
copies verbatim at that scope would have quietly reduced every assertion to
sizeof(X) == sizeof(X). The comparisons are therefore explicitly qualified,
linalg_abi::X against ::X, and the comment above them records why the
qualification must not be tidied away.
Matching the header also removed the last field name divergence: the union
member the runtime calls ThreadOuterProductSupport was mirrored as
ThreadOuterProduct, a difference the offset assertions had been absorbing by
taking both names. Both assertion macros now take a single name, and the
two-name variant is gone.
Enum-typed fields remain UINT. The values crossing CheckFeatureSupport are
opaque to this code, and linalg_test's own enums are the typed surface; their
values are already pinned to the D3D12 enumerators by the neighbouring value
assertions.
Validation
d3d12.h defines DIRECT3D_LINEAR_ALGEBRA itself, so compiling this translation
unit with a linear algebra capable header on the include path arms every ABI
static_assert in the file. That compiles clean. A negative control confirms
the assertions are live rather than merely absent, and specifically that the
shared naming has not made them vacuous: inserting one spurious UINT into a
mirror fails the build on the size assertion and on three field offset
assertions.
The mirrors and call sequence were then checked against an independent oracle.
A standalone probe sends the same hand-rolled structs this file uses, not the
SDK types, and compares the answers to the D3D12 runtime team's own expected
value table for WARP, extended with wave-size discrimination cases. All
fourteen cases agree, eight of them negative, so the query demonstrably
discriminates rather than answering TRUE unconditionally.
Measured against a matched D3D12Core/WARP/SDKLayers triple, all configurations
built from the same tree:
unmodified main 23 total, 18 passed, 4 failed, 1 skipped
before this change 24 total, 18 passed, 5 failed, 1 skipped
after this change 24 total, 19 passed, 4 failed, 1 skipped
Before, the query reported hr=0x80070057 and CopyConvert_Wave_4x8_F32_Transpose
hard-failed. After, it matches at wave 4 for source 4x8 and destination 8x4.
The four remaining failures and the one skip are identical to unmodified main
and are pre-existing runtime lag behind the shipped spec, so this change is
zero regression.
The strict fail-on-failed-query applicability policy is deliberately retained.
It is what surfaced this defect, and weakening it to a skip would have let an
obsolete ABI pass silently.
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
Adds SDK-neutral D3D12 linear-algebra capability handling for SM6.10 execution tests.
Changes:
- Adds typed wrappers and validation for six LinAlg capability queries.
- Introduces capability policy and predicate tests.
- Gates the F32 transpose test using tier, shape, and wave-size support.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
LinAlgTests.cpp |
Adds capability tests and CopyConvert gating. |
HlslExecTestUtils.h |
Declares capability types and APIs. |
HlslExecTestUtils.cpp |
Implements ABI mirrors, validation, and queries. |
Suppressed comments (1)
tools/clang/unittests/HLSLExec/HlslExecTestUtils.cpp:1554
- These conversions similarly normalize malformed
BOOLoutputs totrue. If either field contains a noncanonical value, the wrapper returnsS_OKand consumers trust the response, contrary to the response-validation policy used by the matrix-construction wrapper.
Support.RWByteAddressBufferSupported =
RuntimeSupport.AccumulateStore.RWByteAddressBufferSupported != FALSE;
Support.GroupSharedSupported =
RuntimeSupport.AccumulateStore.GroupSharedSupported != FALSE;
The thread outer-product and atomic accumulate-store wrappers converted the runtime's BOOL with a bare `!= FALSE`, so a malformed response such as 2 was normalized to true and returned as S_OK. The matrix-construction wrapper already rejected non-canonical BOOLs, which left the file inconsistent with its own response-validation policy: two of the three BOOL-valued categories were not actually validated before use. That inconsistency matters more here than it would in a functional test. A driver reporting a non-canonical BOOL is exactly the class of defect an HLK conformance test exists to catch, so normalizing it away silently converts a reportable driver bug into a passing run. Factor the canonical check into a single isCanonicalBool() helper and route all three categories through it. MatrixConstructionSupport::valid() keeps its existing behaviour, and the two remaining wrappers now log the offending value and return E_UNEXPECTED rather than trusting it. Verified by negative control. Injecting a non-canonical BOOL into the outer-product path changed nothing, which was itself the useful result: that wrapper has no live caller yet. Only queryTierSupport and queryMatrixConstruction are reached from LinAlgTests.cpp, the remaining five being scaffolding for later work in this series. Re-running the control through matrix construction, which shares the new helper and is exercised, failed the run as intended and logged MatrixConstruction query returned a non-boolean result: 0x2 With the injection removed, WARP reports Total=24, Passed=19, Failed=4, Skipped=1, an identical non-passing set to the previous commit, so this is zero regression. Raised by automated review on microsoft#8735. 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 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (2)
tools/clang/unittests/HLSLExec/HlslExecTestUtils.cpp:1320
- The new wave-multiply wrapper, like the following thread-group, thread-vector, outer-product, and atomic wrappers, has no automated call path: the capability test only exercises the result predicates, and the execution test reaches only tier and matrix-construction queries. Consequently, operation/union selection, input marshalling, and malformed-response handling for five of the six advertised wrappers can regress without detection. Please add focused query-path coverage, for example by separating payload construction/response parsing from
CheckFeatureSupportso those parts can be tested deterministically.
HRESULT queryWaveMatrixMultiply(ID3D12Device *Device,
const WaveMatrixMultiplyQuery &Query,
WaveMatrixMultiplySupport &Support) {
tools/clang/unittests/HLSLExec/HlslExecTestUtils.cpp:1259
- This rejects every tier value above 1.0 as malformed. The D3D12 Linear Algebra spec deliberately tests
LinearAlgebraTier >= D3D12_LINEAR_ALGEBRA_TIER_1_0, so a future conforming tier would make these tests fail withE_UNEXPECTEDinstead of recognizing Tier 1 support. Please accept later tiers (and makeTierSupport::supported()use the same ordered comparison) while still rejecting values below Tier 1.0 other thanNotSupported.
if (RuntimeSupport.LinearAlgebraTier !=
static_cast<UINT>(Tier::NotSupported) &&
RuntimeSupport.LinearAlgebraTier != static_cast<UINT>(Tier::Tier1_0)) {
LogCommentFmt(L"Linear algebra tier query returned invalid tier: 0x%x",
RuntimeSupport.LinearAlgebraTier);
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.
Summary
Adds SDK-neutral wrappers around the D3D12 linear algebra capability queries, plus the typed predicates and applicability policy the SM6.10 LinAlg execution tests use to decide whether a case is mandatory or capability-gated.
CopyConvert_Wave_4x8_F32_Transposeon the tier and on construction support at the wave size the query selectedThe granular query (78) is used rather than the enumeration (80), because it is the only form the runtime can always answer; and the D3D12 structs are copied locally, because the released Windows SDK does not declare them.
Closes #8686
Assisted-by: GitHub Copilot