SpirvShader: Use struct for MemoryVisitor callback.
Instead of calling a function with two uint32_t parameters, pass a MemoryElement structure that has named and documented fields.
This structure includes the Type field, which will be used by the debugger for knowing how to interpret the value.
Bug: b/145351270
Change-Id: I21e86b9ba117bcde21150b408d8a7552729203e2
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/38911
Presubmit-Ready: Ben Clayton <bclayton@google.com>
Tested-by: Ben Clayton <bclayton@google.com>
Reviewed-by: Chris Forbes <chrisforbes@google.com>
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
diff --git a/src/Pipeline/SpirvShader.hpp b/src/Pipeline/SpirvShader.hpp
index 899ff7e..827eefc 100644
--- a/src/Pipeline/SpirvShader.hpp
+++ b/src/Pipeline/SpirvShader.hpp
@@ -769,10 +769,23 @@
int VisitInterfaceInner(Type::ID id, Decorations d, const InterfaceVisitor& v) const;
- using MemoryVisitor = std::function<void(uint32_t index, uint32_t offset)>;
+ // MemoryElement describes a scalar element within a structure, and is
+ // used by the callback function of VisitMemoryObject().
+ struct MemoryElement
+ {
+ uint32_t index; // index of the scalar element
+ uint32_t offset; // offset (in bytes) from the base of the object
+ const Type& type; // element type
+ };
+ using MemoryVisitor = std::function<void(const MemoryElement&)>;
+
+ // VisitMemoryObject() walks a type tree in an explicitly laid out
+ // storage class, calling the MemoryVisitor for each scalar element
+ // within the
void VisitMemoryObject(Object::ID id, const MemoryVisitor& v) const;
+ // VisitMemoryObjectInner() is internally called by VisitMemoryObject()
void VisitMemoryObjectInner(Type::ID id, Decorations d, uint32_t &index, uint32_t offset, const MemoryVisitor& v) const;
Object& CreateConstant(InsnIterator it);
diff --git a/src/Pipeline/SpirvShaderMemory.cpp b/src/Pipeline/SpirvShaderMemory.cpp
index 498c274..806fb78 100644
--- a/src/Pipeline/SpirvShaderMemory.cpp
+++ b/src/Pipeline/SpirvShaderMemory.cpp
@@ -57,11 +57,11 @@
auto &dst = state->createIntermediate(resultId, resultTy.sizeInComponents);
auto robustness = state->getOutOfBoundsBehavior(pointerTy.storageClass);
- VisitMemoryObject(pointerId, [&](uint32_t i, uint32_t offset)
+ VisitMemoryObject(pointerId, [&](const MemoryElement& el)
{
- auto p = ptr + offset;
+ auto p = ptr + el.offset;
if (interleavedByLane) { p = InterleaveByLane(p); } // TODO: Interleave once, then add offset?
- dst.move(i, p.Load<SIMD::Float>(robustness, state->activeLaneMask(), atomic, memoryOrder));
+ dst.move(el.index, p.Load<SIMD::Float>(robustness, state->activeLaneMask(), atomic, memoryOrder));
});
return EmitResult::Continue;
@@ -101,22 +101,22 @@
{
// Constant source data.
const uint32_t *src = object.constantValue.get();
- VisitMemoryObject(pointerId, [&](uint32_t i, uint32_t offset)
+ VisitMemoryObject(pointerId, [&](const MemoryElement& el)
{
- auto p = ptr + offset;
+ auto p = ptr + el.offset;
if (interleavedByLane) { p = InterleaveByLane(p); }
- p.Store(SIMD::Int(src[i]), robustness, mask, atomic, memoryOrder);
+ p.Store(SIMD::Int(src[el.index]), robustness, mask, atomic, memoryOrder);
});
}
else
{
// Intermediate source data.
auto &src = state->getIntermediate(objectId);
- VisitMemoryObject(pointerId, [&](uint32_t i, uint32_t offset)
+ VisitMemoryObject(pointerId, [&](const MemoryElement& el)
{
- auto p = ptr + offset;
+ auto p = ptr + el.offset;
if (interleavedByLane) { p = InterleaveByLane(p); }
- p.Store(src.Float(i), robustness, mask, atomic, memoryOrder);
+ p.Store(src.Float(el.index), robustness, mask, atomic, memoryOrder);
});
}
@@ -239,12 +239,12 @@
bool interleavedByLane = IsStorageInterleavedByLane(objectTy.storageClass);
auto ptr = GetPointerToData(resultId, 0, state);
GenericValue initialValue(this, state, initializerId);
- VisitMemoryObject(resultId, [&](uint32_t i, uint32_t offset)
+ VisitMemoryObject(resultId, [&](const MemoryElement& el)
{
- auto p = ptr + offset;
+ auto p = ptr + el.offset;
if (interleavedByLane) { p = InterleaveByLane(p); }
auto robustness = OutOfBoundsBehavior::UndefinedBehavior; // Local variables are always within bounds.
- p.Store(initialValue.Float(i), robustness, state->activeLaneMask());
+ p.Store(initialValue.Float(el.index), robustness, state->activeLaneMask());
});
break;
}
@@ -271,13 +271,14 @@
std::unordered_map<uint32_t, uint32_t> srcOffsets;
- VisitMemoryObject(srcPtrId, [&](uint32_t i, uint32_t srcOffset) { srcOffsets[i] = srcOffset; });
+ VisitMemoryObject(srcPtrId, [&](const MemoryElement& el) { srcOffsets[el.index] = el.offset; });
- VisitMemoryObject(dstPtrId, [&](uint32_t i, uint32_t dstOffset)
+ VisitMemoryObject(dstPtrId, [&](const MemoryElement& el)
{
- auto it = srcOffsets.find(i);
+ auto it = srcOffsets.find(el.index);
ASSERT(it != srcOffsets.end());
auto srcOffset = it->second;
+ auto dstOffset = el.offset;
auto dst = dstPtr + dstOffset;
auto src = srcPtr + srcOffset;
@@ -302,15 +303,8 @@
return EmitResult::Continue;
}
-void SpirvShader::VisitMemoryObjectInner(sw::SpirvShader::Type::ID id, sw::SpirvShader::Decorations d, uint32_t& index, uint32_t offset, const MemoryVisitor &f) const
+void SpirvShader::VisitMemoryObjectInner(sw::SpirvShader::Type::ID id, sw::SpirvShader::Decorations d, uint32_t& index, uint32_t offset, const MemoryVisitor& 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 bytes) from the base of the
- // object.
-
ApplyDecorationsForId(&d, id);
auto const &type = getType(id);
@@ -327,7 +321,8 @@
break;
case spv::OpTypeInt:
case spv::OpTypeFloat:
- f(index++, offset);
+ case spv::OpTypeRuntimeArray:
+ f(MemoryElement{index++, offset, type});
break;
case spv::OpTypeVector:
{
@@ -371,7 +366,7 @@
}
}
-void SpirvShader::VisitMemoryObject(sw::SpirvShader::Object::ID id, const MemoryVisitor &f) const
+void SpirvShader::VisitMemoryObject(sw::SpirvShader::Object::ID id, const MemoryVisitor& f) const
{
auto typeId = getObject(id).type;
auto const & type = getType(typeId);
@@ -385,9 +380,11 @@
else
{
// Objects without explicit layout are tightly packed.
- for (auto i = 0u; i < getType(type.element).sizeInComponents; i++)
+ auto &elType = getType(type.element);
+ for (auto index = 0u; index < elType.sizeInComponents; index++)
{
- f(i, i * sizeof(float));
+ auto offset = static_cast<uint32_t>(index * sizeof(float));
+ f({index, offset, elType});
}
}
}
