Fix handling of loads/stores of explicitly-laid-out objects [Note: New change-id as I'm experimenting with moving this on top of Nicolas' decoration tweaks] Bug: b/128690261 Test: dEQP-VK.glsl.* Test: dEQP-VK.ubo.* Test: dEQP-VK.ssbo.* Change-Id: I535c07964cd74dd4fd9c92c179aa34c53b3ff907 Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/28910 Tested-by: Chris Forbes <chrisforbes@google.com> Presubmit-Ready: Chris Forbes <chrisforbes@google.com> Reviewed-by: Nicolas Capens <nicolascapens@google.com> Reviewed-by: Ben Clayton <bclayton@google.com> Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
diff --git a/src/Pipeline/SpirvShader.cpp b/src/Pipeline/SpirvShader.cpp index 0063dbc..6025001 100644 --- a/src/Pipeline/SpirvShader.cpp +++ b/src/Pipeline/SpirvShader.cpp
@@ -490,6 +490,15 @@ } DefineResult(insn); + + if (insn.opcode() == spv::OpAccessChain || insn.opcode() == spv::OpInBoundsAccessChain) + { + Decorations dd{}; + ApplyDecorationsForAccessChain(&dd, pointerId, insn.wordCount() - 4, insn.wordPointer(4)); + // Note: offset is the one thing that does *not* propagate, as the access chain accounts for it. + dd.HasOffset = false; + decorations[resultId].Apply(dd); + } } break; @@ -959,6 +968,91 @@ VisitInterfaceInner<F>(def.word(1), d, f); } + template<typename F> + void SpirvShader::VisitMemoryObjectInner(sw::SpirvShader::Type::ID id, sw::SpirvShader::Decorations d, uint32_t& index, uint32_t offset, F f) const + { + // Walk a type tree in an explicitly laid out storage class, calling + // a functor for each scalar element within the object. + + // The functor's first parameter is the index of the scalar element; + // the second parameter is the offset (in sizeof(float) units) from + // the base of the object. + + ApplyDecorationsForId(&d, id); + auto const &type = getType(id); + + if (d.HasOffset) + { + offset += d.Offset / sizeof(float); + d.HasOffset = false; + } + + switch (type.opcode()) + { + case spv::OpTypePointer: + VisitMemoryObjectInner<F>(type.definition.word(3), d, index, offset, f); + break; + case spv::OpTypeInt: + case spv::OpTypeFloat: + f(index++, offset); + break; + case spv::OpTypeVector: + for (auto i = 0u; i < type.definition.word(3); i++) + { + VisitMemoryObjectInner(type.definition.word(2), d, index, offset + i, f); + } + break; + case spv::OpTypeMatrix: + for (auto i = 0u; i < type.definition.word(3); i++) + { + ASSERT(d.HasMatrixStride); + VisitMemoryObjectInner(type.definition.word(2), d, index, offset + i * d.MatrixStride / sizeof(float), f); + } + break; + case spv::OpTypeStruct: + for (auto i = 0u; i < type.definition.wordCount() - 2; i++) + { + ApplyDecorationsForIdMember(&d, id, i); + VisitMemoryObjectInner<F>(type.definition.word(i + 2), d, index, offset, f); + } + break; + case spv::OpTypeArray: + { + auto arraySize = GetConstantInt(type.definition.word(3)); + for (auto i = 0u; i < arraySize; i++) + { + ASSERT(d.HasArrayStride); + VisitMemoryObjectInner<F>(type.definition.word(2), d, index, offset + i * d.ArrayStride / sizeof(float), f); + } + break; + } + default: + UNIMPLEMENTED("%s", OpcodeName(type.opcode()).c_str()); + } + } + + template<typename F> + void SpirvShader::VisitMemoryObject(sw::SpirvShader::Object::ID id, F f) const + { + auto typeId = getObject(id).type; + auto const & type = getType(typeId); + if (!IsStorageInterleavedByLane(type.storageClass)) // TODO: really "is explicit layout" + { + Decorations d{}; + ApplyDecorationsForId(&d, id); + uint32_t index = 0; + VisitMemoryObjectInner<F>(typeId, d, index, 0, f); + } + else + { + // Objects without explicit layout are tightly packed. + for (auto i = 0u; i < getType(type.element).sizeInComponents; i++) + { + f(i, i); + } + } + } + SIMD::Pointer SpirvShader::GetPointerToData(Object::ID id, int arrayIndex, SpirvRoutine *routine) const { auto &object = getObject(id); @@ -1002,6 +1096,39 @@ } } + void SpirvShader::ApplyDecorationsForAccessChain(Decorations *d, Object::ID baseId, uint32_t numIndexes, uint32_t const *indexIds) const + { + ApplyDecorationsForId(d, baseId); + auto &baseObject = getObject(baseId); + ApplyDecorationsForId(d, baseObject.type); + auto typeId = getType(baseObject.type).element; + + for (auto i = 0u; i < numIndexes; i++) + { + ApplyDecorationsForId(d, typeId); + auto & type = getType(typeId); + switch (type.opcode()) + { + case spv::OpTypeStruct: + { + int memberIndex = GetConstantInt(indexIds[i]); + ApplyDecorationsForIdMember(d, typeId, memberIndex); + typeId = type.definition.word(2u + memberIndex); + break; + } + case spv::OpTypeArray: + case spv::OpTypeRuntimeArray: + case spv::OpTypeMatrix: + case spv::OpTypeVector: + typeId = type.element; + break; + default: + UNIMPLEMENTED("Unexpected type '%s' in ApplyDecorationsForAccessChain", + OpcodeName(type.definition.opcode()).c_str()); + } + } + } + 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 @@ -1033,6 +1160,8 @@ for (auto i = 0u; i < numIndexes; i++) { auto & type = getType(typeId); + ApplyDecorationsForId(&d, typeId); + switch (type.definition.opcode()) { case spv::OpTypeStruct: @@ -1048,7 +1177,6 @@ case spv::OpTypeRuntimeArray: { // TODO: b/127950082: Check bounds. - ApplyDecorationsForId(&d, typeId); ASSERT(d.HasArrayStride); auto & obj = getObject(indexIds[i]); if (obj.kind == Object::Kind::Constant) @@ -1061,7 +1189,6 @@ case spv::OpTypeMatrix: { // TODO: b/127950082: Check bounds. - ApplyDecorationsForId(&d, typeId); ASSERT(d.HasMatrixStride); auto & obj = getObject(indexIds[i]); if (obj.kind == Object::Kind::Constant) @@ -2001,19 +2128,19 @@ If(!ptr.uniform || anyInactiveLanes) { // Divergent offsets or masked lanes. - for (auto i = 0u; i < resultTy.sizeInComponents; i++) + VisitMemoryObject(pointerId, [&](uint32_t i, uint32_t o) { // i wish i had a Float,Float,Float,Float constructor here.. for (int j = 0; j < SIMD::Width; j++) { If(Extract(state->activeLaneMask(), j) != 0) { - Int offset = Int(i) + Extract(ptr.offset, j); + Int offset = Int(o) + Extract(ptr.offset, j); if (interleavedByLane) { offset = offset * SIMD::Width + j; } load[i] = Insert(load[i], Load(&ptr.base[offset], sizeof(float), atomic, memoryOrder), j); } } - } + }); } Else { @@ -2022,18 +2149,18 @@ { // Lane-interleaved data. Pointer<SIMD::Float> src = ptr.base; - for (auto i = 0u; i < resultTy.sizeInComponents; i++) + VisitMemoryObject(pointerId, [&](uint32_t i, uint32_t o) { - load[i] = Load(&src[i], sizeof(float), atomic, memoryOrder); // TODO: optimize alignment - } + load[i] = Load(&src[o], sizeof(float), atomic, memoryOrder); // TODO: optimize alignment + }); } else { // Non-interleaved data. - for (auto i = 0u; i < resultTy.sizeInComponents; i++) + VisitMemoryObject(pointerId, [&](uint32_t i, uint32_t o) { - load[i] = RValue<SIMD::Float>(Load(&ptr.base[i], sizeof(float), atomic, memoryOrder)); // TODO: optimize alignment - } + load[i] = RValue<SIMD::Float>(Load(&ptr.base[o], sizeof(float), atomic, memoryOrder)); // TODO: optimize alignment + }); } } @@ -2084,29 +2211,28 @@ If(!ptr.uniform || anyInactiveLanes) { // Divergent offsets or masked lanes. - - for (auto i = 0u; i < elementTy.sizeInComponents; i++) + VisitMemoryObject(pointerId, [&](uint32_t i, uint32_t o) { for (int j = 0; j < SIMD::Width; j++) { If(Extract(state->activeLaneMask(), j) != 0) { - Int offset = Int(i) + Extract(ptr.offset, j); + Int offset = Int(o) + Extract(ptr.offset, j); if (interleavedByLane) { offset = offset * SIMD::Width + j; } Store(RValue<Float>(src[i]), &ptr.base[offset], sizeof(float), atomic, memoryOrder); } } - } + }); } Else { // Constant source data. // No divergent offsets or masked lanes. Pointer<SIMD::Float> dst = ptr.base; - for (auto i = 0u; i < elementTy.sizeInComponents; i++) + VisitMemoryObject(pointerId, [&](uint32_t i, uint32_t o) { - Store(RValue<SIMD::Float>(src[i]), &dst[i], sizeof(float), atomic, memoryOrder); // TODO: optimize alignment - } + Store(RValue<SIMD::Float>(src[i]), &dst[o], sizeof(float), atomic, memoryOrder); // TODO: optimize alignment + }); } } else @@ -2116,18 +2242,18 @@ If(!ptr.uniform || anyInactiveLanes) { // Divergent offsets or masked lanes. - for (auto i = 0u; i < elementTy.sizeInComponents; i++) + VisitMemoryObject(pointerId, [&](uint32_t i, uint32_t o) { for (int j = 0; j < SIMD::Width; j++) { If(Extract(state->activeLaneMask(), j) != 0) { - Int offset = Int(i) + Extract(ptr.offset, j); + Int offset = Int(o) + Extract(ptr.offset, j); if (interleavedByLane) { offset = offset * SIMD::Width + j; } Store(Extract(src.Float(i), j), &ptr.base[offset], sizeof(float), atomic, memoryOrder); } } - } + }); } Else { @@ -2136,19 +2262,19 @@ { // Lane-interleaved data. Pointer<SIMD::Float> dst = ptr.base; - for (auto i = 0u; i < elementTy.sizeInComponents; i++) + VisitMemoryObject(pointerId, [&](uint32_t i, uint32_t o) { - Store(src.Float(i), &dst[i], sizeof(float), atomic, memoryOrder); // TODO: optimize alignment - } + Store(src.Float(i), &dst[o], sizeof(float), atomic, memoryOrder); // TODO: optimize alignment + }); } else { // Intermediate source data. Non-interleaved data. Pointer<SIMD::Float> dst = ptr.base; - for (auto i = 0u; i < elementTy.sizeInComponents; i++) + VisitMemoryObject(pointerId, [&](uint32_t i, uint32_t o) { - Store<SIMD::Float>(SIMD::Float(src.Float(i)), &dst[i], sizeof(float), atomic, memoryOrder); // TODO: optimize alignment - } + Store<SIMD::Float>(SIMD::Float(src.Float(i)), &dst[o], sizeof(float), atomic, memoryOrder); // TODO: optimize alignment + }); } } }
diff --git a/src/Pipeline/SpirvShader.hpp b/src/Pipeline/SpirvShader.hpp index 1527c9a..7af96df 100644 --- a/src/Pipeline/SpirvShader.hpp +++ b/src/Pipeline/SpirvShader.hpp
@@ -534,6 +534,7 @@ uint32_t ComputeTypeSize(InsnIterator insn); void ApplyDecorationsForId(Decorations *d, TypeOrObjectID id) const; void ApplyDecorationsForIdMember(Decorations *d, Type::ID id, uint32_t member) const; + void ApplyDecorationsForAccessChain(Decorations *d, Object::ID baseId, uint32_t numIndexes, uint32_t const *indexIds) const; // Creates an Object for the instruction's result in 'defs'. void DefineResult(const InsnIterator &insn); @@ -588,6 +589,12 @@ template<typename F> void VisitInterface(Object::ID id, F f) const; + template<typename F> + void VisitMemoryObject(Object::ID id, F f) const; + + template<typename F> + void VisitMemoryObjectInner(Type::ID id, Decorations d, uint32_t &index, uint32_t offset, F f) const; + uint32_t GetConstantInt(Object::ID id) const; Object& CreateConstant(InsnIterator it);