From e01bd39ff32e4441ca32827e07e5282a79135092 Mon Sep 17 00:00:00 2001 From: Jack Elliott Date: Wed, 12 Aug 2026 07:32:55 +1200 Subject: [PATCH 1/2] [HLSL] Add F16 coverage for the LinAlg orthogonal test axes Three behaviour axes in the LinAlg suite were reachable only through F32: descriptor cross-layout round trips, and out-of-bounds element Get and Set. None of those behaviours has anything to do with the element type, but proposal 0029's Minimum Support Set constrains component types and F16 is the only float type a tier is required to support. A conforming F16-only device therefore skips all three axes at the capability gate and loses the coverage entirely, which is the same defect class NVIDIA reported against the previously ungated tests. Adds an F16 sibling for each axis at 16x16, the configuration the rest of the suite leans on hardest. The existing F32 cases are kept rather than converted: 4x8 is rectangular, so its transpose changes the buffer shape and retains rigour a square 16x16 case cannot. The out-of-bounds Get runner asserted F32 because its per-lane record packed the result as a 4-byte element. Rather than branch the record layout by type, the shader now widens both results to float. Only zero and the poison constant are ever compared, and both are exactly representable in F16, so the widening cannot mask a wrong value. No stride rule constrains the new layouts: the validator has no stride predicate and the suite already uses 12, 16, 32 and 48. Validated on WARP against the matched experimental-tier runtime. The suite goes from 27 to 30 GPU tests with the non-passing set unchanged: the same four runtime and WARP spec-skew failures, and the same deliberate OuterProduct skip. Each axis was negative-controlled, because passing first go proves nothing. Dropping the out-of-bounds Get calls left the poison in place and failed both the F32 and the new F16 case with 999.000000, which also confirms the F16 poison round trips the widened store. Making the shader reuse the load layout on the store side failed exactly the two cross-layout cases while both same-layout cases kept passing. Aiming the out-of-bounds Set at an in-range index failed both Set cases. Assisted-by: GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 83725f5d-8e98-4c1d-91ee-ad47629e007b --- .../clang/unittests/HLSLExec/LinAlgTests.cpp | 107 +++++++++++++++++- 1 file changed, 103 insertions(+), 4 deletions(-) diff --git a/tools/clang/unittests/HLSLExec/LinAlgTests.cpp b/tools/clang/unittests/HLSLExec/LinAlgTests.cpp index baf56d2c21..a7fee2df9c 100644 --- a/tools/clang/unittests/HLSLExec/LinAlgTests.cpp +++ b/tools/clang/unittests/HLSLExec/LinAlgTests.cpp @@ -2118,6 +2118,7 @@ class DxilConf_SM610_LinAlg { TEST_METHOD(LoadStoreDescriptor_Wave_16x16_F16); TEST_METHOD(LoadStoreDescriptor_Wave_4x8_F16_RowMajorOffsetPadded); TEST_METHOD(LoadStoreDescriptor_Wave_4x8_F32_RowMajorToColumnMajor); + TEST_METHOD(LoadStoreDescriptor_Wave_16x16_F16_RowMajorToColumnMajor); TEST_METHOD(LoadDescriptorOOB_Wave_16x16_F16_PartialView); TEST_METHOD(LoadDescriptorOOB_Wave_4x8_F16_OffsetPaddedPartialView); TEST_METHOD(SplatStore_Wave_16x16_F16); @@ -2134,6 +2135,8 @@ class DxilConf_SM610_LinAlg { TEST_METHOD(ElementSet_Wave_16x16_F16); TEST_METHOD(ElementGetOOB_Wave_4x8_F32); TEST_METHOD(ElementSetOOB_Wave_4x8_F32); + TEST_METHOD(ElementGetOOB_Wave_16x16_F16); + TEST_METHOD(ElementSetOOB_Wave_16x16_F16); // Cast/Convert TEST_METHOD(CopyConvert_Wave_16x16_F16); @@ -2562,6 +2565,51 @@ void DxilConf_SM610_LinAlg:: VerboseLogging, SelectedWaveSize); } +// The same cross-layout axis on F16. Proposal 0029's Minimum Support Set +// constrains component types only, and F16 is the one float type any tier is +// required to support, so the F32 case above can skip in its entirety on a +// conforming device and take the layout coverage with it. Layout handling has +// nothing to do with the element type, so it should not be reachable only +// through an optional one. +void DxilConf_SM610_LinAlg:: + LoadStoreDescriptor_Wave_16x16_F16_RowMajorToColumnMajor() { + MatrixParams Params = {}; + Params.CompType = ComponentType::F16; + Params.M = 16; + Params.N = 16; + Params.Use = MatrixUse::A; + Params.Scope = MatrixScope::Wave; + Params.Layout = MatrixLayout::RowMajor; + Params.NumThreads = 128; + Params.Enable16Bit = true; + + UINT SelectedWaveSize = 0; + if (!matrixConstructionApplicable( + D3DDevice, Params, {Params.Use}, + L"LoadStoreDescriptor_Wave_16x16_F16_RowMajorToColumnMajor", + SelectedWaveSize)) + return; + + // Source rows of 16 F16 values are 32 bytes packed, padded here to 48. + const cpu_oracle::MatrixBufferLayout LoadLayout = { + MatrixLayout::RowMajor, + /*OffsetBytes=*/DescriptorAlignedOffset, + /*StrideBytes=*/48, + }; + + // The matrix is square, so a column is the same 32 bytes as a row. Storing + // it packed makes the two sides differ in stride as well as layout, so a + // reversed layout cannot land on the same bytes by coincidence. + const cpu_oracle::MatrixBufferLayout StoreLayout = { + MatrixLayout::ColumnMajor, + /*OffsetBytes=*/DescriptorAlignedOffset, + /*StrideBytes=*/32, + }; + + runLoadStoreDescriptor(D3DDevice, DxcSupport, Params, LoadLayout, StoreLayout, + VerboseLogging, SelectedWaveSize); +} + // Half the source matrix lies outside the view the descriptor carries. The // boundary is deliberately placed mid-row rather than on a row boundary, so an // implementation that bounds checks a row at a time cannot pass it. @@ -3100,8 +3148,12 @@ static const char ElementGetOOBShader[] = R"( uint Base = threadID * OOB_RECORD_SIZE; Output.Store(Base + 0, Len); Output.Store(Base + 4, 1); - Output.Store(Base + 8, Just); - Output.Store(Base + 12, Far); + // Widened to float so one record layout serves every element type. Only + // zero and the poison constant are ever compared, and both are exactly + // representable in F16 as well as F32, so the widening cannot mask a + // wrong value. + Output.Store(Base + 8, (float)Just); + Output.Store(Base + 12, (float)Far); } )"; @@ -3141,8 +3193,9 @@ static void runElementGetOOB(ID3D12Device *Device, dxc::SpecificDllLoader &DxcSupport, const MatrixParams &Params, bool Verbose, UINT ForcedWaveSize) { - VERIFY_IS_TRUE(Params.CompType == ComponentType::F32, - "Out-of-bounds Get records assume a 4-byte element"); + VERIFY_IS_TRUE(Params.CompType == ComponentType::F32 || + Params.CompType == ComponentType::F16, + "Out-of-bounds Get records widen the element to float"); const size_t NumElements = Params.totalElements(); const size_t NumThreads = Params.NumThreads; const size_t MatrixSize = Params.totalBytes(); @@ -3345,6 +3398,52 @@ void DxilConf_SM610_LinAlg::ElementSetOOB_Wave_4x8_F32() { SelectedWaveSize); } +// Out-of-bounds element access on F16. Both cases above pin the boundary +// behaviour to F32, which no tier is required to support, so a conforming +// F16-only device would exercise neither. Bounds handling is independent of +// the element type and should not be reachable only through an optional one. +void DxilConf_SM610_LinAlg::ElementGetOOB_Wave_16x16_F16() { + MatrixParams Params = {}; + Params.CompType = ComponentType::F16; + Params.M = 16; + Params.N = 16; + Params.Use = MatrixUse::Accumulator; + Params.Scope = MatrixScope::Wave; + Params.Layout = MatrixLayout::RowMajor; + Params.NumThreads = 128; + Params.Enable16Bit = true; + + UINT SelectedWaveSize = 0; + if (!matrixConstructionApplicable(D3DDevice, Params, {Params.Use}, + L"ElementGetOOB_Wave_16x16_F16", + SelectedWaveSize)) + return; + + runElementGetOOB(D3DDevice, DxcSupport, Params, VerboseLogging, + SelectedWaveSize); +} + +void DxilConf_SM610_LinAlg::ElementSetOOB_Wave_16x16_F16() { + MatrixParams Params = {}; + Params.CompType = ComponentType::F16; + Params.M = 16; + Params.N = 16; + Params.Use = MatrixUse::Accumulator; + Params.Scope = MatrixScope::Wave; + Params.Layout = MatrixLayout::RowMajor; + Params.NumThreads = 128; + Params.Enable16Bit = true; + + UINT SelectedWaveSize = 0; + if (!matrixConstructionApplicable(D3DDevice, Params, {Params.Use}, + L"ElementSetOOB_Wave_16x16_F16", + SelectedWaveSize)) + return; + + runElementSetOOB(D3DDevice, DxcSupport, Params, VerboseLogging, + SelectedWaveSize); +} + static const char CopyConvertShader[] = R"( RWByteAddressBuffer Input : register(u0); RWByteAddressBuffer Output : register(u1); From 28fded88ea9164d99891ebf2999c4fda56b73d65 Mon Sep 17 00:00:00 2001 From: Jack Elliott Date: Wed, 12 Aug 2026 13:24:06 +1200 Subject: [PATCH 2/2] [HLSL] Use a non-square shape for the F16 cross-layout LinAlg test The F16 RowMajor-to-ColumnMajor descriptor case was 16x16. A square matrix cannot detect the defect the cross-layout axis exists to catch: an implementation that swaps the two layouts transposes on load and transposes back on store, and for a square matrix those cancel exactly, byte for byte, whatever strides the two sides use. The case would have passed against precisely the implementation it was added to catch. Using 4x8 restores the detection, and matches the F32 case it mirrors, which is 4x8 for the same reason. Demonstrated by swapping the two layouts in the shader defines while leaving the host oracle unchanged, which models such an implementation. Under that control the square version passes and the 4x8 version fails. Also corrects the rationale comment. It claimed proposal 0029's Minimum Support Set makes F16 the one float type any tier is required to support, but that minimum set governs cooperative vectors rather than wave-scope matrix construction. Neither Fp32 nor Fp16 matrices are required at Tier 1, as the element-access comment in this file already states. The case is still worth having, because a device that supports F16 but not F32 would otherwise get no cross-layout coverage at all, but it is additional capability-gated coverage rather than a guaranteed floor. Also trims the comments this PR adds. Prior review feedback on this suite is that they run long. The block on the cross-layout case was ten lines against a house style of three to five, so it is cut to five, keeping only why the F16 duplicate exists and why the shape must stay non-square. The out-of-bounds block loses a sentence that restated the preceding one. The record-widening comment is rewritten to give the actual reason. The runner reads each lane record with a fixed four-byte memcpy into a float, so a half store would write two bytes and leave the upper two holding the seed sentinel, and even a correct zero would not compare equal. Half to float is lossless, so the widening masks nothing. The record layout comment above it still described the fields as ELEM_TYPE and is corrected to match. Assisted-by: GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 83725f5d-8e98-4c1d-91ee-ad47629e007b --- .../clang/unittests/HLSLExec/LinAlgTests.cpp | 42 +++++++++---------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/tools/clang/unittests/HLSLExec/LinAlgTests.cpp b/tools/clang/unittests/HLSLExec/LinAlgTests.cpp index a7fee2df9c..0a56d01b96 100644 --- a/tools/clang/unittests/HLSLExec/LinAlgTests.cpp +++ b/tools/clang/unittests/HLSLExec/LinAlgTests.cpp @@ -2118,7 +2118,7 @@ class DxilConf_SM610_LinAlg { TEST_METHOD(LoadStoreDescriptor_Wave_16x16_F16); TEST_METHOD(LoadStoreDescriptor_Wave_4x8_F16_RowMajorOffsetPadded); TEST_METHOD(LoadStoreDescriptor_Wave_4x8_F32_RowMajorToColumnMajor); - TEST_METHOD(LoadStoreDescriptor_Wave_16x16_F16_RowMajorToColumnMajor); + TEST_METHOD(LoadStoreDescriptor_Wave_4x8_F16_RowMajorToColumnMajor); TEST_METHOD(LoadDescriptorOOB_Wave_16x16_F16_PartialView); TEST_METHOD(LoadDescriptorOOB_Wave_4x8_F16_OffsetPaddedPartialView); TEST_METHOD(SplatStore_Wave_16x16_F16); @@ -2565,18 +2565,17 @@ void DxilConf_SM610_LinAlg:: VerboseLogging, SelectedWaveSize); } -// The same cross-layout axis on F16. Proposal 0029's Minimum Support Set -// constrains component types only, and F16 is the one float type any tier is -// required to support, so the F32 case above can skip in its entirety on a -// conforming device and take the layout coverage with it. Layout handling has -// nothing to do with the element type, so it should not be reachable only -// through an optional one. +// The same cross-layout axis on F16, because no tier is required to support +// Fp32 matrices and the F32 case above can skip in its entirety. The shape +// must stay non-square: swapping the two layouts transposes on load and back +// on store, and for a square matrix those cancel byte for byte whatever +// strides are used. void DxilConf_SM610_LinAlg:: - LoadStoreDescriptor_Wave_16x16_F16_RowMajorToColumnMajor() { + LoadStoreDescriptor_Wave_4x8_F16_RowMajorToColumnMajor() { MatrixParams Params = {}; Params.CompType = ComponentType::F16; - Params.M = 16; - Params.N = 16; + Params.M = 4; + Params.N = 8; Params.Use = MatrixUse::A; Params.Scope = MatrixScope::Wave; Params.Layout = MatrixLayout::RowMajor; @@ -2586,24 +2585,23 @@ void DxilConf_SM610_LinAlg:: UINT SelectedWaveSize = 0; if (!matrixConstructionApplicable( D3DDevice, Params, {Params.Use}, - L"LoadStoreDescriptor_Wave_16x16_F16_RowMajorToColumnMajor", + L"LoadStoreDescriptor_Wave_4x8_F16_RowMajorToColumnMajor", SelectedWaveSize)) return; - // Source rows of 16 F16 values are 32 bytes packed, padded here to 48. + // Source rows of 8 F16 values are 16 bytes packed, padded here to 48. const cpu_oracle::MatrixBufferLayout LoadLayout = { MatrixLayout::RowMajor, /*OffsetBytes=*/DescriptorAlignedOffset, /*StrideBytes=*/48, }; - // The matrix is square, so a column is the same 32 bytes as a row. Storing - // it packed makes the two sides differ in stride as well as layout, so a - // reversed layout cannot land on the same bytes by coincidence. + // Destination columns of 4 F16 values are 8 bytes, padded here to 16 so the + // column-major side carries a gap of its own rather than sitting packed. const cpu_oracle::MatrixBufferLayout StoreLayout = { MatrixLayout::ColumnMajor, /*OffsetBytes=*/DescriptorAlignedOffset, - /*StrideBytes=*/32, + /*StrideBytes=*/16, }; runLoadStoreDescriptor(D3DDevice, DxcSupport, Params, LoadLayout, StoreLayout, @@ -3099,7 +3097,7 @@ void DxilConf_SM610_LinAlg::ElementSet_Wave_16x16_F16() { // only at the wave total and one that wraps a large index back into range. static constexpr UINT FarOOBOffset = 64; -// Per-lane record: {uint Length, uint Executed, ELEM_TYPE Just, ELEM_TYPE Far}. +// Per-lane record: {uint Length, uint Executed, float Just, float Far}. static constexpr UINT OOBRecordSize = 16; // Seeds every output byte so a lane that never writes cannot be mistaken for a @@ -3148,10 +3146,9 @@ static const char ElementGetOOBShader[] = R"( uint Base = threadID * OOB_RECORD_SIZE; Output.Store(Base + 0, Len); Output.Store(Base + 4, 1); - // Widened to float so one record layout serves every element type. Only - // zero and the poison constant are ever compared, and both are exactly - // representable in F16 as well as F32, so the widening cannot mask a - // wrong value. + // Widened to float so the runner can read a fixed-width record whatever + // the element type: a half store would leave the record's upper two bytes + // holding the sentinel. Half to float is lossless, so no value is masked. Output.Store(Base + 8, (float)Just); Output.Store(Base + 12, (float)Far); } @@ -3400,8 +3397,7 @@ void DxilConf_SM610_LinAlg::ElementSetOOB_Wave_4x8_F32() { // Out-of-bounds element access on F16. Both cases above pin the boundary // behaviour to F32, which no tier is required to support, so a conforming -// F16-only device would exercise neither. Bounds handling is independent of -// the element type and should not be reachable only through an optional one. +// F16-only device would exercise neither. void DxilConf_SM610_LinAlg::ElementGetOOB_Wave_16x16_F16() { MatrixParams Params = {}; Params.CompType = ComponentType::F16;