Split types from other kinds of definitions
We always know from the context whether a particular id refers to a type
or something else. Split types out into their own map, and add an
accessor which ensures the type actually exists. This isn't so much
about checking for the provided code being valid, and more about
catching likely foul-ups in our own code.
Change-Id: If18831b1b604eed03fbbeaf352272b5ba15b37a9
Reviewed-on: https://swiftshader-review.googlesource.com/c/23608
Tested-by: Chris Forbes <chrisforbes@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>
diff --git a/src/Pipeline/SpirvShader.cpp b/src/Pipeline/SpirvShader.cpp
index 060a6ff..51c67d7 100644
--- a/src/Pipeline/SpirvShader.cpp
+++ b/src/Pipeline/SpirvShader.cpp
@@ -109,7 +109,7 @@
case spv::OpTypeFunction:
{
auto resultId = insn.word(1);
- auto &object = defs[resultId];
+ auto &object = types[resultId];
object.kind = Object::Kind::Type;
object.definition = insn;
object.sizeInComponents = ComputeTypeSize(insn);
@@ -134,7 +134,7 @@
else if (insn.opcode() == spv::OpTypePointer)
{
auto pointeeType = insn.word(3);
- object.isBuiltInBlock = defs[pointeeType].isBuiltInBlock;
+ object.isBuiltInBlock = getType(pointeeType).isBuiltInBlock;
}
break;
}
@@ -152,7 +152,7 @@
object.definition = insn;
object.storageClass = storageClass;
- auto &type = defs[typeId];
+ auto &type = getType(typeId);
object.sizeInComponents = type.sizeInComponents;
object.isBuiltInBlock = type.isBuiltInBlock;
@@ -177,7 +177,7 @@
auto &object = defs[resultId];
object.kind = Object::Kind::Constant;
object.definition = insn;
- object.sizeInComponents = defs[typeId].sizeInComponents;
+ object.sizeInComponents = getType(typeId).sizeInComponents;
break;
}
@@ -206,17 +206,17 @@
if (object.isBuiltInBlock)
{
// walk the builtin block, registering each of its members separately.
- auto ptrType = defs[object.definition.word(1)].definition;
+ auto ptrType = getType(object.definition.word(1)).definition;
assert(ptrType.opcode() == spv::OpTypePointer);
auto pointeeType = ptrType.word(3);
auto m = memberDecorations.find(pointeeType);
assert(m != memberDecorations.end()); // otherwise we wouldn't have marked the type chain
- auto structType = defs[pointeeType].definition;
+ auto &structType = getType(pointeeType).definition;
auto offset = 0u;
auto word = 2u;
for (auto &member : m->second)
{
- auto &memberType = defs[structType.word(word)];
+ auto &memberType = getType(structType.word(word));
if (member.HasBuiltIn)
{
@@ -301,13 +301,13 @@
case spv::OpTypeVector:
case spv::OpTypeMatrix:
// Vectors and matrices both consume element count * element size.
- return defs[insn.word(2)].sizeInComponents * insn.word(3);
+ return getType(insn.word(2)).sizeInComponents * insn.word(3);
case spv::OpTypeArray:
{
// Element count * element size. Array sizes come from constant ids.
auto arraySize = GetConstantInt(insn.word(3));
- return defs[insn.word(2)].sizeInComponents * arraySize;
+ return getType(insn.word(2)).sizeInComponents * arraySize;
}
case spv::OpTypeStruct:
@@ -315,14 +315,15 @@
uint32_t size = 0;
for (uint32_t i = 2u; i < insn.wordCount(); i++)
{
- size += defs[insn.word(i)].sizeInComponents;
+ size += getType(insn.word(i)).sizeInComponents;
}
return size;
}
case spv::OpTypePointer:
// Pointer 'size' is just pointee size
- return defs[insn.word(3)].sizeInComponents;
+ // TODO: this isn't really correct. we should look through pointers as appropriate.
+ return getType(insn.word(3)).sizeInComponents;
default:
// Some other random insn.
@@ -360,7 +361,7 @@
d.Apply(it->second);
}
- auto const &obj = defs[id];
+ auto const &obj = getType(id);
switch (obj.definition.opcode())
{
case spv::OpVariable:
@@ -503,7 +504,7 @@
// but is possible to construct integer constant 0 via OpConstantNull.
auto insn = defs[id].definition;
assert(insn.opcode() == spv::OpConstant);
- assert(defs[insn.word(1)].definition.opcode() == spv::OpTypeInt);
+ assert(getType(insn.word(1)).definition.opcode() == spv::OpTypeInt);
return insn.word(3);
}
}
diff --git a/src/Pipeline/SpirvShader.hpp b/src/Pipeline/SpirvShader.hpp
index d900da0..28a0245 100644
--- a/src/Pipeline/SpirvShader.hpp
+++ b/src/Pipeline/SpirvShader.hpp
@@ -214,10 +214,18 @@
const int serialID;
static volatile int serialCounter;
Modes modes;
+ std::unordered_map<uint32_t, Object> types;
std::unordered_map<uint32_t, Object> defs;
std::unordered_map<spv::BuiltIn, BuiltinMapping> inputBuiltins;
std::unordered_map<spv::BuiltIn, BuiltinMapping> outputBuiltins;
+ Object const &getType(uint32_t id) const
+ {
+ auto it = types.find(id);
+ assert(it != types.end());
+ return it->second;
+ }
+
void ProcessExecutionMode(InsnIterator it);
uint32_t ComputeTypeSize(InsnIterator insn);
