[SM6.10][HLK] Add LinAlg execution test infrastructure with Load/Store/Splat tests#8285
[SM6.10][HLK] Add LinAlg execution test infrastructure with Load/Store/Splat tests#8285V-FEXrt merged 16 commits intomicrosoft:mainfrom
Conversation
Introduce LinAlgTests.cpp with a new pattern for execution tests where ShaderOp objects are built programmatically in C++ no ShaderOpArith.xml entries required. Shader source, resources, and root signatures are all defined in the .cpp file. Each test compiles its shader via IDxcCompiler3 to validate the HLSL, then skips GPU dispatch if no SM 6.10 device is available. This ensures shader authoring correctness is always verified. Tests added: - LoadStoreRoundtrip_Wave_F32/I32: MatrixLoadFromDescriptor + MatrixStoreToDescriptor - SplatStore_Wave_F32/I32: FillMatrix + MatrixStoreToDescriptor Co-authored-by: Copilot <[email protected]>
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
| } | ||
|
|
||
| std::vector<float> ExpectedFloats(NumElements, FillValue); | ||
| std::vector<int32_t> ExpectedInts(NumElements, |
|
|
||
| auto Op = | ||
| createComputeOp(SplatStoreShader, "cs_6_10", "UAV(u0)", Args.c_str()); | ||
| addUAVBuffer(Op.get(), "Output", BufferSize, true); |
There was a problem hiding this comment.
nit: We may want to consider adding addUAVBuffer and others to HLSLExecTestUtils.
And there may be potential to update the ShaderOpTest.cpp code to use those helpers.
For this PR I think just putting these generic helpers in HLSLExecTestUtils would be a good first step. They're likely to be useful for future tests looking to follow the same patterns in these tests.
- Remove NOMINMAX block (not needed without std::min/max + windows.h) - Remove 'Unlike older execution tests...' paragraph from file header - Convert block comments to /// doc comment style - Remove unhelpful 'Always initialize DXC compiler' comment - Apply clang-format fixes Co-authored-by: Copilot <[email protected]>
- Replace custom ComponentType/MatrixUse/MatrixScope/MatrixLayout enums with hlsl::DXIL types from DxilConstants.h - Add elemSize() helper to eliminate duplicated element size logic in strideBytes() and totalBytes() - Make buildCompilerArgs take const MatrixParams& instead of individual params - Use switch statements for CompType dispatch instead of if-else chains Co-authored-by: Copilot <[email protected]>
- Update setupClass with SkipUnsupported variable and more informative error message following existing HLK test patterns - Update setupMethod with robust device-loss detection and recreation that fails hard (VERIFY) since a working device existed previously - Fix potential UB in runSplatStore by only constructing the expected int vector when CompType is I32 (avoids unconditional float-to-int cast) Co-authored-by: Copilot <[email protected]>
| /// Return the byte size of a single element for the given component type. | ||
| static int elemSize(ComponentType CT) { | ||
| switch (CT) { | ||
| case ComponentType::F16: |
There was a problem hiding this comment.
How about the 8-bit component types I8, U8, F8_E4M3 and F8_E5M2?
There was a problem hiding this comment.
These are just the first pass of tests, not the final ones! Planning on adding a lot more detailed tests after getting the basic tests/test framework in
Co-authored-by: Alex Sepkowski <[email protected]> Co-authored-by: Ashley Coleman <[email protected]>
| Buf.Size = SourceBlob->GetBufferSize(); | ||
| Buf.Encoding = DXC_CP_UTF8; | ||
|
|
||
| CComPtr<IDxcResult> Result; |
There was a problem hiding this comment.
nit: Don't have to add it this PR. But it might be helpful to log the arguments being passed to the compiler.
|
|
||
| private: | ||
| bool createDevice(); | ||
|
|
There was a problem hiding this comment.
| WEX::TestExecution::SetVerifyOutput VerifyOutput{ | |
| WEX::TestExecution::VerifyOutputSettings::LogOnlyFailures}; |
If you add this member variable it will cut down on the logging of every single verify macro and instead only log it on a failure.
| ExpectedFloats.assign(NumElements, FillValue); | ||
| break; | ||
| case ComponentType::I32: | ||
| ExpectedInts.assign(NumElements, static_cast<int32_t>(FillValue)); |
There was a problem hiding this comment.
nit: this could still potentially produce UB if FillValue can't be represented as an int32. Not a problem right now. But we'll probably want to consider bounds checking on values somewhere at some point to prevent this.
There was a problem hiding this comment.
oh right! I forgot about our discussion here!
alsepkow
left a comment
There was a problem hiding this comment.
LGTM. A few comments I'm ok being addressed on a subsequent change if you want.
|
Sounds good! I'm going to add the remaining comments to the ticket for this work item and we can circle back around to resolve them! I'd like to get something merged so we can have a couple tests landed |
Introduce
LinAlgTests.cppwith a new pattern for execution tests where ShaderOp objects are built programmatically in C++ no ShaderOpArith.xml entries required. Shader source, resources, and root signatures are all defined in the.cppfile.Tests will currently be skipped since no driver reports SM6.10 support
Co-authored-by: Copilot [email protected]