ComputeProgram: Remove data field to fix race.
data is the 0th argument to the coroutine.
There's no good reason why this was stored as a field when all other arguments were fetched in ComputeProgram::emit().
Having this as a field caused a data race in the rr::Variable materializing, as the destructor of this field was called outside the reactor mutex.
Test: dEQP-VK.synchronization.internally_synchronized_objects.pipeline_cache_compute
Bug: b/133127573
Change-Id: I44545b71714fdebd8f1150bf7d6325f7d4d4d3eb
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/32569
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
Tested-by: Ben Clayton <bclayton@google.com>
Reviewed-by: Nicolas Capens <nicolascapens@google.com>
diff --git a/src/Pipeline/ComputeProgram.cpp b/src/Pipeline/ComputeProgram.cpp
index 253c479..9ca2d9c 100644
--- a/src/Pipeline/ComputeProgram.cpp
+++ b/src/Pipeline/ComputeProgram.cpp
@@ -28,8 +28,7 @@
namespace sw
{
ComputeProgram::ComputeProgram(SpirvShader const *shader, vk::PipelineLayout const *pipelineLayout, const vk::DescriptorSet::Bindings &descriptorSets)
- : data(Arg<0>()),
- shader(shader),
+ : shader(shader),
pipelineLayout(pipelineLayout),
descriptorSets(descriptorSets)
{
@@ -47,7 +46,7 @@
shader->emitEpilog(&routine);
}
- void ComputeProgram::setWorkgroupBuiltins(SpirvRoutine* routine, Int workgroupID[3])
+ void ComputeProgram::setWorkgroupBuiltins(Pointer<Byte> data, SpirvRoutine* routine, Int workgroupID[3])
{
setInputBuiltin(routine, spv::BuiltInNumWorkgroups, [&](const SpirvShader::BuiltinMapping& builtin, Array<SIMD::Float>& value)
{
@@ -106,7 +105,7 @@
});
}
- void ComputeProgram::setSubgroupBuiltins(SpirvRoutine* routine, Int workgroupID[3], SIMD::Int localInvocationIndex, Int subgroupIndex)
+ void ComputeProgram::setSubgroupBuiltins(Pointer<Byte> data, SpirvRoutine* routine, Int workgroupID[3], SIMD::Int localInvocationIndex, Int subgroupIndex)
{
Int4 numWorkgroups = *Pointer<Int4>(data + OFFSET(Data, numWorkgroups));
Int4 workgroupSize = *Pointer<Int4>(data + OFFSET(Data, workgroupSize));
@@ -163,6 +162,7 @@
void ComputeProgram::emit(SpirvRoutine* routine)
{
+ Pointer<Byte> data = Arg<0>();
Int workgroupX = Arg<1>();
Int workgroupY = Arg<2>();
Int workgroupZ = Arg<3>();
@@ -179,7 +179,7 @@
Int invocationsPerWorkgroup = *Pointer<Int>(data + OFFSET(Data, invocationsPerWorkgroup));
Int workgroupID[3] = {workgroupX, workgroupY, workgroupZ};
- setWorkgroupBuiltins(routine, workgroupID);
+ setWorkgroupBuiltins(data, routine, workgroupID);
For(Int i = 0, i < subgroupCount, i++)
{
@@ -191,7 +191,7 @@
// Disable lanes where (invocationIDs >= invocationsPerWorkgroup)
auto activeLaneMask = CmpLT(localInvocationIndex, SIMD::Int(invocationsPerWorkgroup));
- setSubgroupBuiltins(routine, workgroupID, localInvocationIndex, subgroupIndex);
+ setSubgroupBuiltins(data, routine, workgroupID, localInvocationIndex, subgroupIndex);
shader->emit(routine, activeLaneMask, descriptorSets);
}
diff --git a/src/Pipeline/ComputeProgram.hpp b/src/Pipeline/ComputeProgram.hpp
index 843812c..569a2c0 100644
--- a/src/Pipeline/ComputeProgram.hpp
+++ b/src/Pipeline/ComputeProgram.hpp
@@ -64,12 +64,10 @@
protected:
void emit(SpirvRoutine* routine);
- void setWorkgroupBuiltins(SpirvRoutine* routine, Int workgroupID[3]);
- void setSubgroupBuiltins(SpirvRoutine* routine, Int workgroupID[3], SIMD::Int localInvocationIndex, Int subgroupIndex);
+ void setWorkgroupBuiltins(Pointer<Byte> data, SpirvRoutine* routine, Int workgroupID[3]);
+ void setSubgroupBuiltins(Pointer<Byte> data, SpirvRoutine* routine, Int workgroupID[3], SIMD::Int localInvocationIndex, Int subgroupIndex);
void setInputBuiltin(SpirvRoutine* routine, spv::BuiltIn id, std::function<void(const SpirvShader::BuiltinMapping& builtin, Array<SIMD::Float>& value)> cb);
- Pointer<Byte> data; // argument 0
-
struct Data
{
vk::DescriptorSet::Bindings descriptorSets;
