From 9dcfd55adaa5d6266ab55e0f17551636aa3d006c Mon Sep 17 00:00:00 2001 From: Jack Elliott Date: Fri, 4 Sep 2026 09:36:04 +1200 Subject: [PATCH 1/2] [HLSL] Add LinAlg descriptor bounds coverage for matrix accumulation Proposal 0035 permits an implementation to resolve an out-of-bounds descriptor access either by dropping the whole operation or by dropping only the elements the view does not admit whole, and it states that bounds checking is not required for root descriptors. The existing accumulate coverage binds through addRootView, so it cannot reach either rule; only the store and load paths had bounded-view cases. These two cases bind MatrixAccumulateToDescriptor through a bounded descriptor table and accept exactly the two permitted outcomes, mirroring the shapes already used for MatrixStoreToDescriptor: a packed 16x16 F16 matrix behind a 260 byte view, which cuts two elements into row 8, and an offset 4x8 F16 matrix with a padded 32 byte stride behind a 172 byte view, which cuts within row 1 rather than on the padding. Both boundaries fall mid-row so a row-granular bounds check cannot pass, and both are carried over from the store cases so the two opcodes are compared on identical geometry. Accumulation needs a destination seed that a store cannot reproduce, otherwise accumulating onto the seed and storing the addend give the same buffer and the case cannot tell MatrixAccumulateToDescriptor from MatrixStoreToDescriptor. The addend and the seed are therefore distinct arithmetic sequences, starting at 1 and 1000, and the expected sums are generated from their own closed form with a step of two rather than by replaying the addition the GPU performs. Every value stays below 2048, so all of them, and all of the seeds, are exact in F16 and the comparison can be for equality. makeSequentialMatrix gains an optional step so the expected image comes from the same range-guarded helper as the other two rather than from a hand-rolled loop; it defaults to one, so existing callers are unaffected. accumulateBufferBoundedByView is the accumulate-side counterpart to storeBufferBoundedByView: an accumulation the bounds check rejects never touches memory, so the elements the view does not admit whole keep the value the destination was seeded with rather than the poison the store version restores. Both non-vacuity guards are carried across from the store runner. They are what stop a view that admits everything, or nothing, from making the case pass without discriminating: the two candidates must differ from each other and from the unbounded image. The source is deliberately viewed in full so an observed result cannot be attributed to a bounds check on the load instead of the accumulation. Validated on WARP against a matched runtime. The full LinAlg class goes from 87 tests, 74 passed, 8 failed, 5 skipped to 89 tests, 76 passed, 8 failed, 5 skipped, and a per-test comparison shows the only two differences are the cases added here; no existing test changes bucket. The eight failures are pre-existing and unrelated, covering group-shared memory skew and the three known WARP Convert defects. 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 | 268 +++++++++++++++++- 1 file changed, 262 insertions(+), 6 deletions(-) diff --git a/tools/clang/unittests/HLSLExec/LinAlgTests.cpp b/tools/clang/unittests/HLSLExec/LinAlgTests.cpp index 56dbd9b12c..91ee7b711f 100644 --- a/tools/clang/unittests/HLSLExec/LinAlgTests.cpp +++ b/tools/clang/unittests/HLSLExec/LinAlgTests.cpp @@ -809,7 +809,7 @@ static std::optional makeTypedMatrix(MatrixDim M, MatrixDim N, static std::optional makeSequentialMatrix(ComponentType CompType, MatrixDim M, MatrixDim N, - uint32_t StartingValue = 1) { + uint32_t StartingValue = 1, uint32_t Step = 1) { size_t NumElements; if (M == 0 || N == 0 || !checkedMultiply(static_cast(M), static_cast(N), @@ -819,8 +819,15 @@ makeSequentialMatrix(ComponentType CompType, MatrixDim M, MatrixDim N, return std::nullopt; } + if (Step == 0) { + hlsl_test::LogErrorFmt(L"Sequential matrix step must be non-zero"); + return std::nullopt; + } + + size_t SpanSize; size_t LastValueSize; - if (!checkedAdd(static_cast(StartingValue), NumElements - 1, + if (!checkedMultiply(static_cast(Step), NumElements - 1, SpanSize) || + !checkedAdd(static_cast(StartingValue), SpanSize, LastValueSize)) { hlsl_test::LogErrorFmt(L"Sequential matrix value calculation overflowed"); return std::nullopt; @@ -838,7 +845,8 @@ makeSequentialMatrix(ComponentType CompType, MatrixDim M, MatrixDim N, Values.reserve(NumElements); for (size_t I = 0; I < NumElements; ++I) Values.emplace_back(static_cast( - static_cast(StartingValue) + static_cast(I))); + static_cast(StartingValue) + + static_cast(Step) * static_cast(I))); return makeTypedMatrix(M, N, std::move(Values)); } case ComponentType::F32: { @@ -852,7 +860,8 @@ makeSequentialMatrix(ComponentType CompType, MatrixDim M, MatrixDim N, Values.reserve(NumElements); for (size_t I = 0; I < NumElements; ++I) Values.push_back(static_cast(static_cast(StartingValue) + - static_cast(I))); + static_cast(Step) * + static_cast(I))); return makeTypedMatrix(M, N, std::move(Values)); } case ComponentType::I32: { @@ -866,7 +875,8 @@ makeSequentialMatrix(ComponentType CompType, MatrixDim M, MatrixDim N, Values.reserve(NumElements); for (size_t I = 0; I < NumElements; ++I) Values.push_back(static_cast( - static_cast(StartingValue) + static_cast(I))); + static_cast(StartingValue) + + static_cast(Step) * static_cast(I))); return makeTypedMatrix(M, N, std::move(Values)); } case ComponentType::U32: { @@ -879,7 +889,8 @@ makeSequentialMatrix(ComponentType CompType, MatrixDim M, MatrixDim N, Values.reserve(NumElements); for (size_t I = 0; I < NumElements; ++I) Values.push_back(static_cast( - static_cast(StartingValue) + static_cast(I))); + static_cast(StartingValue) + + static_cast(Step) * static_cast(I))); return makeTypedMatrix(M, N, std::move(Values)); } default: @@ -1398,6 +1409,54 @@ storeBufferBoundedByView(const TypedMatrix &Source, return Buffer; } +// Accumulate-side counterpart to storeBufferBoundedByView. Elements the view +// does not admit whole keep their seeded value. +static std::optional> accumulateBufferBoundedByView( + const TypedMatrix &Initial, const TypedMatrix &Accumulated, + const MatrixBufferLayout &Layout, size_t ViewBytes) { + if (!isMatrixValid(Initial) || !isMatrixValid(Accumulated)) { + hlsl_test::LogErrorFmt(L"Cannot bound an invalid typed matrix to a view"); + return std::nullopt; + } + if (Initial.compType() != Accumulated.compType() || + Initial.M != Accumulated.M || Initial.N != Accumulated.N) { + hlsl_test::LogErrorFmt( + L"Initial and accumulated matrices must share component type and " + L"shape"); + return std::nullopt; + } + + std::optional BufferSize = getMatrixBufferSize(Initial, Layout); + if (!BufferSize) + return std::nullopt; + + std::vector Buffer(*BufferSize); + std::vector InitialImage(*BufferSize); + fillPoison(Buffer.data(), Buffer.size()); + fillPoison(InitialImage.data(), InitialImage.size()); + if (!writeMatrixBuffer(Accumulated, Layout, Buffer) || + !writeMatrixBuffer(Initial, Layout, InitialImage)) + return std::nullopt; + + const size_t ElementBytes = elementSize(Initial.compType()); + for (MatrixDim Row = 0; Row < Initial.M; ++Row) { + for (MatrixDim Column = 0; Column < Initial.N; ++Column) { + std::optional ByteOffset = getElementByteOffset( + Initial.compType(), Initial.M, Initial.N, Row, Column, Layout); + if (!ByteOffset) + return std::nullopt; + size_t ElementEnd; + if (!checkedAdd(*ByteOffset, ElementBytes, ElementEnd)) + return std::nullopt; + if (ElementEnd <= ViewBytes) + continue; + for (size_t I = *ByteOffset; I < ElementEnd; ++I) + Buffer[I] = InitialImage[I]; + } + } + return Buffer; +} + // Byte-level counterpart to verifyMatrixBuffer. The permitted store outcomes // differ in which bytes they leave alone rather than in the values they // produce, and the poison pattern does not always decode to a comparable @@ -3546,6 +3605,8 @@ class DxilConf_SM610_LinAlg { TEST_METHOD(LoadDescriptorOOB_Wave_4x8_F16_OffsetPaddedPartialView); TEST_METHOD(StoreDescriptorOOB_Wave_16x16_F16_PartialView); TEST_METHOD(StoreDescriptorOOB_Wave_4x8_F16_OffsetPaddedPartialView); + TEST_METHOD(AccumulateDescriptorOOB_Wave_16x16_F16_PartialView); + TEST_METHOD(AccumulateDescriptorOOB_Wave_4x8_F16_OffsetPaddedPartialView); TEST_METHOD(SplatStore_Wave_16x16_F16); TEST_METHOD(AccumulateDescriptor_Wave_16x16_F16); TEST_METHOD(AccumulateDescriptorContention_Wave_4x8_I32); @@ -4009,6 +4070,135 @@ static void runStoreDescriptorOutOfBounds( Verbose)); } +static const char AccumulateDescriptorOOBShader[] = R"( + RWByteAddressBuffer Input : register(u0); + RWByteAddressBuffer Output : register(u1); + + #ifdef FORCED_WAVE_SIZE + [WaveSize(FORCED_WAVE_SIZE)] + #else + [WaveSize(4, 128)] + #endif + [numthreads(NUMTHREADS, 1, 1)] + void main() { + if (GetGroupWaveIndex() != 0) + return; + + __builtin_LinAlgMatrix + [[__LinAlgMatrix_Attributes(COMP_TYPE, M_DIM, N_DIM, USE, SCOPE)]] + Mat; + __builtin_LinAlg_MatrixLoadFromDescriptor( + Mat, Input, LOAD_OFFSET, LOAD_STRIDE, LOAD_LAYOUT, DECLARED_ALIGN); + __builtin_LinAlg_MatrixAccumulateToDescriptor( + Mat, Output, STORE_OFFSET, STORE_STRIDE, STORE_LAYOUT, DECLARED_ALIGN); + } +)"; + +static constexpr uint32_t AccumulateOOBAddendBase = 1; +static constexpr uint32_t AccumulateOOBInitialBase = 1000; + +static void runAccumulateDescriptorOutOfBounds( + ID3D12Device *Device, dxc::SpecificDllLoader &DxcSupport, + const MatrixParams &Params, const cpu_oracle::MatrixBufferLayout &Layout, + size_t OutputViewBytes, bool Verbose, UINT ForcedWaveSize = 0) { + std::optional Addend = + cpu_oracle::makeSequentialMatrix(Params.CompType, Params.M, Params.N, + AccumulateOOBAddendBase); + std::optional Initial = + cpu_oracle::makeSequentialMatrix(Params.CompType, Params.M, Params.N, + AccumulateOOBInitialBase); + std::optional Accumulated = + cpu_oracle::makeSequentialMatrix( + Params.CompType, Params.M, Params.N, + AccumulateOOBInitialBase + AccumulateOOBAddendBase, /*Step=*/2); + VERIFY_IS_TRUE(Addend.has_value() && Initial.has_value() && + Accumulated.has_value(), + "Unable to construct typed AccumulateDescriptorOOB matrices"); + + std::optional BufferSize = + cpu_oracle::getMatrixBufferSize(*Initial, Layout); + VERIFY_IS_TRUE(BufferSize.has_value(), + "Unable to size the AccumulateDescriptorOOB buffers"); + VERIFY_IS_TRUE(OutputViewBytes < *BufferSize, + "The destination view must be shorter than its buffer"); + + std::optional> PerElement = + cpu_oracle::accumulateBufferBoundedByView(*Initial, *Accumulated, Layout, + OutputViewBytes); + std::optional> WholeOperation = + cpu_oracle::accumulateBufferBoundedByView(*Initial, *Accumulated, Layout, + 0); + std::optional> Unbounded = + cpu_oracle::accumulateBufferBoundedByView(*Initial, *Accumulated, Layout, + *BufferSize); + VERIFY_IS_TRUE(PerElement.has_value() && WholeOperation.has_value() && + Unbounded.has_value(), + "Unable to derive the AccumulateDescriptorOOB candidates"); + + VERIFY_IS_TRUE(*PerElement != *WholeOperation, + "The destination view must admit at least one whole element"); + VERIFY_IS_TRUE(*PerElement != *Unbounded, + "The destination view must exclude at least one element"); + + std::stringstream ExtraDefs; + ExtraDefs << " -DLOAD_OFFSET=" << Layout.OffsetBytes; + ExtraDefs << " -DLOAD_STRIDE=" << Layout.StrideBytes; + ExtraDefs << " -DLOAD_LAYOUT=" << static_cast(Layout.Layout); + ExtraDefs << " -DSTORE_OFFSET=" << Layout.OffsetBytes; + ExtraDefs << " -DSTORE_STRIDE=" << Layout.StrideBytes; + ExtraDefs << " -DSTORE_LAYOUT=" << static_cast(Layout.Layout); + ExtraDefs << " -DDECLARED_ALIGN=" << DescriptorDeclaredAlignment; + + if (ForcedWaveSize != 0) + ExtraDefs << " -DFORCED_WAVE_SIZE=" << ForcedWaveSize; + + std::string Args = buildCompilerArgs(Params, ExtraDefs.str().c_str()); + + compileShader(DxcSupport, AccumulateDescriptorOOBShader, "cs_6_10", Args, + Verbose); + + // The source is viewed in full so only the destination bound is exercised. + const cpu_oracle::TypedMatrix AddendMatrix = *Addend; + const cpu_oracle::TypedMatrix InitialMatrix = *Initial; + auto Op = createComputeOp(AccumulateDescriptorOOBShader, "cs_6_10", + "DescriptorTable(UAV(u0), UAV(u1))", Args.c_str()); + addUAVBuffer(Op.get(), "Input", *BufferSize, false, "byname"); + addUAVBuffer(Op.get(), "Output", *BufferSize, true, "byname"); + addHeapRawUAV(Op.get(), "ResHeap", "Input", *BufferSize); + addHeapRawUAV(Op.get(), "ResHeap", "Output", OutputViewBytes); + addRootTable(Op.get(), 0, "ResHeap"); + + auto Result = runShaderOp( + Device, DxcSupport, std::move(Op), + [AddendMatrix, InitialMatrix, + Layout](LPCSTR Name, std::vector &Data, st::ShaderOp *) { + cpu_oracle::fillPoison(Data.data(), Data.size()); + const cpu_oracle::TypedMatrix *Source = nullptr; + if (_stricmp(Name, "Input") == 0) + Source = &AddendMatrix; + else if (_stricmp(Name, "Output") == 0) + Source = &InitialMatrix; + if (!Source) + return; + VERIFY_IS_TRUE(cpu_oracle::writeMatrixBuffer(*Source, Layout, Data), + "Unable to encode AccumulateDescriptorOOB buffer"); + }, + [Layout](ID3D12GraphicsCommandList *, st::ShaderOpTest *Test) { + verifyDescriptorBaseAlignment(Test, "Input", Layout.OffsetBytes); + verifyDescriptorBaseAlignment(Test, "Output", Layout.OffsetBytes); + }); + + MappedData OutData; + Result->Test->GetReadBackData("Output", &OutData); + + VERIFY_IS_TRUE(cpu_oracle::verifyStoreBuffer( + OutData.data(), OutData.size(), {*PerElement, *WholeOperation}, + L"HLSL proposal 0035 bounds checking on MatrixAccumulateToDescriptor: " + L"either the whole accumulation or only the out-of-view element " + L"accumulations become a no-op", + Verbose)); +} + // No offset and a tightly packed stride: a matrix occupying the whole buffer. static cpu_oracle::MatrixBufferLayout packedLayout(const MatrixParams &Params) { return cpu_oracle::MatrixBufferLayout{ @@ -4294,6 +4484,72 @@ void DxilConf_SM610_LinAlg:: SelectedWaveSize); } +void DxilConf_SM610_LinAlg:: + AccumulateDescriptorOOB_Wave_16x16_F16_PartialView() { + 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"AccumulateDescriptorOOB_Wave_16x16_F16_" + L"PartialView", + SelectedWaveSize)) + return; + if (!accumulateStoreApplicable( + D3DDevice, Params.CompType, + linalg_test::AtomicDestination::RWByteAddressBuffer, + L"AccumulateDescriptorOOB_Wave_16x16_F16_PartialView")) + return; + + // 260 bytes: rows 0 to 7 whole plus two elements of row 8. + runAccumulateDescriptorOutOfBounds( + D3DDevice, DxcSupport, Params, packedLayout(Params), + /*OutputViewBytes=*/260, VerboseLogging, SelectedWaveSize); +} + +void DxilConf_SM610_LinAlg:: + AccumulateDescriptorOOB_Wave_4x8_F16_OffsetPaddedPartialView() { + MatrixParams Params = {}; + Params.CompType = ComponentType::F16; + Params.M = 4; + Params.N = 8; + 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"AccumulateDescriptorOOB_Wave_4x8_F16_" + L"OffsetPaddedPartialView", + SelectedWaveSize)) + return; + if (!accumulateStoreApplicable( + D3DDevice, Params.CompType, + linalg_test::AtomicDestination::RWByteAddressBuffer, + L"AccumulateDescriptorOOB_Wave_4x8_F16_OffsetPaddedPartialView")) + return; + + const cpu_oracle::MatrixBufferLayout Layout = { + MatrixLayout::RowMajor, + /*OffsetBytes=*/DescriptorAlignedOffset, + /*StrideBytes=*/32, + }; + + // 172 bytes: row 0 whole plus columns 0 to 5 of row 1. + runAccumulateDescriptorOutOfBounds(D3DDevice, DxcSupport, Params, Layout, + /*OutputViewBytes=*/172, VerboseLogging, + SelectedWaveSize); +} + static const char SplatStoreShader[] = R"( RWByteAddressBuffer Output : register(u0); From c05e0fe97f8f00d80d5033872fd643169e1cf7a5 Mon Sep 17 00:00:00 2001 From: Jack Elliott Date: Fri, 4 Sep 2026 12:50:47 +1200 Subject: [PATCH 2/2] Change Op to use unique_ptr for compute operation Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- tools/clang/unittests/HLSLExec/LinAlgTests.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/clang/unittests/HLSLExec/LinAlgTests.cpp b/tools/clang/unittests/HLSLExec/LinAlgTests.cpp index 91ee7b711f..958ad84d2a 100644 --- a/tools/clang/unittests/HLSLExec/LinAlgTests.cpp +++ b/tools/clang/unittests/HLSLExec/LinAlgTests.cpp @@ -4160,8 +4160,9 @@ static void runAccumulateDescriptorOutOfBounds( // The source is viewed in full so only the destination bound is exercised. const cpu_oracle::TypedMatrix AddendMatrix = *Addend; const cpu_oracle::TypedMatrix InitialMatrix = *Initial; - auto Op = createComputeOp(AccumulateDescriptorOOBShader, "cs_6_10", - "DescriptorTable(UAV(u0), UAV(u1))", Args.c_str()); + std::unique_ptr Op = + createComputeOp(AccumulateDescriptorOOBShader, "cs_6_10", + "DescriptorTable(UAV(u0), UAV(u1))", Args.c_str()); addUAVBuffer(Op.get(), "Input", *BufferSize, false, "byname"); addUAVBuffer(Op.get(), "Output", *BufferSize, true, "byname"); addHeapRawUAV(Op.get(), "ResHeap", "Input", *BufferSize);