[HLSL] Add MatVec interpretation and bias coverage for LinAlg by JoeCitizen · Pull Request #8775 · microsoft/DirectXShaderCompiler

Jack Elliott (JoeCitizen) · 2026-08-13T21:03:02Z

Proposal 0035 permits a thread-scope Multiply vector to be native or an InterpretedVector, pairing a packed vector with an interpretation type. The interpreted forms had no coverage.

These six tests cover non-uniform F16 in row-major and column-major layouts, packed SInt8 and UInt8 inputs, unsigned UInt32 output, and MultiplyAdd with the bias in a separate resource. The UInt8 case reuses the SInt8 bytes so high-bit lanes decode differently under the two interpretations. Expected values come from overflow-checked host dot products, and the full output allocation including padding and guard bytes is compared against a poison fill.

Column-major is required rather than gated: a thread-scope load permits row-major, column-major and optimal layouts, and only transposed loads need a driver query.

Copilot

Pull request overview

Adds thread-scope LinAlg matrix-vector tests for layouts, interpreted inputs, unsigned output, and independent bias.

Changes:

Adds six Multiply/MultiplyAdd execution tests.
Adds host-side encoding, overflow-checked oracles, capability checks, and guard-byte validation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Damyan Pepper (damyanp)

Some observations, none of these should block the PR after it has been reviewed by someone with more domain knowledge than me.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

tools/clang/unittests/HLSLExec/LinAlgTests.cpp:2287

Please use the explicit result type under the repository's almost-never-auto convention. runShaderOp declares this as std::shared_ptr<st::ShaderOpTestResult> in HlslExecTestUtils.h:601.

  auto Result =

tools/clang/unittests/HLSLExec/LinAlgTests.cpp:2272

Per the repository's almost-never-auto convention, this straightforward return type should be explicit. createComputeOp is declared as returning std::unique_ptr<st::ShaderOp> in HlslExecTestUtils.h:571.

This issue also appears on line 2287 of the same file.

  auto Op = createComputeOp(Shader, "cs_6_10", RootSignature, Args->c_str());

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Exercise non-uniform F16 row-major and column-major layouts, packed SInt8 and UInt8 interpreted inputs, unsigned UInt32 output, and a separate non-uniform bias resource. High-bit UInt8 lanes distinguish unsigned from signed decoding of the same bytes. Derive every expected result with overflow-checked host dot products plus optional bias, encode packed lanes least-significant-byte first, and compare the complete poisoned output including padding and guard bytes. Query the exact vector, matrix, bias, and result capability tuple, requiring both mandatory F16 layouts and gating only optional output cases. Column-major is required rather than capability gated because a thread scope matrix load permits row-major, column-major and optimal layouts, and only transposed loads are implementation specific and need a driver query. Assisted-by: GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 83725f5d-8e98-4c1d-91ee-ad47629e007b

LinAlgTests.cpp places helper namespaces before the test classes that use them: cpu_oracle sits at the top of the file, ahead of the first test class. matvec_interpretation was appended after the classes instead, which left no way for a LinAlgCPUOracleTests method to call into it without a forward declaration. This is a pure relocation of the namespace block. No line is added, removed or edited: the file has the same 6060 lines before and after, and the sorted set of lines is identical. clang-format reports no drift. Verified with the full HLSLExec LinAlg selection on WARP, compared per test rather than by totals: 45 total, 39 passed, 5 failed, 1 skipped, with the non-passing set unchanged from the parent commit. Assisted-by: GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 83725f5d-8e98-4c1d-91ee-ad47629e007b

