Propagate descriptor decorations to access-chain and load results

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 @@ namespace sw
 			{
 				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 @@ namespace sw
 			{
 				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 @@ namespace sw
 
 			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 @@ namespace sw
 			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 @@ namespace sw
 			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 @@ namespace sw
 			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 @@ namespace sw
 
 			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 @@ namespace sw
 		}
 	}
 
-	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 @@ namespace sw
 			}
 		}
 
-		auto ptr = GetPointerToData(id, arrayIndex, routine);
+		auto ptr = GetPointerToData(baseId, arrayIndex, routine);
 
 		int constantOffset = 0;
 
@@ -1054,21 +1090,21 @@ namespace sw
 		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 @@ namespace sw
 			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 @@ namespace sw
 			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 @@ namespace sw
 		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 @@ namespace sw
 		}
 	}
 
+	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 @@ namespace sw
 
 	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 @@ namespace sw
 			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 @@ namespace sw
 		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 @@ namespace sw
 			break;
 		}
 		default:
+			UNIMPLEMENTED("Storage class %d", objectTy.storageClass);
 			break;
 		}
 
@@ -1933,6 +1976,10 @@ namespace sw
 		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 @@ namespace sw
 			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");

src/Pipeline/SpirvShader.hpp

@@ -282,8 +282,9 @@ namespace sw
 				// 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 @@ namespace sw
 
 		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 @@ namespace sw
 		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 @@ namespace sw
 		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.
 		//