[SM6.10] LinAlg Validation: MatrixStoreToMemory by V-FEXrt · Pull Request #8834 · microsoft/DirectXShaderCompiler

Ashley Coleman (V-FEXrt) · 2026-08-26T17:41:28Z

Fixes #8498

Implements LinAlg MatrixStoreToMemory validation rules

_{Stack created with GitHub Stacks CLI • Give Feedback 💬}

Fixes #8498 Implements LinAlg MatrixStoreToMemory validation rules

Copilot

Pull request overview

Implements Shader Model 6.10 validation for LinAlgMatrixStoreToMemory.

Changes:

Validates matrix scope, groupshared type/capacity, offset, and stride.
Adds validation rules, diagnostics, and type utilities.
Adds validation coverage and updates CodeGen fixtures.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`utils/hct/hctdb.py`	Defines new validation diagnostics.
`tools/clang/test/LitDXILValidation/LinAlgMatrix/linalgmatrix-matrixstoretomemory.ll`	Tests store validation rules.
`tools/clang/test/CodeGenDXIL/hlsl/linalg/builtins/matrixstoretomemory/vector-array.hlsl`	Updates vector-array fixture.
`tools/clang/test/CodeGenDXIL/hlsl/linalg/builtins/matrixstoretomemory/nominal.hlsl`	Updates nominal fixture.
`lib/DxilValidation/DxilValidationUtils.h`	Declares native-type comparison helper.
`lib/DxilValidation/DxilValidationUtils.cpp`	Implements component/native-type matching.
`lib/DxilValidation/DxilValidation.cpp`	Implements store-to-memory validation.
`docs/DXIL.rst`	Documents new validation rules.

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

@@ -1225,6 +1226,66 @@ static void ValidateLinAlgMatrixStoreToDescriptor(CallInst *CI,
 static void ValidateLinAlgMatrixStoreToMemory(CallInst *CI,
                                              ValidationContext &ValCtx) {
  ValidateLinAlgOpParameters(CI, ValCtx);


Chris B (llvm-beanz) · 2026-09-01T19:44:04Z

+  GEPOperator *GSGEP = cast<GEPOperator>(Op.get_memory());
+  GlobalVariable *GSMem = cast<GlobalVariable>(GSGEP->getPointerOperand());


I think this comment is technically correct in that we don't disallow such a formation, but I'm not actually sure it is possible to generate such code from the frontend.

Joshua Batista (bob80905) · 2026-08-26T18:01:00Z

+  GlobalVariable *GSMem = cast<GlobalVariable>(GSGEP->getPointerOperand());
+  Type *GSMemInnerTy = GSMem->getType();
+  unsigned GSScalarCount = 1;
+  if (PointerType *GSMemPtrTy = dyn_cast<PointerType>(GSMemInnerTy))


Consider making this an unconditional cast ? Or assert if this fails?

Ashley Coleman (V-FEXrt) · 2026-08-26T22:54:53Z

This was ported over from #8824 going to copy comments over

Ashley Coleman (V-FEXrt) · 2026-08-26T22:55:55Z

+        CI, ValidationRule::InstrLinAlgMatrixScopeMismatch2,
+        {"Input", MatrixScopeToString(Mat->Scope), "Wave", "ThreadGroup"});
+
+  GEPOperator *GSGEP = cast<GEPOperator>(Op.get_memory());


Chris B (@llvm-beanz) I'm pretty sure there is a better way to pull the inner most type out from the memory operator here but I just wanted to get something written down to unblock progress.

Does this seem right or should I do something else here. also can we even assume its always a GEP? Copilot seems to say no but I'm not sure what else it would be

You can simplify it a bit using getSequentialElementType(). Something like:

while(SequentialType* ST = dyn_cast<SequentialType>(GSMemInnerTy)) GSMemInnerTy = ST->getSequentialElementType();

That will walk the types up because getSequentialElementType works for pointers, arrays, and vectors.

Ashley Coleman (V-FEXrt) · 2026-08-26T22:56:20Z

+  unsigned GSScalarCount = 1;
+  if (PointerType *GSMemPtrTy = dyn_cast<PointerType>(GSMemInnerTy))
+    GSMemInnerTy = GSMemPtrTy->getPointerElementType();
+  if (ArrayType *GSMemArrTy = dyn_cast<ArrayType>(GSMemInnerTy)) {


From Alex: Should we check if nested arrarys are permitted?

this will be resolved by the loop version Chris provided above

Ashley Coleman (V-FEXrt) · 2026-08-26T22:56:52Z

  return OS.str();
 }

+bool IsComponentTypeSameNativeType(DXIL::ComponentType CT, llvm::Type *Ty) {


Chris B (@llvm-beanz) I think this is necessary since we don't really have a mapping for ComponentType to llvm::Type but figured I'd specifically highlight it since imo it's not trivially correct

seems reasonable to me

Damyan Pepper (damyanp)

LGTM, but I suspect you will want to get an answer to https://github.com/microsoft/DirectXShaderCompiler/pull/8834/changes#r3867347681 (or address it in a follow-up.)

Chris B (llvm-beanz)

Left a few comments. Take or leave the feedback. The only one that probably matters a bit is that maybe looping to get the sequential element type is a good changes since it does handle more cases and is simpler.

Chris B (llvm-beanz) · 2026-09-01T19:41:39Z

+        CI, ValidationRule::InstrLinAlgMatrixScopeMismatch2,
+        {"Input", MatrixScopeToString(Mat->Scope), "Wave", "ThreadGroup"});
+
+  GEPOperator *GSGEP = cast<GEPOperator>(Op.get_memory());


You can simplify it a bit using getSequentialElementType(). Something like:

while(SequentialType* ST = dyn_cast<SequentialType>(GSMemInnerTy)) GSMemInnerTy = ST->getSequentialElementType();

That will walk the types up because getSequentialElementType works for pointers, arrays, and vectors.

Chris B (llvm-beanz) · 2026-09-01T19:42:37Z

  return OS.str();
 }

