-
Notifications
You must be signed in to change notification settings - Fork 898
[SM6.10][Exec][Bugfix] Add matrix layout conversion helpers, add post-dispatch callback, and re-enable OuterProduct test #8606
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d45ae90
c5f5a56
766b220
2a915ec
6ea2dbb
5b626d2
d62a82d
49e9e86
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -349,9 +349,7 @@ class DxilConf_SM610_LinAlg { | |
| // Matrix Vector Arithmetic | ||
| TEST_METHOD(MatVecMul_Thread_16x16_F16); | ||
| TEST_METHOD(MatVecMulAdd_Thread_16x16_F16); | ||
| #if 0 | ||
| TEST_METHOD(OuterProduct_Thread_16x16_F16); | ||
| #endif | ||
|
|
||
| // Query Accumulator Layout | ||
| TEST_METHOD(QueryAccumLayout); | ||
|
|
@@ -1321,7 +1319,29 @@ void DxilConf_SM610_LinAlg::MatVecMulAdd_Thread_16x16_F16() { | |
| ComponentType::F16); | ||
| } | ||
|
|
||
| #if 0 | ||
| // Map a DXIL ComponentType to the D3D12 linear-algebra datatype used by the | ||
| // host-side matrix conversion API. | ||
| #if defined(DIRECT3D_LINEAR_ALGEBRA) | ||
| static D3D12_LINEAR_ALGEBRA_DATATYPE toLinAlgDataType(ComponentType CT) { | ||
| switch (CT) { | ||
| case ComponentType::F16: | ||
| return D3D12_LINEAR_ALGEBRA_DATATYPE_FLOAT16; | ||
| case ComponentType::F32: | ||
| return D3D12_LINEAR_ALGEBRA_DATATYPE_FLOAT32; | ||
| case ComponentType::I16: | ||
| return D3D12_LINEAR_ALGEBRA_DATATYPE_SINT16; | ||
| case ComponentType::U16: | ||
| return D3D12_LINEAR_ALGEBRA_DATATYPE_UINT16; | ||
| case ComponentType::I32: | ||
| return D3D12_LINEAR_ALGEBRA_DATATYPE_SINT32; | ||
| case ComponentType::U32: | ||
| return D3D12_LINEAR_ALGEBRA_DATATYPE_UINT32; | ||
| default: | ||
| VERIFY_IS_TRUE(false, "Unsupported component type for linalg conversion"); | ||
| return D3D12_LINEAR_ALGEBRA_DATATYPE_FLOAT16; | ||
| } | ||
| } | ||
|
|
||
| static const char OuterProductShader[] = R"( | ||
| #define USE_A 0 | ||
| #define SCOPE_THREAD 0 | ||
|
|
@@ -1348,18 +1368,35 @@ static const char OuterProductShader[] = R"( | |
| Mat; | ||
| __builtin_LinAlg_MatrixOuterProduct(Mat, VecA, VecB); | ||
|
|
||
| // Outer product accumulators are stored in the OuterProductOptimal layout | ||
| // with stride 0 and no alignment requirement (align 0), matching the | ||
| // dx::linalg header's thread-scoped InterlockedAccumulate. | ||
| __builtin_LinAlg_MatrixAccumulateToDescriptor( | ||
| Mat, Output, 0, STRIDE, LAYOUT, 128); | ||
| Mat, Output, 0, STRIDE, LAYOUT, 0); | ||
| } | ||
| )"; | ||
|
|
||
| static void runOuterProduct(ID3D12Device *Device, | ||
| dxc::SpecificDllLoader &DxcSupport, | ||
| const MatrixParams &Params, bool Verbose) { | ||
| VERIFY_IS_TRUE( | ||
| Params.Layout == LinalgMatrixLayout::OuterProductOptimal, | ||
| "Outer product must output its matrix in OuterProductOptimal layout"); | ||
| const size_t NumVecElements = Params.M + Params.N; | ||
| const size_t InBuffSize = NumVecElements * elementSize(Params.CompType); | ||
| const size_t NumMatElements = Params.totalElements(); | ||
| const size_t OutBufferSize = Params.totalBytes(); | ||
| const D3D12_LINEAR_ALGEBRA_DATATYPE DataType = | ||
| toLinAlgDataType(Params.CompType); | ||
|
|
||
| const UINT OutBufferSize = getLinAlgMatrixByteSize( | ||
| Device, Params.M, Params.N, DataType, | ||
| D3D12_LINEAR_ALGEBRA_MATRIX_LAYOUT_OUTER_PRODUCT_OPTIMAL, /*Stride=*/0); | ||
|
|
||
| const UINT RowMajorStride = | ||
| static_cast<UINT>(Params.N * elementSize(Params.CompType)); | ||
| const UINT RowMajorSize = getLinAlgMatrixByteSize( | ||
| Device, Params.M, Params.N, DataType, | ||
| D3D12_LINEAR_ALGEBRA_MATRIX_LAYOUT_ROW_MAJOR, RowMajorStride); | ||
|
|
||
| std::string Args = buildCompilerArgs(Params); | ||
|
|
||
|
|
@@ -1371,7 +1408,8 @@ static void runOuterProduct(ID3D12Device *Device, | |
| auto Op = createComputeOp(OuterProductShader, "cs_6_10", "UAV(u0), UAV(u1)", | ||
| Args.c_str()); | ||
| addUAVBuffer(Op.get(), "Input", InBuffSize, false, "byname"); | ||
| addUAVBuffer(Op.get(), "Output", OutBufferSize, true); | ||
| addUAVBuffer(Op.get(), "Output", OutBufferSize, /*ReadBack=*/false); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe I'm wrong, but I see you using GetResource on the "Output" buffer, in the code below. Doesn't that mean that this ReadBack bool should remain true?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The "Output" buffer is not being read by the CPU, so the ReadBack bool is false. The "Output" buffer is also in the opaque outer-product-optimal layout, so we shouldn't even try to interpret it without first converting it to a layout we do know how to read. The GetResource just obtains a handle to be able to refer to the "Output" buffer so that we can record a ConvertLinearAlgebraMatrix command into the command list to tell the GPU to convert it into row-major order and store it into "OutputRowMajor", which the CPU does read back. |
||
| addUAVBuffer(Op.get(), "OutputRowMajor", RowMajorSize, /*ReadBack=*/true); | ||
| addRootView(Op.get(), 0, "Input"); | ||
| addRootView(Op.get(), 1, "Output"); | ||
|
|
||
|
|
@@ -1383,27 +1421,57 @@ static void runOuterProduct(ID3D12Device *Device, | |
| NumVecElements, | ||
| /*StartingVal=*/2, /*Increment=*/false), | ||
| "Saw unsupported component type"); | ||
| }, | ||
| [OutBufferSize, RowMajorSize, RowMajorStride, DataType, | ||
| Params](ID3D12GraphicsCommandList *List, st::ShaderOpTest *Test) { | ||
| ID3D12Resource *OptimalBuffer = nullptr; | ||
| ID3D12Resource *RowMajorBuffer = nullptr; | ||
| Test->GetResource("Output", &OptimalBuffer); | ||
| Test->GetResource("OutputRowMajor", &RowMajorBuffer); | ||
| recordLinAlgMatrixConversion( | ||
| List, OptimalBuffer, OutBufferSize, RowMajorBuffer, RowMajorSize, | ||
| Params.M, Params.N, DataType, | ||
| D3D12_LINEAR_ALGEBRA_MATRIX_LAYOUT_OUTER_PRODUCT_OPTIMAL, | ||
| /*SrcStride=*/0, D3D12_LINEAR_ALGEBRA_MATRIX_LAYOUT_ROW_MAJOR, | ||
| RowMajorStride); | ||
| }); | ||
|
|
||
| MappedData OutData; | ||
| Result->Test->GetReadBackData("Output", &OutData); | ||
| Result->Test->GetReadBackData("OutputRowMajor", &OutData); | ||
|
|
||
| VERIFY_IS_TRUE(verifyComponentBuffer(Params.CompType, OutData.data(), | ||
| Expected, NumMatElements, Verbose)); | ||
| } | ||
| #endif // defined(DIRECT3D_LINEAR_ALGEBRA) | ||
|
|
||
| void DxilConf_SM610_LinAlg::OuterProduct_Thread_16x16_F16() { | ||
| #if defined(DIRECT3D_LINEAR_ALGEBRA) | ||
| MatrixParams Params = {}; | ||
| Params.CompType = ComponentType::F16; | ||
| Params.M = 16; | ||
| Params.N = 16; | ||
| Params.Scope = MatrixScope::Thread; | ||
| Params.Layout = LinalgMatrixLayout::RowMajor; | ||
| Params.Layout = LinalgMatrixLayout::OuterProductOptimal; | ||
| Params.NumThreads = 1; | ||
| Params.Enable16Bit = true; | ||
| runOuterProduct(D3DDevice, DxcSupport, Params, VerboseLogging); | ||
| #else | ||
| #ifdef _HLK_CONF | ||
| // HLK forbids skipping, so treat the missing linear-algebra matrix-conversion | ||
| // API as a failure rather than emitting a (compiled-out) skip. | ||
| hlsl_test::LogErrorFmt(L"OuterProduct_Thread_16x16_F16 requires the " | ||
| L"linear-algebra matrix-conversion API " | ||
| L"(DIRECT3D_LINEAR_ALGEBRA), which this build lacks"); | ||
| #else | ||
| WEX::Logging::Log::Comment( | ||
|
V-FEXrt marked this conversation as resolved.
|
||
| L"Skipping OuterProduct_Thread_16x16_F16: built against a D3D12 SDK " | ||
| L"without the linear-algebra matrix-conversion API " | ||
| L"(DIRECT3D_LINEAR_ALGEBRA undefined); the host-side conversion helpers " | ||
| L"are compiled out."); | ||
| WEX::Logging::Log::Result(WEX::Logging::TestResults::Skipped); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can't skip HLK tests. Only exec tests. |
||
| #endif // _HLK_CONF | ||
| #endif // defined(DIRECT3D_LINEAR_ALGEBRA) | ||
| } | ||
| #endif | ||
|
|
||
| static const char QueryAccumLayoutShader[] = R"( | ||
| RWByteAddressBuffer Output : register(u0); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems to be assuming that
Params.Layoutis always RowMajor? That's fine but we should probably assert or maybe EXPECT_TRUE that assumption!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(we are actually expecting OuterProductOptimal, but still we should ensure that to avoid future issues with M/N swaps :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing you want me to assert
Params.Layout == OuterProductOptimalbefore performing the conversion from OuterProductOptimal to RowMajor?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep! Just so that it gets called out if someone changes it without realizing that the test won't respect the value