[SM6.10][HLK] Add LinAlg execution test infrastructure with Load/Store/Splat tests#8285
Open
V-FEXrt wants to merge 1 commit intomicrosoft:mainfrom
Open
[SM6.10][HLK] Add LinAlg execution test infrastructure with Load/Store/Splat tests#8285V-FEXrt wants to merge 1 commit intomicrosoft:mainfrom
V-FEXrt wants to merge 1 commit 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 <223556219+Copilot@users.noreply.github.com>
Contributor
You can test this locally with the following command:git-clang-format --diff 54afd2c6a5f1a5bcbef66967515fa69f5a3650fd babf3f70c05e6b5b4dd11d27a4c1f8c22dfb8d1b -- tools/clang/unittests/HLSLExec/LinAlgTests.cppView the diff from clang-format here.diff --git a/tools/clang/unittests/HLSLExec/LinAlgTests.cpp b/tools/clang/unittests/HLSLExec/LinAlgTests.cpp
index 4c11460a..619fc486 100644
--- a/tools/clang/unittests/HLSLExec/LinAlgTests.cpp
+++ b/tools/clang/unittests/HLSLExec/LinAlgTests.cpp
@@ -144,9 +144,8 @@ runShaderOp(ID3D12Device *Device, dxc::SpecificDllLoader &DxcSupport,
auto OpSet = std::make_shared<st::ShaderOpSet>();
OpSet->ShaderOps.push_back(std::move(Op));
- return st::RunShaderOpTestAfterParse(Device, DxcSupport, nullptr,
- std::move(InitCallback),
- std::move(OpSet));
+ return st::RunShaderOpTestAfterParse(
+ Device, DxcSupport, nullptr, std::move(InitCallback), std::move(OpSet));
}
// ===========================================================================
@@ -214,10 +213,10 @@ static void compileShader(dxc::SpecificDllLoader &DxcSupport,
// Compiler arguments builder
// ===========================================================================
-static std::string
-buildCompilerArgs(int CompType, int M, int N, int Use, int Scope, int Stride,
- int Layout, int NumThreads, bool Enable16Bit = false,
- const char *ExtraDefines = nullptr) {
+static std::string buildCompilerArgs(int CompType, int M, int N, int Use,
+ int Scope, int Stride, int Layout,
+ int NumThreads, bool Enable16Bit = false,
+ const char *ExtraDefines = nullptr) {
std::stringstream SS;
SS << "-HV 202x";
SS << " -DCOMP_TYPE=" << CompType;
@@ -324,11 +323,9 @@ struct MatrixParams {
class DxilConf_SM610_LinAlg {
public:
BEGIN_TEST_CLASS(DxilConf_SM610_LinAlg)
- TEST_CLASS_PROPERTY(
- "Kits.TestName",
- "D3D12 - Shader Model 6.10 - LinAlg Matrix Operations")
- TEST_CLASS_PROPERTY("Kits.TestId",
- "a1b2c3d4-e5f6-7890-abcd-ef1234567890")
+ TEST_CLASS_PROPERTY("Kits.TestName",
+ "D3D12 - Shader Model 6.10 - LinAlg Matrix Operations")
+ TEST_CLASS_PROPERTY("Kits.TestId", "a1b2c3d4-e5f6-7890-abcd-ef1234567890")
TEST_CLASS_PROPERTY(
"Kits.Description",
"Validates SM 6.10 linear algebra matrix operations execute correctly")
@@ -387,9 +384,8 @@ bool DxilConf_SM610_LinAlg::setupClass() {
if (!D3D12SDK->createDevice(&D3DDevice, D3D_SHADER_MODEL_6_10,
/*SkipUnsupported=*/false)) {
if (FailIfRequirementsNotMet) {
- hlsl_test::LogErrorFmt(
- L"Device creation failed for SM 6.10, and "
- L"FailIfRequirementsNotMet is set.");
+ hlsl_test::LogErrorFmt(L"Device creation failed for SM 6.10, and "
+ L"FailIfRequirementsNotMet is set.");
return false;
}
// No device — tests will compile shaders and skip execution.
@@ -438,10 +434,9 @@ static void runLoadStoreRoundtrip(ID3D12Device *Device,
const size_t BufferSize = Params.totalBytes();
const int Stride = Params.strideBytes();
- std::string Args =
- buildCompilerArgs(Params.CompType, Params.M, Params.N, Params.Use,
- Params.Scope, Stride, Params.Layout,
- Params.NumThreads, Params.Enable16Bit);
+ std::string Args = buildCompilerArgs(
+ Params.CompType, Params.M, Params.N, Params.Use, Params.Scope, Stride,
+ Params.Layout, Params.NumThreads, Params.Enable16Bit);
// Always verify the shader compiles.
compileShader(DxcSupport, LoadStoreShader, "cs_6_10", Args);
@@ -553,10 +548,10 @@ static void runSplatStore(ID3D12Device *Device,
std::stringstream ExtraDefs;
ExtraDefs << "-DFILL_VALUE=" << FillValue;
- std::string Args = buildCompilerArgs(
- Params.CompType, Params.M, Params.N, Params.Use, Params.Scope, Stride,
- Params.Layout, Params.NumThreads, Params.Enable16Bit,
- ExtraDefs.str().c_str());
+ std::string Args =
+ buildCompilerArgs(Params.CompType, Params.M, Params.N, Params.Use,
+ Params.Scope, Stride, Params.Layout, Params.NumThreads,
+ Params.Enable16Bit, ExtraDefs.str().c_str());
// Always verify the shader compiles.
compileShader(DxcSupport, SplatStoreShader, "cs_6_10", Args);
|
alsepkow
reviewed
Mar 21, 2026
Comment on lines
+23
to
+25
| #ifndef NOMINMAX | ||
| #define NOMINMAX 1 | ||
| #endif |
Contributor
There was a problem hiding this comment.
Looks like copilot decided you need these? You should only need these if you're calling std::min/max and including windows.h
alsepkow
reviewed
Mar 21, 2026
Comment on lines
+10
to
+13
| // Unlike older execution tests, these tests define shader source and // | ||
| // resources entirely in C++ — no ShaderOpArith.xml entries required. // | ||
| // ShaderOp objects are constructed programmatically and passed to the // | ||
| // existing RunShaderOpTestAfterParse infrastructure. // |
Contributor
There was a problem hiding this comment.
nit: I'd remove this comment. Some of the tests do exactly what these tests are doing.
It's a big ol mixing pot.
Comment on lines
+565
to
+570
| if (!Device) { | ||
| hlsl_test::LogCommentFmt( | ||
| L"Shader compiled OK; skipping execution (no SM 6.10 device)"); | ||
| WEX::Logging::Log::Result(WEX::Logging::TestResults::Skipped); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
Suggested change
| if (!Device) { | |
| hlsl_test::LogCommentFmt( | |
| L"Shader compiled OK; skipping execution (no SM 6.10 device)"); | |
| WEX::Logging::Log::Result(WEX::Logging::TestResults::Skipped); | |
| return; | |
| } |
If we end up in here with no device something went wrong. We definitely don't want to skip. In the HLK this will effectively be a pass. Better to fail out when something tries to use it.
Generally, no skipping in HLK. And skipping in a TAEF test is usually reserved for "Does this system support my test? If not, skip"
Member
There was a problem hiding this comment.
We don't want this test to fail when we run it outside of the HLK though, so we still need the skip in there. We could ifdef it for the HLK.
| } | ||
|
|
||
| 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); |
Contributor
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.
| MappedData OutData; | ||
| Result->Test->GetReadBackData("Output", &OutData); | ||
|
|
||
| if (Params.CompType == CT_F32) { |
Contributor
There was a problem hiding this comment.
nit: Would a switch be cleaner?
| L"FailIfRequirementsNotMet is set."); | ||
| return false; | ||
| } | ||
| // No device — tests will compile shaders and skip execution. |
Contributor
There was a problem hiding this comment.
Suggested change
| // No device — tests will compile shaders and skip execution. |
Comment on lines
+384
to
+394
| // Try to create a device. In HLK mode, fail if unavailable. | ||
| // In dev mode, D3DDevice stays null and tests will compile shaders | ||
| // then skip GPU execution. | ||
| if (!D3D12SDK->createDevice(&D3DDevice, D3D_SHADER_MODEL_6_10, | ||
| /*SkipUnsupported=*/false)) { | ||
| if (FailIfRequirementsNotMet) { | ||
| hlsl_test::LogErrorFmt( | ||
| L"Device creation failed for SM 6.10, and " | ||
| L"FailIfRequirementsNotMet is set."); | ||
| return false; | ||
| } |
Contributor
There was a problem hiding this comment.
Suggested change
| // Try to create a device. In HLK mode, fail if unavailable. | |
| // In dev mode, D3DDevice stays null and tests will compile shaders | |
| // then skip GPU execution. | |
| if (!D3D12SDK->createDevice(&D3DDevice, D3D_SHADER_MODEL_6_10, | |
| /*SkipUnsupported=*/false)) { | |
| if (FailIfRequirementsNotMet) { | |
| hlsl_test::LogErrorFmt( | |
| L"Device creation failed for SM 6.10, and " | |
| L"FailIfRequirementsNotMet is set."); | |
| return false; | |
| } | |
| bool FailIfRequirementsNotMet = false; | |
| #ifdef _HLK_CONF | |
| FailIfRequirementsNotMet = true; | |
| #endif | |
| WEX::TestExecution::RuntimeParameters::TryGetValue( | |
| L"FailIfRequirementsNotMet", FailIfRequirementsNotMet); | |
| const bool SkipUnsupported = !FailIfRequirementsNotMet; | |
| if (!D3D12SDK->createDevice(&D3DDevice, D3D_SHADER_MODEL_6_9, | |
| SkipUnsupported)) { | |
| if (FailIfRequirementsNotMet) | |
| hlsl_test::LogErrorFmt( | |
| L"Device Creation failed, resulting in test failure, since " | |
| L"FailIfRequirementsNotMet is set. The expectation is that this " | |
| L"test will only be executed if something has previously " | |
| L"determined that the system meets the requirements of this " | |
| L"test."); | |
| return false; | |
| } |
Member
There was a problem hiding this comment.
Line 392 of this should be SM 6_10, right?
Comment on lines
+381
to
+382
| WEX::TestExecution::RuntimeParameters::TryGetValue( | ||
| L"FailIfRequirementsNotMet", FailIfRequirementsNotMet); |
Contributor
There was a problem hiding this comment.
Suggested change
| WEX::TestExecution::RuntimeParameters::TryGetValue( | |
| L"FailIfRequirementsNotMet", FailIfRequirementsNotMet); |
Comment on lines
+403
to
+404
| // Re-create device if it was lost. If we never had one, that's fine — | ||
| // tests compile shaders and skip GPU execution. |
Contributor
There was a problem hiding this comment.
Suggested change
| // Re-create device if it was lost. If we never had one, that's fine — | |
| // tests compile shaders and skip GPU execution. |
Comment on lines
+405
to
+411
| if (D3DDevice && D3DDevice->GetDeviceRemovedReason() != S_OK) { | ||
| hlsl_test::LogCommentFmt(L"Device was lost!"); | ||
| D3DDevice.Release(); | ||
| D3D12SDK->createDevice(&D3DDevice, D3D_SHADER_MODEL_6_10, | ||
| /*SkipUnsupported=*/false); | ||
| } | ||
| return true; |
Contributor
There was a problem hiding this comment.
It's more robust separate out the check for device loss and re-creation.
This ensures that if the device was lost and invalidated by something else that we'll still re-create it.
Suggested change
| if (D3DDevice && D3DDevice->GetDeviceRemovedReason() != S_OK) { | |
| hlsl_test::LogCommentFmt(L"Device was lost!"); | |
| D3DDevice.Release(); | |
| D3D12SDK->createDevice(&D3DDevice, D3D_SHADER_MODEL_6_10, | |
| /*SkipUnsupported=*/false); | |
| } | |
| return true; | |
| // It's possible a previous test case caused a device removal. If it did we | |
| // need to try and create a new device. | |
| if (D3DDevice && D3DDevice->GetDeviceRemovedReason() != S_OK) { | |
| hlsl_test::LogCommentFmt(L"Device was lost!"); | |
| D3DDevice.Release(); | |
| } | |
| if (!D3DDevice) { | |
| hlsl_test::LogCommentFmt(L"Creating device"); | |
| // We expect this to succeed, and fail if it doesn't, because classSetup() | |
| // has already ensured that the system configuration meets the | |
| // requirements of all the tests in this class. | |
| const bool SkipUnsupported = false; | |
| VERIFY_IS_TRUE(D3D12SDK->createDevice(&D3DDevice, D3D_SHADER_MODEL_6_10, | |
| SkipUnsupported)); | |
| } | |
| return true; |
damyanp
reviewed
Mar 21, 2026
Comment on lines
+152
to
+159
| // =========================================================================== | ||
| // Shader compilation helper | ||
| // | ||
| // Compiles an HLSL shader using the DXC API to verify it is well-formed. | ||
| // This runs without a D3D12 device, so it works even when no SM 6.10 | ||
| // hardware is available. Fails the test (via VERIFY) on compile error. | ||
| // =========================================================================== | ||
|
|
Member
There was a problem hiding this comment.
We should encourage these to be written using /// style comments.
| // Verification helpers | ||
| // =========================================================================== | ||
|
|
||
| static bool verifyFloatBuffer(const void *Actual, const float *Expected, |
Member
There was a problem hiding this comment.
That might be a case of diminishing returns - the int one doesn't have a tolerance, and they use different format strings for logging them.
| if (!Initialized) { | ||
| Initialized = true; | ||
|
|
||
| // Always initialize DXC compiler — needed for shader compilation checks. |
Member
There was a problem hiding this comment.
Suggested change
| // Always initialize DXC compiler — needed for shader compilation checks. |
I don't think this comment helps much!
Comment on lines
+384
to
+394
| // Try to create a device. In HLK mode, fail if unavailable. | ||
| // In dev mode, D3DDevice stays null and tests will compile shaders | ||
| // then skip GPU execution. | ||
| if (!D3D12SDK->createDevice(&D3DDevice, D3D_SHADER_MODEL_6_10, | ||
| /*SkipUnsupported=*/false)) { | ||
| if (FailIfRequirementsNotMet) { | ||
| hlsl_test::LogErrorFmt( | ||
| L"Device creation failed for SM 6.10, and " | ||
| L"FailIfRequirementsNotMet is set."); | ||
| return false; | ||
| } |
Member
There was a problem hiding this comment.
Line 392 of this should be SM 6_10, right?
Comment on lines
+218
to
+219
| buildCompilerArgs(int CompType, int M, int N, int Use, int Scope, int Stride, | ||
| int Layout, int NumThreads, bool Enable16Bit = false, |
Member
There was a problem hiding this comment.
Could this just take a MatrixParams?
Comment on lines
+48
to
+58
| enum ComponentType { | ||
| CT_I16 = 2, | ||
| CT_U16 = 3, | ||
| CT_I32 = 4, | ||
| CT_U32 = 5, | ||
| CT_I64 = 6, | ||
| CT_U64 = 7, | ||
| CT_F16 = 8, | ||
| CT_F32 = 9, | ||
| CT_F64 = 10, | ||
| }; |
Member
There was a problem hiding this comment.
Can we use DXIL::ComponentType from DxilConstants.h for this?
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
3 participants
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.
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 223556219+Copilot@users.noreply.github.com