Move the checked int64 helpers into cpu_oracle, next to the existing checkedMultiply and checkedAdd for size_t. That namespace is already the file's home for arithmetic the oracle relies on, so matvec_interpretation no longer carries its own copies. The checks stay: encodeComponents accepts UInt32 values up to 4294967295, so a UInt32 matrix times a UInt32 vector overflows int64 by construction. An overflow in the oracle yields a wrong expected value, which can pass a broken implementation rather than fail a correct one, so this is the direction worth guarding. Report the rejected value when a component cannot be represented in the target type. All six rejection sites now name the type and the value instead of failing with no context, and componentTypeName covers SInt8 and UInt8 rather than returning "Unsupported" for them. Promote the oracle self-test out of runCase into its own method on LinAlgCPUOracleTests, so it runs once instead of once per case. That class already exists for exactly this purpose and deliberately carries no Kits metadata, so HLK runs never select it. Each assertion gets its own VERIFY, replacing a single six-term conjunction that could not say which part failed. Verified with the full HLSLExec LinAlg selection on WARP, compared per test rather than by totals: 46 total, 40 passed, 5 failed, 1 skipped. The only difference from the parent commit is the added MatVecHostOracle passing; the non-passing set is unchanged. Assisted-by: GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 83725f5d-8e98-4c1d-91ee-ad47629e007b

Follows the same review feedback Damyan and Chris gave on microsoft#8774, applied here so the two pull requests stay consistent and so neither lands this code. Every value these helpers guarded is authored by the test: dimensions are literals of at most M=16 and N=16, and the largest integer literal in the file is 65504. The widest accumulation the oracle can perform is far below the int64 range, so the overflow branches were unreachable, and an overflow would have indicated a bug in the test rather than a driver failing conformance. checkedAddInt64 and checkedMultiplyInt64 are removed together with the two self-test assertions that existed only to exercise them. calculateExpected returns its result directly rather than an optional and reads as ordinary arithmetic. Its size preconditions are already established by isCaseValid, which runCase verifies before any of this is reached. This also removes a collision that neither pull request shows in its own diff. microsoft#8774 defines the same two helpers in the same cpu_oracle namespace but in a different region of the file, so git would have merged both 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: 46 total, 40 passed, 5 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

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

tools/clang/unittests/HLSLExec/LinAlgTests.cpp:1924

The host oracle performs the dot product with unchecked signed int64_t multiplication and addition. A valid supported case using large U32 operands can therefore trigger signed-overflow undefined behavior and produce unreliable expected bytes, contrary to the PR description's overflow-checked oracle claim. Please use checked or wider arithmetic and reject results that cannot be represented before encoding them.

      Expected[Row] +=
          Case.MatrixValues[static_cast<size_t>(Row) * Case.N + Column] *
          Case.InterpretedVectorValues[Column];

tools/clang/unittests/HLSLExec/LinAlgTests.cpp:1811

This round-trip check can itself invoke undefined behavior: INT64_MAX rounds to 2^63 when converted to float, and converting that out-of-range float back to int64_t is undefined. Check that FloatValue is within the half-open int64_t range before performing the cast, then do the exactness comparison.

This issue also appears on line 1922 of the same file.

      if (static_cast<int64_t>(FloatValue) != Value)
        return reportUnrepresentable(Type, Value);

Damyan Pepper (damyanp)

LGTM - my previous comments are all addressed.

Ashley Coleman (V-FEXrt) · 2026-08-17T22:29:42Z

+  case ComponentType::I32:
+  case ComponentType::U32:
+    return 4;
+  default:


Assuming no return 8 because we just haven't gotten around to 64bit types yet?

TBH we should just go ahead and fully fill this out since we know the full set of ComponentTypes we will eventually test. Less likely that we'll forget to update them that way.

Ashley Coleman (V-FEXrt) · 2026-08-17T22:31:15Z

