Skip to content

S
swiftshader
Commit 518f4384 by Ben Clayton
Options

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: 's avatarBen Clayton <bclayton@google.com> Reviewed-by: 's avatarNicolas Capens <nicolascapens@google.com>
parent 3429bb56
Showing with 8 additions and 10 deletions
src/Pipeline/ComputeProgram.cpp
...@@ -28,8 +28,7 @@ namespace ...@@ -28,8 +28,7 @@ namespace
namespace sw namespace sw
{ {
ComputeProgram::ComputeProgram(SpirvShader const *shader, vk::PipelineLayout const *pipelineLayout, const vk::DescriptorSet::Bindings &descriptorSets) ComputeProgram::ComputeProgram(SpirvShader const *shader, vk::PipelineLayout const *pipelineLayout, const vk::DescriptorSet::Bindings &descriptorSets)
: data(Arg<0>()), : shader(shader),
shader(shader),
pipelineLayout(pipelineLayout), pipelineLayout(pipelineLayout),
descriptorSets(descriptorSets) descriptorSets(descriptorSets)
{ {
...@@ -47,7 +46,7 @@ namespace sw ...@@ -47,7 +46,7 @@ namespace sw
shader->emitEpilog(&routine); 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) setInputBuiltin(routine, spv::BuiltInNumWorkgroups, [&](const SpirvShader::BuiltinMapping& builtin, Array<SIMD::Float>& value)
{ {
...@@ -106,7 +105,7 @@ namespace sw ...@@ -106,7 +105,7 @@ namespace sw
}); });
} }
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 numWorkgroups = *Pointer<Int4>(data + OFFSET(Data, numWorkgroups));
Int4 workgroupSize = *Pointer<Int4>(data + OFFSET(Data, workgroupSize)); Int4 workgroupSize = *Pointer<Int4>(data + OFFSET(Data, workgroupSize));
...@@ -163,6 +162,7 @@ namespace sw ...@@ -163,6 +162,7 @@ namespace sw
void ComputeProgram::emit(SpirvRoutine* routine) void ComputeProgram::emit(SpirvRoutine* routine)
{ {
Pointer<Byte> data = Arg<0>();
Int workgroupX = Arg<1>(); Int workgroupX = Arg<1>();
Int workgroupY = Arg<2>(); Int workgroupY = Arg<2>();
Int workgroupZ = Arg<3>(); Int workgroupZ = Arg<3>();
...@@ -179,7 +179,7 @@ namespace sw ...@@ -179,7 +179,7 @@ namespace sw
Int invocationsPerWorkgroup = *Pointer<Int>(data + OFFSET(Data, invocationsPerWorkgroup)); Int invocationsPerWorkgroup = *Pointer<Int>(data + OFFSET(Data, invocationsPerWorkgroup));
Int workgroupID[3] = {workgroupX, workgroupY, workgroupZ}; Int workgroupID[3] = {workgroupX, workgroupY, workgroupZ};
setWorkgroupBuiltins(routine, workgroupID); setWorkgroupBuiltins(data, routine, workgroupID);
For(Int i = 0, i < subgroupCount, i++) For(Int i = 0, i < subgroupCount, i++)
{ {
...@@ -191,7 +191,7 @@ namespace sw ...@@ -191,7 +191,7 @@ namespace sw
// Disable lanes where (invocationIDs >= invocationsPerWorkgroup) // Disable lanes where (invocationIDs >= invocationsPerWorkgroup)
auto activeLaneMask = CmpLT(localInvocationIndex, SIMD::Int(invocationsPerWorkgroup)); auto activeLaneMask = CmpLT(localInvocationIndex, SIMD::Int(invocationsPerWorkgroup));
setSubgroupBuiltins(routine, workgroupID, localInvocationIndex, subgroupIndex); setSubgroupBuiltins(data, routine, workgroupID, localInvocationIndex, subgroupIndex);
shader->emit(routine, activeLaneMask, descriptorSets); shader->emit(routine, activeLaneMask, descriptorSets);
} }
......
src/Pipeline/ComputeProgram.hpp
...@@ -64,12 +64,10 @@ namespace sw ...@@ -64,12 +64,10 @@ namespace sw
protected: protected:
void emit(SpirvRoutine* routine); void emit(SpirvRoutine* routine);
void setWorkgroupBuiltins(SpirvRoutine* routine, Int workgroupID[3]); void setWorkgroupBuiltins(Pointer<Byte> data, SpirvRoutine* routine, Int workgroupID[3]);
void setSubgroupBuiltins(SpirvRoutine* routine, Int workgroupID[3], SIMD::Int localInvocationIndex, Int subgroupIndex); 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); 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 struct Data
{ {
vk::DescriptorSet::Bindings descriptorSets; vk::DescriptorSet::Bindings descriptorSets;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment