Limit SPIR-V phi variable lifetimes
This change moves phi variables from SpirvRoutine to EmitState. The
former class represents the interface of a shader, and phis are not part
of that. EmitState only lives during emission of the actual SPIR-V code
into Reactor code, so it deletes these variables automatically and
prevents unnecessary materialization and reduces compilation overhead.
This also enabled reducing the public interface of EmitState to a single
static emit() function.
Bug: b/178662288
Bug: b/247020580
Change-Id: I88110c02e925699ad9706aaa3dc8779f9bbd3430
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/68729
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>
diff --git a/src/Pipeline/ComputeProgram.cpp b/src/Pipeline/ComputeProgram.cpp
index bb05cee..530491a 100644
--- a/src/Pipeline/ComputeProgram.cpp
+++ b/src/Pipeline/ComputeProgram.cpp
@@ -47,7 +47,6 @@
shader->emitProlog(&routine);
emit(&routine);
shader->emitEpilog(&routine);
- shader->clearPhis(&routine);
}
void ComputeProgram::setWorkgroupBuiltins(Pointer<Byte> data, SpirvRoutine *routine, Int workgroupID[3])
diff --git a/src/Pipeline/PixelProgram.cpp b/src/Pipeline/PixelProgram.cpp
index 6919cb3..df4f273 100644
--- a/src/Pipeline/PixelProgram.cpp
+++ b/src/Pipeline/PixelProgram.cpp
@@ -199,12 +199,6 @@
spirvShader->emit(&routine, activeLaneMask, storesAndAtomicsMask, descriptorSets, state.multiSampleCount);
spirvShader->emitEpilog(&routine);
- // At the last invocation of the fragment shader, clear phi data.
- // TODO(b/178662288): Automatically clear phis through SpirvRoutine lifetime reduction.
- if(samples[0] == static_cast<int>(state.multiSampleCount - 1))
- {
- spirvShader->clearPhis(&routine);
- }
for(int i = 0; i < MAX_COLOR_BUFFERS; i++)
{
diff --git a/src/Pipeline/SpirvShader.cpp b/src/Pipeline/SpirvShader.cpp
index 06fa405..08ebc52 100644
--- a/src/Pipeline/SpirvShader.cpp
+++ b/src/Pipeline/SpirvShader.cpp
@@ -1781,20 +1781,13 @@
auto resultPointerType = getType(insn.resultTypeId());
auto pointeeType = getType(resultPointerType.element);
- if(pointeeType.componentCount > 0) // TODO: what to do about zero-slot objects?
+ if(pointeeType.componentCount > 0)
{
routine->createVariable(insn.resultId(), pointeeType.componentCount);
}
}
break;
- case spv::OpPhi:
- {
- auto type = getType(insn.resultTypeId());
- routine->phis.emplace(insn.resultId(), SpirvRoutine::Variable(type.componentCount));
- }
- break;
-
case spv::OpImageSampleImplicitLod:
case spv::OpImageSampleExplicitLod:
case spv::OpImageSampleDrefImplicitLod:
@@ -1824,21 +1817,60 @@
void SpirvShader::emit(SpirvRoutine *routine, const RValue<SIMD::Int> &activeLaneMask, const RValue<SIMD::Int> &storesAndAtomicsMask, const vk::DescriptorSet::Bindings &descriptorSets, unsigned int multiSampleCount) const
{
- EmitState state(*this, routine, entryPoint, activeLaneMask, storesAndAtomicsMask, descriptorSets, multiSampleCount);
+ EmitState::emit(*this, routine, entryPoint, activeLaneMask, storesAndAtomicsMask, descriptorSets, multiSampleCount);
+}
+
+EmitState::EmitState(const SpirvShader &shader,
+ SpirvRoutine *routine,
+ SpirvShader::Function::ID entryPoint,
+ RValue<SIMD::Int> activeLaneMask,
+ RValue<SIMD::Int> storesAndAtomicsMask,
+ const vk::DescriptorSet::Bindings &descriptorSets,
+ unsigned int multiSampleCount)
+ : shader(shader)
+ , routine(routine)
+ , function(entryPoint)
+ , activeLaneMaskValue(activeLaneMask.value())
+ , storesAndAtomicsMaskValue(storesAndAtomicsMask.value())
+ , descriptorSets(descriptorSets)
+ , multiSampleCount(multiSampleCount)
+{
+}
+
+void EmitState::emit(const SpirvShader &shader,
+ SpirvRoutine *routine,
+ SpirvShader::Function::ID entryPoint,
+ RValue<SIMD::Int> activeLaneMask,
+ RValue<SIMD::Int> storesAndAtomicsMask,
+ const vk::DescriptorSet::Bindings &descriptorSets,
+ unsigned int multiSampleCount)
+{
+ EmitState state(shader, routine, entryPoint, activeLaneMask, storesAndAtomicsMask, descriptorSets, multiSampleCount);
+
+ // Create phi variables
+ for(auto insn : shader)
+ {
+ if(insn.opcode() == spv::OpPhi)
+ {
+ auto type = shader.getType(insn.resultTypeId());
+ state.phis.emplace(insn.resultId(), Array<SIMD::Float>(type.componentCount));
+ }
+ }
// Emit everything up to the first label
// TODO: Separate out dispatch of block from non-block instructions?
- for(auto insn : *this)
+ for(auto insn : shader)
{
if(insn.opcode() == spv::OpLabel)
{
break;
}
+
state.EmitInstruction(insn);
}
// Emit all the blocks starting from entryPoint.
- state.EmitBlocks(getFunction(entryPoint).entry);
+ state.EmitBlocks(shader.getFunction(entryPoint).entry);
}
void EmitState::EmitInstructions(InsnIterator begin, InsnIterator end)
@@ -2716,15 +2748,6 @@
}
}
-void SpirvShader::clearPhis(SpirvRoutine *routine) const
-{
- // Clear phis that are no longer used. This serves two purposes:
- // (1) The phi rr::Variables are destructed, preventing pointless
- // materialization.
- // (2) Frees memory that will never be used again.
- routine->phis.clear();
-}
-
VkShaderStageFlagBits SpirvShader::executionModelToStage(spv::ExecutionModel model)
{
switch(model)
diff --git a/src/Pipeline/SpirvShader.hpp b/src/Pipeline/SpirvShader.hpp
index 9a3ad64..2a40a53 100644
--- a/src/Pipeline/SpirvShader.hpp
+++ b/src/Pipeline/SpirvShader.hpp
@@ -813,10 +813,10 @@
std::vector<InterfaceComponent> inputs;
std::vector<InterfaceComponent> outputs;
+ // TODO(b/247020580): Move to SpirvRoutine
void emitProlog(SpirvRoutine *routine) const;
void emit(SpirvRoutine *routine, const RValue<SIMD::Int> &activeLaneMask, const RValue<SIMD::Int> &storesAndAtomicsMask, const vk::DescriptorSet::Bindings &descriptorSets, unsigned int multiSampleCount = 0) const;
void emitEpilog(SpirvRoutine *routine) const;
- void clearPhis(SpirvRoutine *routine) const;
uint32_t getWorkgroupSizeX() const;
uint32_t getWorkgroupSizeY() const;
@@ -990,22 +990,28 @@
using Span = SpirvShader::Span;
public:
+ static void emit(const SpirvShader &shader,
+ SpirvRoutine *routine,
+ SpirvShader::Function::ID entryPoint,
+ RValue<SIMD::Int> activeLaneMask,
+ RValue<SIMD::Int> storesAndAtomicsMask,
+ const vk::DescriptorSet::Bindings &descriptorSets,
+ unsigned int multiSampleCount);
+
+ // Helper for calling rr::Yield with result cast to an rr::Int.
+ enum class YieldResult
+ {
+ ControlBarrier = 0,
+ };
+
+private:
EmitState(const SpirvShader &shader,
SpirvRoutine *routine,
- SpirvShader::Function::ID function,
+ SpirvShader::Function::ID entryPoint,
RValue<SIMD::Int> activeLaneMask,
RValue<SIMD::Int> storesAndAtomicsMask,
const vk::DescriptorSet::Bindings &descriptorSets,
- unsigned int multiSampleCount)
- : shader(shader)
- , routine(routine)
- , function(function)
- , activeLaneMaskValue(activeLaneMask.value())
- , storesAndAtomicsMaskValue(storesAndAtomicsMask.value())
- , descriptorSets(descriptorSets)
- , multiSampleCount(multiSampleCount)
- {
- }
+ unsigned int multiSampleCount);
// Returns the mask describing the active lanes as updated by dynamic
// control flow. Active lanes include helper invocations, used for
@@ -1481,16 +1487,11 @@
// StorePhi updates the phi's alloca storage value using the incoming
// values from blocks that are both in the OpPhi instruction and in
// filter.
- void StorePhi(Block::ID blockID, InsnIterator insn, const std::unordered_set<Block::ID> &filter) const;
+ void StorePhi(Block::ID blockID, InsnIterator insn, const std::unordered_set<Block::ID> &filter);
// Emits a rr::Fence for the given MemorySemanticsMask.
void Fence(spv::MemorySemanticsMask semantics) const;
- // Helper for calling rr::Yield with res cast to an rr::Int.
- enum class YieldResult
- {
- ControlBarrier = 0,
- };
void Yield(YieldResult res) const;
// Helper as we often need to take dot products as part of doing other things.
@@ -1511,7 +1512,6 @@
static sw::MipmapType convertMipmapMode(const vk::SamplerState *samplerState);
static sw::AddressingMode convertAddressingMode(int coordinateIndex, const vk::SamplerState *samplerState, VkImageViewType imageViewType);
-private:
const SpirvShader &shader;
SpirvRoutine *const routine; // The current routine being built.
SpirvShader::Function::ID function; // The current function being built.
@@ -1525,6 +1525,7 @@
const vk::DescriptorSet::Bindings &descriptorSets;
std::unordered_map<Object::ID, Intermediate> intermediates;
+ std::unordered_map<Object::ID, Array<SIMD::Float>> phis;
std::unordered_map<Object::ID, SIMD::Pointer> pointers;
std::unordered_map<Object::ID, SampledImagePointer> sampledImages;
@@ -1639,15 +1640,6 @@
f(builtin, getVariable(builtin.Id));
}
}
-
-private:
- // The phis are only accessible to SpirvShader
- // as they are only used and exist between calls to
- // SpirvShader::emitProlog() and SpirvShader::emitEpilog().
- friend class SpirvShader;
-
-public:
- std::unordered_map<Object::ID, Variable> phis;
};
} // namespace sw
diff --git a/src/Pipeline/SpirvShaderControlFlow.cpp b/src/Pipeline/SpirvShaderControlFlow.cpp
index e50d4c5..5783724 100644
--- a/src/Pipeline/SpirvShaderControlFlow.cpp
+++ b/src/Pipeline/SpirvShaderControlFlow.cpp
@@ -679,8 +679,8 @@
auto type = shader.getType(typeId);
auto objectId = Object::ID(insn.word(2));
- auto storageIt = routine->phis.find(objectId);
- ASSERT(storageIt != routine->phis.end());
+ auto storageIt = phis.find(objectId);
+ ASSERT(storageIt != phis.end());
auto &storage = storageIt->second;
auto &dst = createIntermediate(objectId, type.componentCount);
@@ -691,14 +691,14 @@
}
}
-void EmitState::StorePhi(Block::ID currentBlock, InsnIterator insn, const std::unordered_set<Block::ID> &filter) const
+void EmitState::StorePhi(Block::ID currentBlock, InsnIterator insn, const std::unordered_set<Block::ID> &filter)
{
auto typeId = Type::ID(insn.word(1));
auto type = shader.getType(typeId);
auto objectId = Object::ID(insn.word(2));
- auto storageIt = routine->phis.find(objectId);
- ASSERT(storageIt != routine->phis.end());
+ auto storageIt = phis.find(objectId);
+ ASSERT(storageIt != phis.end());
auto &storage = storageIt->second;
for(uint32_t w = 3; w < insn.wordCount(); w += 2)
diff --git a/src/Pipeline/VertexProgram.cpp b/src/Pipeline/VertexProgram.cpp
index 9a553cf..3ffa499 100644
--- a/src/Pipeline/VertexProgram.cpp
+++ b/src/Pipeline/VertexProgram.cpp
@@ -85,7 +85,6 @@
spirvShader->emit(&routine, activeLaneMask, storesAndAtomicsMask, descriptorSets);
spirvShader->emitEpilog(&routine);
- spirvShader->clearPhis(&routine);
}
} // namespace sw