+}
+
+static bool isPackedByteVector(ComponentType Type) {
+  return Type == ComponentType::I8 || Type == ComponentType::U8;


this is incomplete, like above we should go ahead and just fully fill it out

Ashley Coleman (V-FEXrt) · 2026-08-17T22:32:23Z

+  return Type == ComponentType::I8 || Type == ComponentType::U8;
+}
+
+static const char *storageTypeName(ComponentType Type) {


Same here....

I'm realizing this is a larger change so I'm okay to punt to a follow up PR but I'd like to see it sooner vs later. Basically anything that switches over ComponentType should list out every case we are interested in

Ashley Coleman (V-FEXrt) · 2026-08-17T22:33:35Z

+
+static MatrixDim storageElementCount(ComponentType Type,
+                                     MatrixDim LogicalCount) {
+  return isPackedByteVector(Type) ? (LogicalCount + 3) / 4 : LogicalCount;


IIRC 3 and 4 here can't be constants because of BFloat16 which is 2 elements per scalar instead of 4

Ashley Coleman (V-FEXrt) · 2026-08-17T22:43:08Z

+
+static bool isCaseValid(const CaseData &Case) {
+  size_t MatrixElementCount;
+  if (Case.M == 0 || Case.N == 0 ||


This condition is far too complicated to reasonable parse or think through

It should be split into several conditions. They can still be grouped together in ways that make sense like layout or dimension but one mega case just becomes impossible to read

Ashley Coleman (V-FEXrt) · 2026-08-17T22:45:59Z

+  size_t PaddedSize;
+  if (!cpu_oracle::checkedAdd(Logical->size(), size_t(3), PaddedSize))
+    return std::nullopt;
+  PaddedSize &= ~size_t(3);


A short comment on the intent of this line would be nice. It's a small line but its actually doing a surprising amount of stuff

Ashley Coleman (V-FEXrt) · 2026-08-17T22:48:02Z

+}
+
+static const char MatVecMulShader[] = R"(
+  #define USE_A 0


nit:

Not for now, but I wonder if its useful to define all these matrix api values as part of the buildCompilerArgs. Probably not since it'll make the CLI messy. Also its not valuable if we switch over to the header api

Ashley asked that anything switching over ComponentType list every case we care about rather than lean on a default arm. The authoritative set is the fourteen types ComponentTypeTraits declares in hlsl/dx/linalg.h, which is narrower than the twenty four in DxilConstants.h: it excludes I1, the SNorm and UNorm variants, PackedS8x32 and PackedU8x32. componentByteSize, storageTypeName and isPackedByteVector now enumerate those fourteen, and componentByteSize answers the question the review opened with by returning 8 for I64, U64 and F64. storageElementCount no longer hardcodes 3 and 4. The packing factor comes from a new elementsPerScalar that mirrors ComponentTypeTraits::ElementsPerScalar, and Ashley's point about BFloat16 holds: it is two elements per scalar, not four. It is not derivable from the byte size either, since F16 and BFloat16 are both two bytes but only F16 has a native HLSL scalar. isPackedByteVector stays byte only for that same reason, because encodeVectorBuffer routes on it and encodePackedVector packs four bytes to a uint, which would corrupt BFloat16. isCaseValid is now a sequence of grouped guards covering dimensions, input counts, layout, component types, vector form and bias, rather than one nine term conjunction. Its checkedMultiply is gone: MatrixDim is uint32_t, so widening the row by column product to uint64_t cannot overflow. The remaining checked arithmetic this branch introduced is removed on the same grounds as the parent commit, which leaves the file free of qualified cpu_oracle::checked calls. The size_t helpers those calls reached into are pre-existing in main and are a separate cleanup. Unknown types no longer fail quietly. componentByteSize, elementsPerScalar and storageTypeName name the offending type and fail, because each maps a type onto a value its caller needs, so a missing answer is a defect rather than a result. The report includes the numeric enum value, since componentTypeName only knows seven types and prints Unsupported for the rest. isPackedByteVector and isEncodableComponentType stay silent because false is a legitimate answer for them. storageTypeName separates the two failures: a valid matrix type the encoder cannot drive yet reports as unsupported, while anything outside the fourteen reports as unexpected. This matters because isCaseValid has a single caller, so the previous behaviour was a generic invalid case failure that did not say which field was at fault. Verified with the full HLSLExec LinAlg selection on WARP, compared per test rather than by totals: 50 total, 44 passed, 5 failed, 1 skipped, identical to the parent commit in every entry. Both the baseline and this change were measured on the same experimental tier D3D12 runtime, because a default tier runtime cannot enable SM 6.10 and blocks the whole selection. The assertions were confirmed to fire by temporarily calling the helpers with PackedS8x32 and SNormF32 and observing the logged type and the failed test. Assisted-by: GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 83725f5d-8e98-4c1d-91ee-ad47629e007b

Copilot

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:2081

This comment says there are four guard bytes, but OutputGuardBytes is 16. Refer to the guard region instead so the documentation remains accurate if its size changes.

  // Round the byte count up to a whole number of 4-byte words so that the four
  // guard bytes start on a word boundary. Both sizes are bounded by the matrix

Chris B (llvm-beanz) · 2026-08-18T17:22:22Z

+    return std::nullopt;
+
+  std::stringstream Args;
+  Args << "-HV 202x";


Is 202x necessary here? I'm a little nervous about having HLK tests depend on an unstable compiler feature.

Copilot AI balanced review requested due to automatic review settings August 13, 2026 21:03

github-project-automation Bot added this to HLSL Roadmap Aug 13, 2026

github-project-automation Bot moved this to New in HLSL Roadmap Aug 13, 2026

Copilot started reviewing on behalf of Jack Elliott (JoeCitizen) August 13, 2026 21:03 View session

Copilot AI reviewed Aug 13, 2026

View reviewed changes

Comment thread tools/clang/unittests/HLSLExec/LinAlgTests.cpp Outdated

Copilot AI review requested due to automatic review settings August 13, 2026 21:58

Jack Elliott (JoeCitizen) force-pushed the linalg-hlk-matvec-interpretation branch from baa671f to 2010c0e Compare August 13, 2026 21:58

Copilot started reviewing on behalf of Jack Elliott (JoeCitizen) August 13, 2026 21:59 View session

Copilot AI reviewed Aug 13, 2026

View reviewed changes

Damyan Pepper (damyanp) reviewed Aug 14, 2026

View reviewed changes

Comment thread tools/clang/unittests/HLSLExec/LinAlgTests.cpp Outdated

Comment thread tools/clang/unittests/HLSLExec/LinAlgTests.cpp Outdated

Comment thread tools/clang/unittests/HLSLExec/LinAlgTests.cpp Outdated

Copilot AI review requested due to automatic review settings August 14, 2026 20:33

Copilot started reviewing on behalf of Jack Elliott (JoeCitizen) August 14, 2026 20:33 View session

Copilot AI reviewed Aug 14, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 14, 2026 22:06

Copilot started reviewing on behalf of Jack Elliott (JoeCitizen) August 14, 2026 22:07 View session

Copilot AI reviewed Aug 14, 2026

View reviewed changes

Jack Elliott and others added 4 commits August 15, 2026 10:34

Copilot AI review requested due to automatic review settings August 14, 2026 22:47

Jack Elliott (JoeCitizen) force-pushed the linalg-hlk-matvec-interpretation branch from c89b779 to 9b8b5b5 Compare August 14, 2026 22:47

Copilot started reviewing on behalf of Jack Elliott (JoeCitizen) August 14, 2026 22:48 View session

Copilot AI reviewed Aug 14, 2026

View reviewed changes

Damyan Pepper (damyanp) reviewed Aug 15, 2026

View reviewed changes

Jack Elliott (JoeCitizen) enabled auto-merge (squash) August 15, 2026 00:38

Ashley Coleman (V-FEXrt) self-assigned this Aug 17, 2026

Damyan Pepper (damyanp) requested a review from Ashley Coleman (V-FEXrt) August 17, 2026 18:00

Ashley Coleman (V-FEXrt) reviewed Aug 17, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 18, 2026 00:05

Copilot started reviewing on behalf of Jack Elliott (JoeCitizen) August 18, 2026 00:06 View session

Copilot AI reviewed Aug 18, 2026

View reviewed changes

Chris B (llvm-beanz) reviewed Aug 18, 2026

View reviewed changes

Uh oh!

Conversation

Jack Elliott (JoeCitizen) commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Damyan Pepper (damyanp) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Damyan Pepper (damyanp) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Jack Elliott (JoeCitizen) commented Aug 13, 2026 •

edited

Loading