Commit 6fae32cb by Ben Clayton

SpirvShader: Add debug checks on Intermediate.

Memset the scalars to 0, and check their pointers before use. Catches cases where the intermediate was declared but not set. Also switch from using assert() to ASSERT() as the latter still fires with DCHECK_ALWAYS_ON. Change-Id: I81abb50aea41568f95c22a340b90b7c56839d3ce Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/25869Reviewed-by: 's avatarNicolas Capens <nicolascapens@google.com> Reviewed-by: 's avatarChris Forbes <chrisforbes@google.com> Kokoro-Presubmit: kokoro <noreply+kokoro@google.com> Tested-by: 's avatarBen Clayton <bclayton@google.com>
parent 76bc8f2a
...@@ -335,22 +335,22 @@ namespace sw ...@@ -335,22 +335,22 @@ namespace sw
void SpirvShader::ProcessInterfaceVariable(Object &object) void SpirvShader::ProcessInterfaceVariable(Object &object)
{ {
auto &objectTy = getType(object.type); auto &objectTy = getType(object.type);
assert(objectTy.storageClass == spv::StorageClassInput || objectTy.storageClass == spv::StorageClassOutput); ASSERT(objectTy.storageClass == spv::StorageClassInput || objectTy.storageClass == spv::StorageClassOutput);
assert(objectTy.definition.opcode() == spv::OpTypePointer); ASSERT(objectTy.definition.opcode() == spv::OpTypePointer);
auto pointeeTy = getType(objectTy.element); auto pointeeTy = getType(objectTy.element);
auto &builtinInterface = (objectTy.storageClass == spv::StorageClassInput) ? inputBuiltins : outputBuiltins; auto &builtinInterface = (objectTy.storageClass == spv::StorageClassInput) ? inputBuiltins : outputBuiltins;
auto &userDefinedInterface = (objectTy.storageClass == spv::StorageClassInput) ? inputs : outputs; auto &userDefinedInterface = (objectTy.storageClass == spv::StorageClassInput) ? inputs : outputs;
assert(object.definition.opcode() == spv::OpVariable); ASSERT(object.definition.opcode() == spv::OpVariable);
ObjectID resultId = object.definition.word(2); ObjectID resultId = object.definition.word(2);
if (objectTy.isBuiltInBlock) if (objectTy.isBuiltInBlock)
{ {
// walk the builtin block, registering each of its members separately. // walk the builtin block, registering each of its members separately.
auto m = memberDecorations.find(objectTy.element); auto m = memberDecorations.find(objectTy.element);
assert(m != memberDecorations.end()); // otherwise we wouldn't have marked the type chain ASSERT(m != memberDecorations.end()); // otherwise we wouldn't have marked the type chain
auto &structType = pointeeTy.definition; auto &structType = pointeeTy.definition;
auto offset = 0u; auto offset = 0u;
auto word = 2u; auto word = 2u;
...@@ -381,7 +381,7 @@ namespace sw ...@@ -381,7 +381,7 @@ namespace sw
[&userDefinedInterface](Decorations const &d, AttribType type) { [&userDefinedInterface](Decorations const &d, AttribType type) {
// Populate a single scalar slot in the interface from a collection of decorations and the intended component type. // Populate a single scalar slot in the interface from a collection of decorations and the intended component type.
auto scalarSlot = (d.Location << 2) | d.Component; auto scalarSlot = (d.Location << 2) | d.Component;
assert(scalarSlot >= 0 && ASSERT(scalarSlot >= 0 &&
scalarSlot < static_cast<int32_t>(userDefinedInterface.size())); scalarSlot < static_cast<int32_t>(userDefinedInterface.size()));
auto &slot = userDefinedInterface[scalarSlot]; auto &slot = userDefinedInterface[scalarSlot];
...@@ -564,7 +564,7 @@ namespace sw ...@@ -564,7 +564,7 @@ namespace sw
ApplyDecorationsForId(&d, id); ApplyDecorationsForId(&d, id);
auto def = getObject(id).definition; auto def = getObject(id).definition;
assert(def.opcode() == spv::OpVariable); ASSERT(def.opcode() == spv::OpVariable);
VisitInterfaceInner<F>(def.word(1), d, f); VisitInterfaceInner<F>(def.word(1), d, f);
} }
...@@ -774,8 +774,8 @@ namespace sw ...@@ -774,8 +774,8 @@ namespace sw
// TODO: not encountered yet since we only use this for array sizes etc, // TODO: not encountered yet since we only use this for array sizes etc,
// but is possible to construct integer constant 0 via OpConstantNull. // but is possible to construct integer constant 0 via OpConstantNull.
auto insn = getObject(id).definition; auto insn = getObject(id).definition;
assert(insn.opcode() == spv::OpConstant); ASSERT(insn.opcode() == spv::OpConstant);
assert(getType(insn.word(1)).definition.opcode() == spv::OpTypeInt); ASSERT(getType(insn.word(1)).definition.opcode() == spv::OpTypeInt);
return insn.word(3); return insn.word(3);
} }
......
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
#include "ShaderCore.hpp" #include "ShaderCore.hpp"
#include "SpirvID.hpp" #include "SpirvID.hpp"
#include <cstring>
#include <string> #include <string>
#include <vector> #include <vector>
#include <unordered_map> #include <unordered_map>
...@@ -58,7 +59,11 @@ namespace sw ...@@ -58,7 +59,11 @@ namespace sw
public: public:
using Scalar = RValue<SIMD::Float>; using Scalar = RValue<SIMD::Float>;
Intermediate(uint32_t size) : contents(new ContentsType[size]), size(size) {} Intermediate(uint32_t size) : contents(new ContentsType[size]), size(size) {
#if !defined(NDEBUG) || defined(DCHECK_ALWAYS_ON)
memset(contents, 0, sizeof(ContentsType[size]));
#endif
}
~Intermediate() ~Intermediate()
{ {
...@@ -69,20 +74,24 @@ namespace sw ...@@ -69,20 +74,24 @@ namespace sw
void emplace(uint32_t n, Scalar&& value) void emplace(uint32_t n, Scalar&& value)
{ {
assert(n < size); ASSERT(n < size);
ASSERT(reinterpret_cast<Scalar const *>(&contents[n])->value == nullptr);
new (&contents[n]) Scalar(value); new (&contents[n]) Scalar(value);
} }
void emplace(uint32_t n, const Scalar& value) void emplace(uint32_t n, const Scalar& value)
{ {
assert(n < size); ASSERT(n < size);
ASSERT(reinterpret_cast<Scalar const *>(&contents[n])->value == nullptr);
new (&contents[n]) Scalar(value); new (&contents[n]) Scalar(value);
} }
Scalar const & operator[](uint32_t n) const Scalar const & operator[](uint32_t n) const
{ {
assert(n < size); ASSERT(n < size);
return *reinterpret_cast<Scalar const *>(&contents[n]); auto scalar = reinterpret_cast<Scalar const *>(&contents[n]);
ASSERT(scalar->value != nullptr);
return *scalar;
} }
// No copy/move construction or assignment // No copy/move construction or assignment
...@@ -340,14 +349,14 @@ namespace sw ...@@ -340,14 +349,14 @@ namespace sw
Type const &getType(TypeID id) const Type const &getType(TypeID id) const
{ {
auto it = types.find(id); auto it = types.find(id);
assert(it != types.end()); ASSERT(it != types.end());
return it->second; return it->second;
} }
Object const &getObject(ObjectID id) const Object const &getObject(ObjectID id) const
{ {
auto it = defs.find(id); auto it = defs.find(id);
assert(it != defs.end()); ASSERT(it != defs.end());
return it->second; return it->second;
} }
...@@ -427,14 +436,14 @@ namespace sw ...@@ -427,14 +436,14 @@ namespace sw
Value& getValue(SpirvShader::ObjectID id) Value& getValue(SpirvShader::ObjectID id)
{ {
auto it = lvalues.find(id); auto it = lvalues.find(id);
assert(it != lvalues.end()); ASSERT(it != lvalues.end());
return it->second; return it->second;
} }
Intermediate const& getIntermediate(SpirvShader::ObjectID id) const Intermediate const& getIntermediate(SpirvShader::ObjectID id) const
{ {
auto it = intermediates.find(id); auto it = intermediates.find(id);
assert(it != intermediates.end()); ASSERT(it != intermediates.end());
return it->second; return it->second;
} }
}; };
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment