[SM6.10] LinAlg Validation: MatrixStoreToMemory - #8834
[SM6.10] LinAlg Validation: MatrixStoreToMemory#8834Ashley Coleman (V-FEXrt) wants to merge 1 commit into
Conversation
Fixes #8498 Implements LinAlg MatrixStoreToMemory validation rules
There was a problem hiding this comment.
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); | |||
| GEPOperator *GSGEP = cast<GEPOperator>(Op.get_memory()); | ||
| GlobalVariable *GSMem = cast<GlobalVariable>(GSGEP->getPointerOperand()); |
| GlobalVariable *GSMem = cast<GlobalVariable>(GSGEP->getPointerOperand()); | ||
| Type *GSMemInnerTy = GSMem->getType(); | ||
| unsigned GSScalarCount = 1; | ||
| if (PointerType *GSMemPtrTy = dyn_cast<PointerType>(GSMemInnerTy)) |
There was a problem hiding this comment.
Consider making this an unconditional cast ? Or assert if this fails?
|
This was ported over from #8824 going to copy comments over |
| CI, ValidationRule::InstrLinAlgMatrixScopeMismatch2, | ||
| {"Input", MatrixScopeToString(Mat->Scope), "Wave", "ThreadGroup"}); | ||
|
|
||
| GEPOperator *GSGEP = cast<GEPOperator>(Op.get_memory()); |
There was a problem hiding this comment.
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
| unsigned GSScalarCount = 1; | ||
| if (PointerType *GSMemPtrTy = dyn_cast<PointerType>(GSMemInnerTy)) | ||
| GSMemInnerTy = GSMemPtrTy->getPointerElementType(); | ||
| if (ArrayType *GSMemArrTy = dyn_cast<ArrayType>(GSMemInnerTy)) { |
There was a problem hiding this comment.
From Alex: Should we check if nested arrarys are permitted?
| return OS.str(); | ||
| } | ||
|
|
||
| bool IsComponentTypeSameNativeType(DXIL::ComponentType CT, llvm::Type *Ty) { |
There was a problem hiding this comment.
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
Damyan Pepper (damyanp)
left a comment
Damyan Pepper (damyanp)
left a comment
There was a problem hiding this comment.
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.)
Fixes #8498
Implements LinAlg MatrixStoreToMemory validation rules
Stack created with GitHub Stacks CLI • Give Feedback 💬