Only store component count in Operand
The Operand class is a low-level abstraction of rvalues and constants.
It should not carry the SPIR-V type ID. We only need the size in
components.
Bug: b/129000021
Change-Id: I6cb3ed6341b1ccf5ef759075d7410ba447617c8b
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/43693
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
diff --git a/src/Pipeline/SpirvShader.cpp b/src/Pipeline/SpirvShader.cpp
index 159bc73..b098cb1 100644
--- a/src/Pipeline/SpirvShader.cpp
+++ b/src/Pipeline/SpirvShader.cpp
@@ -795,7 +795,7 @@
auto &objectTy = getType(typeId);
object.kind = Object::Kind::Constant;
object.definition = insn;
- object.constantValue = std::unique_ptr<uint32_t[]>(new uint32_t[objectTy.componentCount]);
+ object.constantValue.resize(objectTy.componentCount);
return object;
}
@@ -2138,7 +2138,7 @@
auto &type = getType(insn.resultTypeId());
auto &dst = state->createIntermediate(insn.resultId(), type.componentCount);
auto cond = Operand(this, state, insn.word(3));
- auto condIsScalar = (getType(cond).componentCount == 1);
+ auto condIsScalar = (cond.componentCount == 1);
auto lhs = Operand(this, state, insn.word(4));
auto rhs = Operand(this, state, insn.word(5));
@@ -2419,11 +2419,18 @@
}
}
-SpirvShader::Operand::Operand(SpirvShader const *shader, EmitState const *state, SpirvShader::Object::ID objId)
- : obj(shader->getObject(objId))
- , intermediate(obj.kind == SpirvShader::Object::Kind::Intermediate ? &state->getIntermediate(objId) : nullptr)
+SpirvShader::Operand::Operand(const SpirvShader *shader, const EmitState *state, SpirvShader::Object::ID objectId)
+ : Operand(state, shader->getObject(objectId))
{}
+SpirvShader::Operand::Operand(const EmitState *state, const Object &object)
+ : constant(object.constantValue.data())
+ , intermediate(object.kind == SpirvShader::Object::Kind::Intermediate ? &state->getIntermediate(object.id()) : nullptr)
+ , componentCount(intermediate ? intermediate->componentCount : object.constantValue.size())
+{
+ ASSERT(intermediate || (object.kind == SpirvShader::Object::Kind::Constant));
+}
+
SpirvRoutine::SpirvRoutine(vk::PipelineLayout const *pipelineLayout)
: pipelineLayout(pipelineLayout)
{
diff --git a/src/Pipeline/SpirvShader.hpp b/src/Pipeline/SpirvShader.hpp
index 38f866e..7fa2a55 100644
--- a/src/Pipeline/SpirvShader.hpp
+++ b/src/Pipeline/SpirvShader.hpp
@@ -72,10 +72,10 @@
{
public:
Intermediate(uint32_t componentCount)
- : scalar(new rr::Value *[componentCount])
- , componentCount(componentCount)
+ : componentCount(componentCount)
+ , scalar(new rr::Value *[componentCount])
{
- memset(scalar, 0, sizeof(rr::Value *) * componentCount);
+ for(auto i = 0u; i < componentCount; i++) { scalar[i] = nullptr; }
}
~Intermediate()
@@ -119,6 +119,8 @@
Intermediate &operator=(Intermediate const &) = delete;
Intermediate &operator=(Intermediate &&) = delete;
+ const uint32_t componentCount;
+
private:
void emplace(uint32_t i, rr::Value *value)
{
@@ -128,7 +130,6 @@
}
rr::Value **const scalar;
- uint32_t componentCount;
};
class SpirvShader
@@ -276,7 +277,7 @@
Object::ID id() const { return definition.resultId(); }
InsnIterator definition;
- std::unique_ptr<uint32_t[]> constantValue = nullptr;
+ std::vector<uint32_t> constantValue;
enum class Kind
{
@@ -989,11 +990,8 @@
// significantly different based on whether the value is uniform across lanes.
class Operand
{
- SpirvShader::Object const &obj;
- Intermediate const *intermediate;
-
public:
- Operand(SpirvShader const *shader, EmitState const *state, SpirvShader::Object::ID objId);
+ Operand(const SpirvShader *shader, const EmitState *state, SpirvShader::Object::ID objectId);
RValue<SIMD::Float> Float(uint32_t i) const
{
@@ -1005,9 +1003,7 @@
// Constructing a constant SIMD::Float is not guaranteed to preserve the data's exact
// bit pattern, but SPIR-V provides 32-bit words representing "the bit pattern for the constant".
// Thus we must first construct an integer constant, and bitcast to float.
- ASSERT(obj.kind == SpirvShader::Object::Kind::Constant);
- auto constantValue = reinterpret_cast<uint32_t *>(obj.constantValue.get());
- return As<SIMD::Float>(SIMD::UInt(constantValue[i]));
+ return As<SIMD::Float>(SIMD::UInt(constant[i]));
}
RValue<SIMD::Int> Int(uint32_t i) const
@@ -1016,9 +1012,8 @@
{
return intermediate->Int(i);
}
- ASSERT(obj.kind == SpirvShader::Object::Kind::Constant);
- auto constantValue = reinterpret_cast<int *>(obj.constantValue.get());
- return SIMD::Int(constantValue[i]);
+
+ return SIMD::Int(constant[i]);
}
RValue<SIMD::UInt> UInt(uint32_t i) const
@@ -1027,15 +1022,19 @@
{
return intermediate->UInt(i);
}
- ASSERT(obj.kind == SpirvShader::Object::Kind::Constant);
- auto constantValue = reinterpret_cast<uint32_t *>(obj.constantValue.get());
- return SIMD::UInt(constantValue[i]);
+
+ return SIMD::UInt(constant[i]);
}
- Type::ID typeId() const
- {
- return obj.typeId();
- }
+ private:
+ // Delegate constructor
+ Operand(const EmitState *state, const Object &object);
+
+ const uint32_t *constant;
+ const Intermediate *intermediate;
+
+ public:
+ const uint32_t componentCount;
};
Type const &getType(Type::ID id) const
@@ -1050,11 +1049,6 @@
return getType(object.typeId());
}
- Type const &getType(const Operand &operand) const
- {
- return getType(operand.typeId());
- }
-
Object const &getObject(Object::ID id) const
{
auto it = defs.find(id);
diff --git a/src/Pipeline/SpirvShaderArithmetic.cpp b/src/Pipeline/SpirvShaderArithmetic.cpp
index 6ea7611..59c499b 100644
--- a/src/Pipeline/SpirvShaderArithmetic.cpp
+++ b/src/Pipeline/SpirvShaderArithmetic.cpp
@@ -41,12 +41,11 @@
auto &dst = state->createIntermediate(insn.resultId(), type.componentCount);
auto lhs = Operand(this, state, insn.word(3));
auto rhs = Operand(this, state, insn.word(4));
- auto rhsType = getType(rhs);
for(auto i = 0u; i < type.componentCount; i++)
{
SIMD::Float v = lhs.Float(i) * rhs.Float(0);
- for(auto j = 1u; j < rhsType.componentCount; j++)
+ for(auto j = 1u; j < rhs.componentCount; j++)
{
v += lhs.Float(i + type.componentCount * j) * rhs.Float(j);
}
@@ -62,14 +61,13 @@
auto &dst = state->createIntermediate(insn.resultId(), type.componentCount);
auto lhs = Operand(this, state, insn.word(3));
auto rhs = Operand(this, state, insn.word(4));
- auto lhsType = getType(lhs);
for(auto i = 0u; i < type.componentCount; i++)
{
- SIMD::Float v = lhs.Float(0) * rhs.Float(i * lhsType.componentCount);
- for(auto j = 1u; j < lhsType.componentCount; j++)
+ SIMD::Float v = lhs.Float(0) * rhs.Float(i * lhs.componentCount);
+ for(auto j = 1u; j < lhs.componentCount; j++)
{
- v += lhs.Float(j) * rhs.Float(i * lhsType.componentCount + j);
+ v += lhs.Float(j) * rhs.Float(i * lhs.componentCount + j);
}
dst.move(i, v);
}
@@ -110,17 +108,9 @@
auto &dst = state->createIntermediate(insn.resultId(), type.componentCount);
auto lhs = Operand(this, state, insn.word(3));
auto rhs = Operand(this, state, insn.word(4));
- auto &lhsType = getType(lhs);
- auto &rhsType = getType(rhs);
- ASSERT(type.definition.opcode() == spv::OpTypeMatrix);
- ASSERT(lhsType.definition.opcode() == spv::OpTypeVector);
- ASSERT(rhsType.definition.opcode() == spv::OpTypeVector);
- ASSERT(getType(lhsType.element).opcode() == spv::OpTypeFloat);
- ASSERT(getType(rhsType.element).opcode() == spv::OpTypeFloat);
-
- auto numRows = lhsType.definition.word(3);
- auto numCols = rhsType.definition.word(3);
+ auto numRows = lhs.componentCount;
+ auto numCols = rhs.componentCount;
for(auto col = 0u; col < numCols; col++)
{
diff --git a/src/Pipeline/SpirvShaderControlFlow.cpp b/src/Pipeline/SpirvShaderControlFlow.cpp
index dc140fe..80cf455 100644
--- a/src/Pipeline/SpirvShaderControlFlow.cpp
+++ b/src/Pipeline/SpirvShaderControlFlow.cpp
@@ -496,7 +496,7 @@
auto falseBlockId = Block::ID(block.branchInstruction.word(3));
auto cond = Operand(this, state, condId);
- ASSERT_MSG(getType(cond).componentCount == 1, "Condition must be a Boolean type scalar");
+ ASSERT_MSG(getType(getObject(condId)).componentCount == 1, "Condition must be a Boolean type scalar");
// TODO: Optimize for case where all lanes take same path.
@@ -515,7 +515,7 @@
auto selId = Object::ID(block.branchInstruction.word(1));
auto sel = Operand(this, state, selId);
- ASSERT_MSG(getType(sel).componentCount == 1, "Selector must be a scalar");
+ ASSERT_MSG(sel.componentCount == 1, "Selector must be a scalar");
auto numCases = (block.branchInstruction.wordCount() - 3) / 2;
diff --git a/src/Pipeline/SpirvShaderDebugger.cpp b/src/Pipeline/SpirvShaderDebugger.cpp
index 0a555d9..be57e54 100644
--- a/src/Pipeline/SpirvShaderDebugger.cpp
+++ b/src/Pipeline/SpirvShaderDebugger.cpp
@@ -1153,11 +1153,12 @@
EmitState *state,
int wordOffset /* = 0 */) const
{
+ auto &obj = shader->getObject(id);
+
if(type != nullptr)
{
if(auto ty = debug::cast<debug::BasicType>(type))
{
- auto &obj = shader->getObject(id);
SIMD::Int val;
switch(obj.kind)
{
@@ -1288,7 +1289,7 @@
// No debug type information. Derive from SPIR-V.
Operand val(shader, state, id);
- switch(shader->getType(val).opcode())
+ switch(shader->getType(obj).opcode())
{
case spv::OpTypeInt:
{
@@ -1302,7 +1303,7 @@
break;
case spv::OpTypeVector:
{
- auto count = shader->getType(val).definition.word(3);
+ auto count = shader->getType(obj).definition.word(3);
switch(count)
{
case 1:
diff --git a/src/Pipeline/SpirvShaderGLSLstd450.cpp b/src/Pipeline/SpirvShaderGLSLstd450.cpp
index 554cbe4..5b8d953 100644
--- a/src/Pipeline/SpirvShaderGLSLstd450.cpp
+++ b/src/Pipeline/SpirvShaderGLSLstd450.cpp
@@ -338,12 +338,11 @@
{
auto p0 = Operand(this, state, insn.word(5));
auto p1 = Operand(this, state, insn.word(6));
- auto p0Type = getType(p0);
// sqrt(dot(p0-p1, p0-p1))
SIMD::Float d = (p0.Float(0) - p1.Float(0)) * (p0.Float(0) - p1.Float(0));
- for(auto i = 1u; i < p0Type.componentCount; i++)
+ for(auto i = 1u; i < p0.componentCount; i++)
{
d += (p0.Float(i) - p1.Float(i)) * (p0.Float(i) - p1.Float(i));
}
@@ -378,14 +377,13 @@
case GLSLstd450ModfStruct:
{
auto val = Operand(this, state, insn.word(5));
- auto valTy = getType(val);
- for(auto i = 0u; i < valTy.componentCount; i++)
+ for(auto i = 0u; i < val.componentCount; i++)
{
SIMD::Float whole, frac;
std::tie(whole, frac) = Modf(val.Float(i));
dst.move(i, frac);
- dst.move(i + valTy.componentCount, whole);
+ dst.move(i + val.componentCount, whole);
}
break;
}
@@ -527,12 +525,12 @@
case GLSLstd450FrexpStruct:
{
auto val = Operand(this, state, insn.word(5));
- auto numComponents = getType(val).componentCount;
- for(auto i = 0u; i < numComponents; i++)
+
+ for(auto i = 0u; i < val.componentCount; i++)
{
auto significandAndExponent = Frexp(val.Float(i));
dst.move(i, significandAndExponent.first);
- dst.move(i + numComponents, significandAndExponent.second);
+ dst.move(val.componentCount + i, significandAndExponent.second);
}
break;
}
@@ -785,8 +783,8 @@
case GLSLstd450Determinant:
{
auto mat = Operand(this, state, insn.word(5));
- auto numComponents = getType(mat).componentCount;
- switch(numComponents)
+
+ switch(mat.componentCount)
{
case 4: // 2x2
dst.move(0, Determinant(
@@ -807,15 +805,15 @@
mat.Float(12), mat.Float(13), mat.Float(14), mat.Float(15)));
break;
default:
- UNREACHABLE("GLSLstd450Determinant can only operate with square matrices. Got %d elements", int(numComponents));
+ UNREACHABLE("GLSLstd450Determinant can only operate with square matrices. Got %d elements", int(mat.componentCount));
}
break;
}
case GLSLstd450MatrixInverse:
{
auto mat = Operand(this, state, insn.word(5));
- auto numComponents = getType(mat).componentCount;
- switch(numComponents)
+
+ switch(mat.componentCount)
{
case 4: // 2x2
{
@@ -854,7 +852,7 @@
break;
}
default:
- UNREACHABLE("GLSLstd450MatrixInverse can only operate with square matrices. Got %d elements", int(numComponents));
+ UNREACHABLE("GLSLstd450MatrixInverse can only operate with square matrices. Got %d elements", int(mat.componentCount));
}
break;
}
diff --git a/src/Pipeline/SpirvShaderImage.cpp b/src/Pipeline/SpirvShaderImage.cpp
index e6dab75..8197df0 100644
--- a/src/Pipeline/SpirvShaderImage.cpp
+++ b/src/Pipeline/SpirvShaderImage.cpp
@@ -119,7 +119,6 @@
auto samplerDescriptor = (sampledImage.opcode() == spv::OpSampledImage) ? state->getPointer(sampledImage.definition.word(4)).base : imageDescriptor;
auto coordinate = Operand(this, state, coordinateId);
- auto &coordinateType = getType(coordinate);
Pointer<Byte> sampler = samplerDescriptor + OFFSET(vk::SampledImageDescriptor, sampler); // vk::Sampler*
Pointer<Byte> texture = imageDescriptor + OFFSET(vk::SampledImageDescriptor, texture); // sw::Texture*
@@ -204,7 +203,7 @@
Array<SIMD::Float> in(16); // Maximum 16 input parameter components.
- uint32_t coordinates = coordinateType.componentCount - instruction.isProj();
+ uint32_t coordinates = coordinate.componentCount - instruction.isProj();
instruction.coordinates = coordinates;
uint32_t i = 0;
@@ -246,17 +245,16 @@
{
auto dxValue = Operand(this, state, gradDxId);
auto dyValue = Operand(this, state, gradDyId);
- auto &dxyType = getType(dxValue);
- ASSERT(dxyType.componentCount == getType(dyValue).componentCount);
+ ASSERT(dxValue.componentCount == dxValue.componentCount);
- instruction.grad = dxyType.componentCount;
+ instruction.grad = dxValue.componentCount;
- for(uint32_t j = 0; j < dxyType.componentCount; j++, i++)
+ for(uint32_t j = 0; j < dxValue.componentCount; j++, i++)
{
in[i] = dxValue.Float(j);
}
- for(uint32_t j = 0; j < dxyType.componentCount; j++, i++)
+ for(uint32_t j = 0; j < dxValue.componentCount; j++, i++)
{
in[i] = dyValue.Float(j);
}
@@ -273,11 +271,9 @@
if(constOffset)
{
auto offsetValue = Operand(this, state, offsetId);
- auto &offsetType = getType(offsetValue);
+ instruction.offset = offsetValue.componentCount;
- instruction.offset = offsetType.componentCount;
-
- for(uint32_t j = 0; j < offsetType.componentCount; j++, i++)
+ for(uint32_t j = 0; j < offsetValue.componentCount; j++, i++)
{
in[i] = As<SIMD::Float>(offsetValue.Int(j)); // Integer values, but transfered as float.
}
@@ -383,7 +379,7 @@
if(lodId != 0)
{
auto lodVal = Operand(this, state, lodId);
- ASSERT(getType(lodVal).componentCount == 1);
+ ASSERT(lodVal.componentCount == 1);
auto lod = lodVal.Int(0);
auto one = SIMD::Int(1);
for(uint32_t i = 0; i < dimensions; i++)
@@ -476,12 +472,12 @@
auto routine = state->routine;
bool isArrayed = imageType.definition.word(5) != 0;
auto dim = static_cast<spv::Dim>(imageType.definition.word(3));
- int dims = getType(coordinate).componentCount - (isArrayed ? 1 : 0);
+ int dims = coordinate.componentCount - (isArrayed ? 1 : 0);
SIMD::Int u = coordinate.Int(0);
SIMD::Int v = SIMD::Int(0);
- if(getType(coordinate).componentCount > 1)
+ if(coordinate.componentCount > 1)
{
v = coordinate.Int(1);
}
diff --git a/src/Pipeline/SpirvShaderMemory.cpp b/src/Pipeline/SpirvShaderMemory.cpp
index b85200f..b37f163 100644
--- a/src/Pipeline/SpirvShaderMemory.cpp
+++ b/src/Pipeline/SpirvShaderMemory.cpp
@@ -99,7 +99,7 @@
if(object.kind == Object::Kind::Constant)
{
// Constant source data.
- const uint32_t *src = object.constantValue.get();
+ const uint32_t *src = object.constantValue.data();
VisitMemoryObject(pointerId, [&](const MemoryElement &el) {
auto p = ptr + el.offset;
if(interleavedByLane) { p = InterleaveByLane(p); }