+bool IsComponentTypeSameNativeType(DXIL::ComponentType CT, llvm::Type *Ty) {


seems reasonable to me

Chris B (llvm-beanz) · 2026-09-01T19:44:04Z

+  GEPOperator *GSGEP = cast<GEPOperator>(Op.get_memory());
+  GlobalVariable *GSMem = cast<GlobalVariable>(GSGEP->getPointerOperand());


I think this comment is technically correct in that we don't disallow such a formation, but I'm not actually sure it is possible to generate such code from the frontend.

Ashley Coleman (V-FEXrt) · 2026-09-01T21:29:48Z

Perfect! That's exactly the kind of feedback I was looking for:)

I'm going to go ahead and merge the PR so I can make progress on the stack but I'll fix this as a follow up once everything goes in. (This change applies to a few of the stacked PRs)

Jesse Natalie (jenatali) · 2026-09-03T18:52:29Z

+  // if it is constant then offset must be 128-byte aligned
+  if (ConstantInt *OffsetV = dyn_cast<ConstantInt>(Op.get_offset())) {
+    unsigned Offset = OffsetV->getLimitedValue();
+    if (Offset % 128 != 0)


Per spec:

For the Store operation on groupshared arrays the Offset and Stride parameters are the number of scalar elements of the scalar element type of the matrix. Meaning if the array is an i32 array, and the matrix is i8, the Offset and Stride are in terms of 8-bit elements.

The offset value must be 128-byte aligned from the base offset, and the stride must be 16-byte aligned.

Directly checking Offset % 128 checks if the offset is 128-element-aligned, not 128-byte-aligned. It needs to be multiplied by element size before this check. Same for the stride.

[SM6.10] LinAlg Validation: MatrixStoreToMemory

c86ad69

Fixes #8498 Implements LinAlg MatrixStoreToMemory validation rules

Copilot AI balanced review requested due to automatic review settings August 26, 2026 17:41

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

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

Copilot started reviewing on behalf of Ashley Coleman (V-FEXrt) August 26, 2026 17:42 View session

Copilot AI reviewed Aug 26, 2026

View reviewed changes

Joshua Batista (bob80905) reviewed Aug 26, 2026

View reviewed changes

Joshua Batista (bob80905) approved these changes Aug 26, 2026

View reviewed changes

Ashley Coleman (V-FEXrt) commented Aug 26, 2026

View reviewed changes

Damyan Pepper (damyanp) reviewed Aug 27, 2026

View reviewed changes

Damyan Pepper (damyanp) mentioned this pull request Aug 27, 2026

[SM6.10] LinAlg Validation: MatrixAccumulateToMemory #8835

Merged

Chris B (llvm-beanz) approved these changes Sep 1, 2026

View reviewed changes

Ashley Coleman (V-FEXrt) merged commit fe09c22 into main Sep 1, 2026
17 checks passed

Ashley Coleman (V-FEXrt) deleted the linalg-vali-matrixstoretomemory branch September 1, 2026 21:35

github-project-automation Bot moved this from New to Done in HLSL Roadmap Sep 1, 2026

Jesse Natalie (jenatali) reviewed Sep 3, 2026

View reviewed changes

		GEPOperator *GSGEP = cast<GEPOperator>(Op.get_memory());
		GlobalVariable *GSMem = cast<GlobalVariable>(GSGEP->getPointerOperand());

Uh oh!

Conversation

Ashley Coleman (V-FEXrt) commented Aug 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ashley Coleman (V-FEXrt) commented Aug 26, 2026

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!

Damyan Pepper (damyanp) left a comment

Choose a reason for hiding this comment

Uh oh!

Chris B (llvm-beanz) 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!

Ashley Coleman (V-FEXrt) commented Sep 1, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants