[SM6.10][LinAlg] Remove TargetType, allow TG accumulate - #8883
Open
Ashley Coleman (V-FEXrt) wants to merge 2 commits into
Open
[SM6.10][LinAlg] Remove TargetType, allow TG accumulate#8883Ashley Coleman (V-FEXrt) wants to merge 2 commits into
Ashley Coleman (V-FEXrt) wants to merge 2 commits into
Conversation
Copilot started reviewing on behalf of
Ashley Coleman (V-FEXrt)
September 4, 2026 16:40
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The public API retains an ignored target-type argument, while release-note and runtime coverage remain incomplete.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates Shader Model 6.10 LinAlg accumulation to match the revised specification.
Changes:
- Removes
TargetTypefrom matrix-to-groupshared-memory accumulation. - Enables thread-group-scope
InterlockedAccumulate. - Updates lowering, validation, APIs, and tests.
File summaries
| File | Description |
|---|---|
utils/hct/hctdb.py |
Updates DXIL operation metadata. |
utils/hct/gen_intrin_main.txt |
Updates builtin signature. |
tools/clang/unittests/HLSLExec/LinAlgTests.cpp |
Updates execution shaders. |
tools/clang/test/SemaHLSL/hlsl/linalg/builtins/vector-array-errors.hlsl |
Updates diagnostic invocation. |
tools/clang/test/SemaHLSL/hlsl/linalg/builtins/unavailable_pre_sm610.hlsl |
Updates availability test. |
tools/clang/test/SemaHLSL/hlsl/linalg/builtins/stage-errors.hlsl |
Updates stage test. |
tools/clang/test/SemaHLSL/hlsl/linalg/builtins/matrixaccumulatetomemory/vector-array-ast.hlsl |
Updates vector AST expectations. |
tools/clang/test/SemaHLSL/hlsl/linalg/builtins/matrixaccumulatetomemory/unavailable_pre_sm610.hlsl |
Updates availability test. |
tools/clang/test/SemaHLSL/hlsl/linalg/builtins/matrixaccumulatetomemory/ast.hlsl |
Updates AST signature. |
tools/clang/test/SemaHLSL/hlsl/linalg/builtins/matrix-builtins-ast.hlsl |
Updates aggregate AST test. |
tools/clang/test/LitDXILValidation/LinAlgMatrix/linalgmatrix-stage-vs.ll |
Updates VS DXIL signature. |
tools/clang/test/LitDXILValidation/LinAlgMatrix/linalgmatrix-stage-raygeneration.ll |
Updates ray-generation DXIL signature. |
tools/clang/test/LitDXILValidation/LinAlgMatrix/linalgmatrix-stage-ps.ll |
Updates PS DXIL signature. |
tools/clang/test/LitDXILValidation/LinAlgMatrix/linalgmatrix-stage-node.ll |
Updates node DXIL signature. |
tools/clang/test/LitDXILValidation/LinAlgMatrix/linalgmatrix-stage-ms.ll |
Updates mesh DXIL signature. |
tools/clang/test/LitDXILValidation/LinAlgMatrix/linalgmatrix-stage-miss.ll |
Updates miss-shader DXIL signature. |
tools/clang/test/LitDXILValidation/LinAlgMatrix/linalgmatrix-stage-intersection.ll |
Updates intersection DXIL signature. |
tools/clang/test/LitDXILValidation/LinAlgMatrix/linalgmatrix-stage-hs.ll |
Updates hull DXIL signature. |
tools/clang/test/LitDXILValidation/LinAlgMatrix/linalgmatrix-stage-gs.ll |
Updates geometry DXIL signature. |
tools/clang/test/LitDXILValidation/LinAlgMatrix/linalgmatrix-stage-ds.ll |
Updates domain DXIL signature. |
tools/clang/test/LitDXILValidation/LinAlgMatrix/linalgmatrix-stage-cs.ll |
Updates compute DXIL signature. |
tools/clang/test/LitDXILValidation/LinAlgMatrix/linalgmatrix-stage-closesthit.ll |
Updates closest-hit DXIL signature. |
tools/clang/test/LitDXILValidation/LinAlgMatrix/linalgmatrix-stage-callable.ll |
Updates callable DXIL signature. |
tools/clang/test/LitDXILValidation/LinAlgMatrix/linalgmatrix-stage-as.ll |
Updates amplification DXIL signature. |
tools/clang/test/LitDXILValidation/LinAlgMatrix/linalgmatrix-stage-anyhit.ll |
Updates any-hit DXIL signature. |
tools/clang/test/LitDXILValidation/LinAlgMatrix/linalgmatrix-matrixaccumulatetomemory.ll |
Revises validation cases. |
tools/clang/test/LitDXILValidation/LinAlgMatrix/linalgmatrix-groupshared-vector-memory.ll |
Updates valid vector-memory operation. |
tools/clang/test/LitDXILValidation/LinAlgMatrix/linalgmatrix-groupshared-vector-memory-invalid.ll |
Updates invalid vector-memory operation. |
tools/clang/test/CodeGenDXIL/hlsl/linalg/builtins/matrixaccumulatetomemory/vector-array.hlsl |
Updates vector codegen expectations. |
tools/clang/test/CodeGenDXIL/hlsl/linalg/builtins/matrixaccumulatetomemory/nominal.hlsl |
Updates nominal codegen expectations. |
tools/clang/test/CodeGenDXIL/hlsl/linalg/api/matrix-groupshared-vector-array.hlsl |
Updates API vector-array codegen. |
tools/clang/test/CodeGenDXIL/hlsl/linalg/api/matrix-class.hlsl |
Adds thread-group codegen coverage. |
tools/clang/lib/Headers/hlsl/dx/linalg.h |
Revises public accumulation overloads. |
lib/HLSL/HLOperationLower.cpp |
Removes target type during lowering. |
lib/DxilValidation/DxilValidation.cpp |
Removes target-type validation. |
lib/DXIL/DxilOperations.cpp |
Updates operation arity. |
include/dxc/DXIL/DxilInstructions.h |
Updates operand accessors and count. |
Review details
- Files reviewed: 37/37 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
379
to
+380
| template <ComponentEnum TargetCompTy = ComponentTy, typename T, | ||
| MatrixUseEnum UseLocal = Use, MatrixScopeEnum ScopeLocal = Scope, | ||
| SIZE_TYPE Size> | ||
| MatrixUseEnum UseLocal = Use, SIZE_TYPE Size> |
There was a problem hiding this comment.
This one would be good to fix.
Collaborator
Author
There was a problem hiding this comment.
Needs to be fixed in the spec PR. I've copied the comment over there
| // CHECK-SAME: float addrspace(3)* getelementptr inbounds ([256 x float], | ||
| // CHECK-SAME: [256 x float] addrspace(3)* @"\01?SharedArr@@3PAMA", i32 0, i32 0), i32 0, i32 16, i32 1) | ||
| // CHECK-SAME: ; LinAlgMatrixAccumulateToMemory(matrix,memory,offset,stride,layout) | ||
| TGAccMat.InterlockedAccumulate(SharedArr, 0, 16, MatrixLayoutEnum::ColMajor); |
There was a problem hiding this comment.
Doesn't need to be in this PR but would be good to make sure there's an issue tracking it.
Collaborator
Author
There was a problem hiding this comment.
Copilot started reviewing on behalf of
Ashley Coleman (V-FEXrt)
September 4, 2026 16:44
View session
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The API retains an ignored target-type template argument, and the user-visible change lacks a preview release note.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
tools/clang/test/CodeGenDXIL/hlsl/linalg/api/matrix-class.hlsl:179
- This adds user-visible experimental SM 6.10 behavior, but the PR does not add a release note. Please document the removed target-type operand and thread-group accumulation support under
### Upcoming Preview Releaseindocs/ReleaseNotes.md, as required by the repository's release-note policy.
tools/clang/lib/Headers/hlsl/dx/linalg.h:380
TargetCompTyis now unused, yet callers can still writeInterlockedAccumulate<ComponentType::I32>(as the updated test does). That explicit type is silently ignored and produces the same operation as the default overload, which can change the meaning of existing source without a diagnostic. Remove this template parameter and update explicit call sites so obsolete target-type requests are rejected rather than accepted as no-ops.
template <ComponentEnum TargetCompTy = ComponentTy, typename T,
MatrixUseEnum UseLocal = Use, SIZE_TYPE Size>
- Files reviewed: 37/37 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Jesse Natalie (jenatali)
approved these changes
Sep 4, 2026
Jesse Natalie (jenatali)
left a comment
Jesse Natalie (jenatali)
left a comment
Member
There was a problem hiding this comment.
One bot comment worth fixing, otherwise LGTM.
Chris B (llvm-beanz)
approved these changes
Sep 4, 2026
Collaborator
|
LGTM. Sorry for the spec churn. |
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.
Implements the spec changes as defined by microsoft/hlsl-specs#927
dx.op.linAlgMatrixAccumulateToMemoryInterlockedAccumulateto gs memoryCo-authored-by: Copilot [email protected]