From 226a341d3c6a1f670597932d9848ddb7c2f5455c Mon Sep 17 00:00:00 2001 From: Jack Elliott Date: Sat, 22 Aug 2026 09:23:37 +1200 Subject: [PATCH 1/2] [HLSL] Add LinAlg vector accumulation contention coverage The vector accumulation tests dispatch a single thread, so they show that an accumulation lands but not that concurrent accumulations all land. An implementation that dropped or duplicated updates under contention would pass every one of them. These two cases dispatch sixty-four threads in each of four groups at one destination. The threads do not all add the same vector. One component varies with the invocation index modulo four, so the expected total depends on which invocations landed and not only on how many did. Had every invocation added the same vector, a dropped update would be cancelled exactly by a duplicated one, and a lowering that assumes the threads hold the same operand and applies one representative vector scaled by the thread count would also match. Varying one component rejects both. The helper already built a NUMTHREADS define through buildCompilerArgs and its shader ignored it, and createComputeOp already accepted a dispatch width. So the change is to use the define the shader was already being given and to pass the width through. Both new parameters default to a single thread in a single group, which is what the three existing callers already did. The varying component is derived from SV_DispatchThreadID, which is zero for those callers, so they are unchanged. The expected values are written out rather than computed so a reviewer can check them against the rule by eye. Invocation t adds 1 + t modulo 4, 2, 3 and 4 to the first four elements. Elements one to three gain 256 * (I + 1), and element zero gains 256 + 384 = 640 because each of the four residues occurs sixty-four times. The F16 case runs from ten to thirteen and reaches 13 + 1024 = 1037; the F32 case runs from twenty to twenty-three and reaches 23 + 1024 = 1047. Every partial sum along the way is a smaller integer, so all of them are exact in both formats and the results can be compared for equality. Because no partial sum rounds, the F32 result cannot depend on the order the hardware applies the additions, which is what OrderInvariant in its name refers to. Both cases carry two guard elements past the accumulated range. Those are pinned by the exact comparison, not by the untouched-byte check, which treats every byte of the destination matrix as its own and so covers the poison below the start offset rather than the guards. Since the expected values are derived for a fixed invocation count, each case asserts that count, and that the count divides evenly by the variation, rather than trusting the constants to stay consistent with the literals. Verified by removing the varying component so every invocation adds the same vector: element zero reaches 266 rather than 650, both new cases fail on that element alone, and the three existing cases are unaffected because their single invocation never varied. Passing at exactly two hundred and fifty-six accumulations also requires both the thread count and the dispatch width to be wired through, since either alone gives a different total. This does not prove the threads physically overlap in time, so an implementation that serialises them passes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 83725f5d-8e98-4c1d-91ee-ad47629e007b Assisted-by: GitHub Copilot --- .../clang/unittests/HLSLExec/LinAlgTests.cpp | 127 +++++++++++++++++- 1 file changed, 122 insertions(+), 5 deletions(-) diff --git a/tools/clang/unittests/HLSLExec/LinAlgTests.cpp b/tools/clang/unittests/HLSLExec/LinAlgTests.cpp index 6a6c3dedc0..2918bc04e2 100644 --- a/tools/clang/unittests/HLSLExec/LinAlgTests.cpp +++ b/tools/clang/unittests/HLSLExec/LinAlgTests.cpp @@ -3194,6 +3194,8 @@ class DxilConf_SM610_LinAlg { TEST_METHOD(VectorAccumulateDescriptor_Thread_F16); TEST_METHOD(VectorAccumulateDescriptor_Thread_F16_Length8_NonZero); TEST_METHOD(VectorAccumulateDescriptor_Thread_F32_Length8_NonZero); + TEST_METHOD(VectorAccumulateDescriptorContention_Thread_F16); + TEST_METHOD(VectorAccumulateDescriptorContention_Thread_F32_OrderInvariant); private: CComPtr D3DDevice; @@ -8103,16 +8105,22 @@ void DxilConf_SM610_LinAlg::Convert() { runConvert(D3DDevice, DxcSupport, VerboseLogging); } +// One vector component varies by invocation modulo this value, so contending +// threads do not all accumulate the same vector. A single-threaded dispatch +// always has invocation zero and so is unaffected. +static constexpr UINT VectorAccumulateVariation = 4; + static const char VectorAccumulateDescriptorShader[] = R"( ByteAddressBuffer Input : register(t0); RWByteAddressBuffer Output : register(u1); - [numthreads(1, 1, 1)] - void main() { + [numthreads(NUMTHREADS, 1, 1)] + void main(uint3 DispatchID : SV_DispatchThreadID) { vector InVec; for (uint I = 0; I < VECTOR_LENGTH; ++I) { InVec[I] = Input.Load(I * ELEM_SIZE); } + InVec[0] += (ELEM_TYPE)(DispatchID.x % THREAD_VARIATION); __builtin_LinAlg_VectorAccumulateToDescriptor( Output, START_OFFSET, 64, InVec); } @@ -8123,7 +8131,8 @@ static void runVectorAccumulateDescriptor( const cpu_oracle::TypedMatrix &Input, const cpu_oracle::TypedMatrix &Initial, const cpu_oracle::TypedMatrix &Expected, UINT StartOffsetBytes, - std::wstring PublicRule, bool Verbose) { + std::wstring PublicRule, bool Verbose, UINT NumThreads = 1, + UINT DispatchX = 1) { VERIFY_ARE_EQUAL(1u, Input.M, "Vector input must have one row"); VERIFY_ARE_EQUAL(Input.compType(), Initial.compType(), "Input and destination component types must match"); @@ -8175,18 +8184,19 @@ static void runVectorAccumulateDescriptor( Params.CompType = Input.compType(); Params.M = Input.M; Params.N = Input.N; - Params.NumThreads = 1; + Params.NumThreads = static_cast(NumThreads); Params.Enable16Bit = Input.compType() == ComponentType::F16; std::stringstream ExtraDefs; ExtraDefs << "-DVECTOR_LENGTH=" << Input.totalElements(); ExtraDefs << " -DSTART_OFFSET=" << StartOffsetBytes; + ExtraDefs << " -DTHREAD_VARIATION=" << VectorAccumulateVariation; const std::string Args = buildCompilerArgs(Params, ExtraDefs.str().c_str()); compileShader(DxcSupport, VectorAccumulateDescriptorShader, "cs_6_10", Args, Verbose); auto Op = createComputeOp(VectorAccumulateDescriptorShader, "cs_6_10", - "SRV(t0), UAV(u1)", Args.c_str()); + "SRV(t0), UAV(u1)", Args.c_str(), DispatchX); addSRVBuffer(Op.get(), "Input", InputBytes.size(), "byname"); addUAVBuffer(Op.get(), "Output", InitialBytes.size(), true, "byname"); addRootView(Op.get(), 0, "Input"); @@ -8331,6 +8341,113 @@ void DxilConf_SM610_LinAlg:: VerboseLogging); } +// The single-threaded cases above show that an accumulation lands, not that +// concurrent accumulations all land. These dispatch many threads across many +// groups at one destination. One vector component varies by invocation so the +// threads do not all add the same vector, which means a dropped update cannot +// be cancelled by a duplicated one, and a lowering that applies one +// representative vector scaled by the thread count does not match either. +static constexpr UINT VectorContentionThreads = 64; +static constexpr UINT VectorContentionGroups = 4; +static constexpr UINT VectorContentionInvocations = 256; + +void DxilConf_SM610_LinAlg::VectorAccumulateDescriptorContention_Thread_F16() { + if (!accumulateStoreApplicable( + D3DDevice, ComponentType::F16, + linalg_test::AtomicDestination::RWByteAddressBuffer, + L"VectorAccumulateDescriptorContention_Thread_F16")) + return; + + VERIFY_ARE_EQUAL(VectorContentionInvocations, + VectorContentionThreads * VectorContentionGroups, + "The expected values below are derived for this many " + "invocations and must be recomputed if it changes"); + VERIFY_ARE_EQUAL(0u, VectorContentionInvocations % VectorAccumulateVariation, + "Element zero's expected value assumes the invocation " + "residues are evenly distributed"); + + // Invocation t adds {1 + t % 4, 2, 3, 4}. Elements 1 to 3 accumulate + // 256 * (I + 1); element 0 accumulates 256 + 384 = 640 because each of the + // four residues occurs 64 times. The largest result is 13 + 1024 = 1037 and + // every partial sum is a smaller integer, so all are exact in F16 and the + // comparison can be for equality. The last two elements are guards the + // accumulation must not reach. + const auto Half = [](float Value) { return HLSLHalf_t(Value); }; + const std::optional Input = + cpu_oracle::makeTypedMatrix( + 1, 4, {Half(1), Half(2), Half(3), Half(4)}); + const std::optional Initial = + cpu_oracle::makeTypedMatrix( + 1, 6, + {Half(10), Half(11), Half(12), Half(13), Half(777), Half(-777)}); + const std::optional Expected = + cpu_oracle::makeTypedMatrix( + 1, 6, + {Half(650), Half(523), Half(780), Half(1037), Half(777), Half(-777)}); + VERIFY_IS_TRUE(Input.has_value()); + VERIFY_IS_TRUE(Initial.has_value()); + VERIFY_IS_TRUE(Expected.has_value()); + if (!Input) + return; + if (!Initial) + return; + if (!Expected) + return; + + runVectorAccumulateDescriptor( + D3DDevice, DxcSupport, *Input, *Initial, *Expected, + /*StartOffsetBytes=*/0, + L"Exact F16 vector descriptor accumulation under contention from many " + L"threads across many groups", + VerboseLogging, VectorContentionThreads, VectorContentionGroups); +} + +void DxilConf_SM610_LinAlg:: + VectorAccumulateDescriptorContention_Thread_F32_OrderInvariant() { + if (!accumulateStoreApplicable( + D3DDevice, ComponentType::F32, + linalg_test::AtomicDestination::RWByteAddressBuffer, + L"VectorAccumulateDescriptorContention_Thread_F32_OrderInvariant")) + return; + + VERIFY_ARE_EQUAL(VectorContentionInvocations, + VectorContentionThreads * VectorContentionGroups, + "The expected values below are derived for this many " + "invocations and must be recomputed if it changes"); + VERIFY_ARE_EQUAL(0u, VectorContentionInvocations % VectorAccumulateVariation, + "Element zero's expected value assumes the invocation " + "residues are evenly distributed"); + + // The same distribution as above, reaching 23 + 1024 = 1047. Every partial + // sum is an integer well inside the range F32 represents exactly, so no + // ordering of the atomic additions can round differently and the result + // cannot depend on the order the hardware happens to apply them. + const std::optional Input = + cpu_oracle::makeTypedMatrix(1, 4, {1, 2, 3, 4}); + const std::optional Initial = + cpu_oracle::makeTypedMatrix(1, 6, + {20, 21, 22, 23, 123456, -654321}); + const std::optional Expected = + cpu_oracle::makeTypedMatrix( + 1, 6, {660, 533, 790, 1047, 123456, -654321}); + VERIFY_IS_TRUE(Input.has_value()); + VERIFY_IS_TRUE(Initial.has_value()); + VERIFY_IS_TRUE(Expected.has_value()); + if (!Input) + return; + if (!Initial) + return; + if (!Expected) + return; + + runVectorAccumulateDescriptor( + D3DDevice, DxcSupport, *Input, *Initial, *Expected, + /*StartOffsetBytes=*/64, + L"Order-independent F32 vector descriptor accumulation under contention " + L"from many threads across many groups", + VerboseLogging, VectorContentionThreads, VectorContentionGroups); +} + void DxilConf_SM610_LinAlg::MatVecMul_Thread_4x8_F16_NonUniform() { const matvec_interpretation::CaseData Case = matvec_interpretation::makeNonUniformF16Case(MatrixLayout::RowMajor); From f6d91c3c6511de345bf31934734632d9f2a58dfa Mon Sep 17 00:00:00 2001 From: Jack Elliott Date: Tue, 25 Aug 2026 13:35:14 +1200 Subject: [PATCH 2/2] [HLSL] Make contention contributions unique per invocation The contention shader varied only element zero, as DispatchThreadID.x modulo four, so the 256 invocations produced just four distinct contributed vectors with 64 copies each. Dropping one invocation's update and applying another's twice therefore left the accumulated sum bit-identical whenever the two shared a residue, and an exhaustive search over ordered pairs finds 16128 such compensating pairs. The comment above the tests claimed the opposite. Each vector component now carries one digit of the invocation index in base THREAD_VARIATION. Four components at base four gives exactly the 256 invocations dispatched, so the contribution is a bijection of the invocation index and the same search finds no compensating pairs. Every digit value occurs 64 times in each position, so element I accumulates 256 * (I + 1) + 384. The largest result is 1421, which keeps every partial sum exactly representable in F16 and so preserves the order invariance the F32 case asserts. The single-threaded callers are unaffected because invocation zero contributes every digit as zero. A discriminating negative control forced one defect against both encodings: dropping invocation zero and duplicating invocation four fails under the base digit encoding, reporting 908 against an expected 907 at component one, and passes unnoticed under the previous encoding. 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 | 69 +++++++++---------- 1 file changed, 33 insertions(+), 36 deletions(-) diff --git a/tools/clang/unittests/HLSLExec/LinAlgTests.cpp b/tools/clang/unittests/HLSLExec/LinAlgTests.cpp index 2918bc04e2..920b6d413f 100644 --- a/tools/clang/unittests/HLSLExec/LinAlgTests.cpp +++ b/tools/clang/unittests/HLSLExec/LinAlgTests.cpp @@ -8105,9 +8105,9 @@ void DxilConf_SM610_LinAlg::Convert() { runConvert(D3DDevice, DxcSupport, VerboseLogging); } -// One vector component varies by invocation modulo this value, so contending -// threads do not all accumulate the same vector. A single-threaded dispatch -// always has invocation zero and so is unaffected. +// The base each vector component carries one digit of the invocation index in, +// so contending threads all accumulate distinct vectors. A single-threaded +// dispatch always has invocation zero and so is unaffected. static constexpr UINT VectorAccumulateVariation = 4; static const char VectorAccumulateDescriptorShader[] = R"( @@ -8117,10 +8117,12 @@ static const char VectorAccumulateDescriptorShader[] = R"( [numthreads(NUMTHREADS, 1, 1)] void main(uint3 DispatchID : SV_DispatchThreadID) { vector InVec; + uint Residual = DispatchID.x; for (uint I = 0; I < VECTOR_LENGTH; ++I) { - InVec[I] = Input.Load(I * ELEM_SIZE); + InVec[I] = Input.Load(I * ELEM_SIZE) + + (ELEM_TYPE)(Residual % THREAD_VARIATION); + Residual /= THREAD_VARIATION; } - InVec[0] += (ELEM_TYPE)(DispatchID.x % THREAD_VARIATION); __builtin_LinAlg_VectorAccumulateToDescriptor( Output, START_OFFSET, 64, InVec); } @@ -8343,14 +8345,24 @@ void DxilConf_SM610_LinAlg:: // The single-threaded cases above show that an accumulation lands, not that // concurrent accumulations all land. These dispatch many threads across many -// groups at one destination. One vector component varies by invocation so the -// threads do not all add the same vector, which means a dropped update cannot -// be cancelled by a duplicated one, and a lowering that applies one -// representative vector scaled by the thread count does not match either. +// groups at one destination. Each component carries one base-four digit of the +// invocation index, so no two invocations contribute the same vector and a +// dropped update cannot be cancelled by a duplicated one. static constexpr UINT VectorContentionThreads = 64; static constexpr UINT VectorContentionGroups = 4; static constexpr UINT VectorContentionInvocations = 256; +static_assert( + VectorContentionInvocations == + VectorContentionThreads * VectorContentionGroups, + "The expected values below are derived for this many invocations"); +static_assert(VectorContentionInvocations == + VectorAccumulateVariation * VectorAccumulateVariation * + VectorAccumulateVariation * VectorAccumulateVariation, + "Each of the four vector components carries one base-four digit " + "of the invocation index, so the expected sums hold only when " + "every digit combination occurs exactly once"); + void DxilConf_SM610_LinAlg::VectorAccumulateDescriptorContention_Thread_F16() { if (!accumulateStoreApplicable( D3DDevice, ComponentType::F16, @@ -8358,20 +8370,12 @@ void DxilConf_SM610_LinAlg::VectorAccumulateDescriptorContention_Thread_F16() { L"VectorAccumulateDescriptorContention_Thread_F16")) return; - VERIFY_ARE_EQUAL(VectorContentionInvocations, - VectorContentionThreads * VectorContentionGroups, - "The expected values below are derived for this many " - "invocations and must be recomputed if it changes"); - VERIFY_ARE_EQUAL(0u, VectorContentionInvocations % VectorAccumulateVariation, - "Element zero's expected value assumes the invocation " - "residues are evenly distributed"); - - // Invocation t adds {1 + t % 4, 2, 3, 4}. Elements 1 to 3 accumulate - // 256 * (I + 1); element 0 accumulates 256 + 384 = 640 because each of the - // four residues occurs 64 times. The largest result is 13 + 1024 = 1037 and - // every partial sum is a smaller integer, so all are exact in F16 and the - // comparison can be for equality. The last two elements are guards the - // accumulation must not reach. + // Invocation t adds digit I of t in base four to element I, on top of the + // base vector {1, 2, 3, 4}. Each digit value occurs 64 times in each + // position, so element I accumulates 256 * (I + 1) + 384. The largest result + // is 13 + 1408 = 1421 and every partial sum is a smaller integer, so all are + // exact in F16 and the comparison can be for equality. The last two elements + // are guards the accumulation must not reach. const auto Half = [](float Value) { return HLSLHalf_t(Value); }; const std::optional Input = cpu_oracle::makeTypedMatrix( @@ -8381,9 +8385,10 @@ void DxilConf_SM610_LinAlg::VectorAccumulateDescriptorContention_Thread_F16() { 1, 6, {Half(10), Half(11), Half(12), Half(13), Half(777), Half(-777)}); const std::optional Expected = - cpu_oracle::makeTypedMatrix( - 1, 6, - {Half(650), Half(523), Half(780), Half(1037), Half(777), Half(-777)}); + cpu_oracle::makeTypedMatrix(1, 6, + {Half(650), Half(907), Half(1164), + Half(1421), Half(777), + Half(-777)}); VERIFY_IS_TRUE(Input.has_value()); VERIFY_IS_TRUE(Initial.has_value()); VERIFY_IS_TRUE(Expected.has_value()); @@ -8410,15 +8415,7 @@ void DxilConf_SM610_LinAlg:: L"VectorAccumulateDescriptorContention_Thread_F32_OrderInvariant")) return; - VERIFY_ARE_EQUAL(VectorContentionInvocations, - VectorContentionThreads * VectorContentionGroups, - "The expected values below are derived for this many " - "invocations and must be recomputed if it changes"); - VERIFY_ARE_EQUAL(0u, VectorContentionInvocations % VectorAccumulateVariation, - "Element zero's expected value assumes the invocation " - "residues are evenly distributed"); - - // The same distribution as above, reaching 23 + 1024 = 1047. Every partial + // The same distribution as above, reaching 23 + 1408 = 1431. Every partial // sum is an integer well inside the range F32 represents exactly, so no // ordering of the atomic additions can round differently and the result // cannot depend on the order the hardware happens to apply them. @@ -8429,7 +8426,7 @@ void DxilConf_SM610_LinAlg:: {20, 21, 22, 23, 123456, -654321}); const std::optional Expected = cpu_oracle::makeTypedMatrix( - 1, 6, {660, 533, 790, 1047, 123456, -654321}); + 1, 6, {660, 917, 1174, 1431, 123456, -654321}); VERIFY_IS_TRUE(Input.has_value()); VERIFY_IS_TRUE(Initial.has_value()); VERIFY_IS_TRUE(Expected.has_value());