Propagate descriptor decorations to access-chain and load results
Split descriptor decorations from other decorations since they're always
used separately and this makes propagating them more lightweight.
Bug b/129523279
Change-Id: I2f5acecc990bf15c7feb4ce81539bd2b5818bdf6
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/28445
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
Tested-by: Nicolas Capens <nicolas.capens@gmail.com>
Reviewed-by: Chris Forbes <chrisforbes@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/Pipeline/SpirvShader.cpp b/src/Pipeline/SpirvShader.cpp
index b7ba738..4aed978 100644
--- a/src/Pipeline/SpirvShader.cpp
+++ b/src/Pipeline/SpirvShader.cpp
@@ -12,9 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-#include <spirv/unified1/spirv.hpp>
-#include <spirv/unified1/GLSL.std.450.h>
#include "SpirvShader.hpp"
+
#include "System/Math.hpp"
#include "Vulkan/VkBuffer.hpp"
#include "Vulkan/VkDebug.hpp"
@@ -22,6 +21,9 @@
#include "Vulkan/VkPipelineLayout.hpp"
#include "Device/Config.hpp"
+#include <spirv/unified1/spirv.hpp>
+#include <spirv/unified1/GLSL.std.450.h>
+
#ifdef Bool
#undef Bool // b/127920555
#endif
@@ -173,9 +175,22 @@
{
TypeOrObjectID targetId = insn.word(1);
auto decoration = static_cast<spv::Decoration>(insn.word(2));
- decorations[targetId].Apply(
- decoration,
- insn.wordCount() > 3 ? insn.word(3) : 0);
+ uint32_t value = insn.wordCount() > 3 ? insn.word(3) : 0;
+
+ decorations[targetId].Apply(decoration, value);
+
+ switch(decoration)
+ {
+ case spv::DecorationDescriptorSet:
+ descriptorDecorations[targetId].DescriptorSet = value;
+ break;
+ case spv::DecorationBinding:
+ descriptorDecorations[targetId].Binding = value;
+ break;
+ default:
+ // Only handling descriptor decorations here.
+ break;
+ }
if (decoration == spv::DecorationCentroid)
modes.NeedsCentroid = true;
@@ -186,13 +201,14 @@
{
Type::ID targetId = insn.word(1);
auto memberIndex = insn.word(2);
+ auto decoration = static_cast<spv::Decoration>(insn.word(3));
+ uint32_t value = insn.wordCount() > 4 ? insn.word(4) : 0;
+
auto &d = memberDecorations[targetId];
if (memberIndex >= d.size())
d.resize(memberIndex + 1); // on demand; exact size would require another pass...
- auto decoration = static_cast<spv::Decoration>(insn.word(3));
- d[memberIndex].Apply(
- decoration,
- insn.wordCount() > 4 ? insn.word(4) : 0);
+
+ d[memberIndex].Apply(decoration, value);
if (decoration == spv::DecorationCentroid)
modes.NeedsCentroid = true;
@@ -207,12 +223,17 @@
case spv::OpGroupDecorate:
{
- auto const &srcDecorations = decorations[insn.word(1)];
+ uint32_t group = insn.word(1);
+ auto const &groupDecorations = decorations[group];
+ auto const &descriptorGroupDecorations = descriptorDecorations[group];
for (auto i = 2u; i < insn.wordCount(); i++)
{
- // remaining operands are targets to apply the group to.
- decorations[insn.word(i)].Apply(srcDecorations);
+ // Remaining operands are targets to apply the group to.
+ uint32_t target = insn.word(i);
+ decorations[target].Apply(groupDecorations);
+ descriptorDecorations[target].Apply(descriptorGroupDecorations);
}
+
break;
}
@@ -456,6 +477,21 @@
case spv::OpLoad:
case spv::OpAccessChain:
case spv::OpInBoundsAccessChain:
+ {
+ // Propagate the descriptor decorations to the result.
+ Object::ID resultId = insn.word(2);
+ Object::ID pointerId = insn.word(3);
+ const auto &d = descriptorDecorations.find(pointerId);
+
+ if(d != descriptorDecorations.end())
+ {
+ descriptorDecorations[resultId] = d->second;
+ }
+
+ DefineResult(insn);
+ }
+ break;
+
case spv::OpCompositeConstruct:
case spv::OpCompositeInsert:
case spv::OpCompositeExtract:
@@ -469,7 +505,8 @@
case spv::OpTranspose:
case spv::OpVectorExtractDynamic:
case spv::OpVectorInsertDynamic:
- case spv::OpNot: // Unary ops
+ // Unary ops
+ case spv::OpNot:
case spv::OpBitFieldInsert:
case spv::OpBitFieldSExtract:
case spv::OpBitFieldUExtract:
@@ -478,7 +515,8 @@
case spv::OpSNegate:
case spv::OpFNegate:
case spv::OpLogicalNot:
- case spv::OpIAdd: // Binary ops
+ // Binary ops
+ case spv::OpIAdd:
case spv::OpISub:
case spv::OpIMul:
case spv::OpSDiv:
@@ -934,10 +972,8 @@
case Object::Kind::DescriptorSet:
{
- Decorations d = {};
- ApplyDecorationsForId(&d, id);
-
- ASSERT(d.DescriptorSet >= 0);
+ const auto &d = descriptorDecorations.at(id);
+ ASSERT(d.DescriptorSet >= 0 && d.DescriptorSet < vk::MAX_BOUND_DESCRIPTOR_SETS);
ASSERT(d.Binding >= 0);
auto set = routine->getPointer(id);
@@ -965,11 +1001,11 @@
}
}
- SIMD::Pointer SpirvShader::WalkExplicitLayoutAccessChain(Object::ID id, uint32_t numIndexes, uint32_t const *indexIds, SpirvRoutine *routine) const
+ SIMD::Pointer SpirvShader::WalkExplicitLayoutAccessChain(Object::ID baseId, uint32_t numIndexes, uint32_t const *indexIds, SpirvRoutine *routine) const
{
// Produce a offset into external memory in sizeof(float) units
- auto &baseObject = getObject(id);
+ auto &baseObject = getObject(baseId);
Type::ID typeId = getType(baseObject.type).element;
Decorations d = {};
ApplyDecorationsForId(&d, baseObject.type);
@@ -989,7 +1025,7 @@
}
}
- auto ptr = GetPointerToData(id, arrayIndex, routine);
+ auto ptr = GetPointerToData(baseId, arrayIndex, routine);
int constantOffset = 0;
@@ -1054,21 +1090,21 @@
return ptr;
}
- SIMD::Int SpirvShader::WalkAccessChain(Object::ID id, uint32_t numIndexes, uint32_t const *indexIds, SpirvRoutine *routine) const
+ SIMD::Int SpirvShader::WalkAccessChain(Object::ID baseId, uint32_t numIndexes, uint32_t const *indexIds, SpirvRoutine *routine) const
{
// TODO: avoid doing per-lane work in some cases if we can?
// Produce a *component* offset into location-oriented memory
int constantOffset = 0;
SIMD::Int dynamicOffset = SIMD::Int(0);
- auto &baseObject = getObject(id);
+ auto &baseObject = getObject(baseId);
Type::ID typeId = getType(baseObject.type).element;
// The <base> operand is a divergent pointer itself.
// Start with its offset and build from there.
if (baseObject.kind == Object::Kind::DivergentPointer)
{
- dynamicOffset += routine->getIntermediate(id).Int(0);
+ dynamicOffset += routine->getIntermediate(baseId).Int(0);
}
for (auto i = 0u; i < numIndexes; i++)
@@ -1166,14 +1202,6 @@
HasComponent = true;
Component = arg;
break;
- case spv::DecorationDescriptorSet:
- HasDescriptorSet = true;
- DescriptorSet = arg;
- break;
- case spv::DecorationBinding:
- HasBinding = true;
- Binding = arg;
- break;
case spv::DecorationBuiltIn:
HasBuiltIn = true;
BuiltIn = static_cast<spv::BuiltIn>(arg);
@@ -1235,18 +1263,6 @@
Component = src.Component;
}
- if (src.HasDescriptorSet)
- {
- HasDescriptorSet = true;
- DescriptorSet = src.DescriptorSet;
- }
-
- if (src.HasBinding)
- {
- HasBinding = true;
- Binding = src.Binding;
- }
-
if (src.HasOffset)
{
HasOffset = true;
@@ -1273,6 +1289,19 @@
RelaxedPrecision |= src.RelaxedPrecision;
}
+ void SpirvShader::DescriptorDecorations::Apply(const sw::SpirvShader::DescriptorDecorations &src)
+ {
+ if(src.DescriptorSet >= 0)
+ {
+ DescriptorSet = src.DescriptorSet;
+ }
+
+ if(src.Binding >= 0)
+ {
+ Binding = src.Binding;
+ }
+ }
+
void SpirvShader::ApplyDecorationsForId(Decorations *d, TypeOrObjectID id) const
{
auto it = decorations.find(id);
@@ -1289,6 +1318,17 @@
}
}
+ void SpirvShader::DefineResult(const InsnIterator &insn)
+ {
+ Type::ID typeId = insn.word(1);
+ Object::ID resultId = insn.word(2);
+ auto &object = defs[resultId];
+ object.type = typeId;
+ object.kind = (getType(typeId).opcode() == spv::OpTypePointer)
+ ? Object::Kind::DivergentPointer : Object::Kind::Intermediate;
+ object.definition = insn;
+ }
+
uint32_t SpirvShader::GetConstantInt(Object::ID id) const
{
// Slightly hackish access to constants very early in translation.
@@ -1644,7 +1684,9 @@
SpirvShader::EmitResult SpirvShader::EmitInstruction(InsnIterator insn, EmitState *state) const
{
- switch (insn.opcode())
+ auto opcode = insn.opcode();
+
+ switch (opcode)
{
case spv::OpTypeVoid:
case spv::OpTypeInt:
@@ -1862,7 +1904,7 @@
return EmitKill(insn, state);
default:
- UNIMPLEMENTED("opcode: %s", OpcodeName(insn.opcode()).c_str());
+ UNIMPLEMENTED("opcode: %s", OpcodeName(opcode).c_str());
break;
}
@@ -1903,9 +1945,9 @@
case spv::StorageClassUniform:
case spv::StorageClassStorageBuffer:
{
- Decorations d{};
- ApplyDecorationsForId(&d, resultId);
- ASSERT(d.DescriptorSet >= 0);
+ const auto &d = descriptorDecorations.at(resultId);
+ ASSERT(d.DescriptorSet >= 0 && d.DescriptorSet < vk::MAX_BOUND_DESCRIPTOR_SETS);
+
routine->createPointer(resultId, routine->descriptorSets[d.DescriptorSet]);
break;
}
@@ -1915,6 +1957,7 @@
break;
}
default:
+ UNIMPLEMENTED("Storage class %d", objectTy.storageClass);
break;
}
@@ -1933,6 +1976,10 @@
auto &pointerTy = getType(pointer.type);
std::memory_order memoryOrder = std::memory_order_relaxed;
+ ASSERT(getType(pointer.type).element == result.type);
+ ASSERT(Type::ID(insn.word(1)) == result.type);
+ ASSERT(!atomic || getType(getType(pointer.type).element).opcode() == spv::OpTypeInt); // Vulkan 1.1: "Atomic instructions must declare a scalar 32-bit integer type, for the value pointed to by Pointer."
+
if(atomic)
{
Object::ID semanticsId = insn.word(5);
@@ -1940,10 +1987,6 @@
memoryOrder = MemoryOrder(memorySemantics);
}
- ASSERT(getType(pointer.type).element == result.type);
- ASSERT(Type::ID(insn.word(1)) == result.type);
- ASSERT(!atomic || getType(getType(pointer.type).element).opcode() == spv::OpTypeInt); // Vulkan 1.1: "Atomic instructions must declare a scalar 32-bit integer type, for the value pointed to by Pointer."
-
if (pointerTy.storageClass == spv::StorageClassImage)
{
UNIMPLEMENTED("StorageClassImage load not yet implemented");
diff --git a/src/Pipeline/SpirvShader.hpp b/src/Pipeline/SpirvShader.hpp
index f3e3919..28709da 100644
--- a/src/Pipeline/SpirvShader.hpp
+++ b/src/Pipeline/SpirvShader.hpp
@@ -282,8 +282,9 @@
// A pointer to a vk::DescriptorSet*.
// Pointer held by SpirvRoutine::pointers.
DescriptorSet,
+ };
- } kind = Kind::Unknown;
+ Kind kind = Kind::Unknown;
};
// Block is an interval of SPIR-V instructions, starting with the
@@ -401,38 +402,36 @@
struct Decorations
{
- int32_t Location;
- int32_t Component;
- int32_t DescriptorSet;
- int32_t Binding;
- spv::BuiltIn BuiltIn;
- int32_t Offset;
- int32_t ArrayStride;
- int32_t MatrixStride;
+ int32_t Location = -1;
+ int32_t Component = 0;
+ spv::BuiltIn BuiltIn = static_cast<spv::BuiltIn>(-1);
+ int32_t Offset = -1;
+ int32_t ArrayStride = -1;
+ int32_t MatrixStride = 1;
+
bool HasLocation : 1;
bool HasComponent : 1;
- bool HasDescriptorSet : 1;
- bool HasBinding : 1;
bool HasBuiltIn : 1;
+ bool HasOffset : 1;
+ bool HasArrayStride : 1;
+ bool HasMatrixStride : 1;
+
bool Flat : 1;
bool Centroid : 1;
bool NoPerspective : 1;
bool Block : 1;
bool BufferBlock : 1;
- bool HasOffset : 1;
- bool HasArrayStride : 1;
- bool HasMatrixStride : 1;
bool RelaxedPrecision : 1;
Decorations()
- : Location{-1}, Component{0}, DescriptorSet{-1}, Binding{-1},
+ : Location{-1}, Component{0},
BuiltIn{static_cast<spv::BuiltIn>(-1)},
Offset{-1}, ArrayStride{-1}, MatrixStride{-1},
HasLocation{false}, HasComponent{false},
- HasDescriptorSet{false}, HasBinding{false},
- HasBuiltIn{false}, Flat{false}, Centroid{false},
- NoPerspective{false}, Block{false}, BufferBlock{false},
- HasOffset{false}, HasArrayStride{false}, HasMatrixStride{false},
+ HasBuiltIn{false}, HasOffset{false},
+ HasArrayStride{false}, HasMatrixStride{false},
+ Flat{false}, Centroid{false}, NoPerspective{false},
+ Block{false}, BufferBlock{false},
RelaxedPrecision{false}
{
}
@@ -447,6 +446,16 @@
std::unordered_map<TypeOrObjectID, Decorations, TypeOrObjectID::Hash> decorations;
std::unordered_map<Type::ID, std::vector<Decorations>> memberDecorations;
+ struct DescriptorDecorations
+ {
+ int32_t DescriptorSet = -1;
+ int32_t Binding = -1;
+
+ void Apply(DescriptorDecorations const &src);
+ };
+
+ std::unordered_map<Object::ID, DescriptorDecorations> descriptorDecorations;
+
struct InterfaceComponent
{
AttribType Type;
@@ -525,6 +534,9 @@
void ApplyDecorationsForId(Decorations *d, TypeOrObjectID id) const;
void ApplyDecorationsForIdMember(Decorations *d, Type::ID id, uint32_t member) const;
+ // Creates an Object for the instruction's result in 'defs'.
+ void DefineResult(const InsnIterator &insn);
+
// Returns true if data in the given storage class is word-interleaved
// by each SIMD vector lane, otherwise data is stored linerally.
//
