From c3d53237f74740adf66f788a1d7e7c91cc164afc Mon Sep 17 00:00:00 2001 From: baldurk Date: Thu, 9 Apr 2020 12:22:07 +0100 Subject: [PATCH] Fix handling of OpUndef and improve disassembly of NULL/Undef values --- .../driver/shaders/spirv/spirv_debug.cpp | 4 +- .../shaders/spirv/spirv_disassemble.cpp | 19 +++++++- .../driver/shaders/spirv/spirv_processor.cpp | 48 +++++++++++++------ .../driver/shaders/spirv/spirv_processor.h | 3 +- 4 files changed, 56 insertions(+), 18 deletions(-) diff --git a/renderdoc/driver/shaders/spirv/spirv_debug.cpp b/renderdoc/driver/shaders/spirv/spirv_debug.cpp index 44d33eae5..eaed30053 100644 --- a/renderdoc/driver/shaders/spirv/spirv_debug.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_debug.cpp @@ -1617,9 +1617,11 @@ void ThreadState::StepNext(ShaderDebugState *state, case Op::Undef: { + // this was processed as a constant, since it can appear in the constants section as well as + // in blocks. Just assign the value to itself so that it shows up as a change OpUndef undef(it); - SetDst(state, undef.result, ShaderVariable()); + SetDst(state, undef.result, GetSrc(undef.result)); break; } diff --git a/renderdoc/driver/shaders/spirv/spirv_disassemble.cpp b/renderdoc/driver/shaders/spirv/spirv_disassemble.cpp index c3b5c87d7..0e0de7297 100644 --- a/renderdoc/driver/shaders/spirv/spirv_disassemble.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_disassemble.cpp @@ -376,7 +376,10 @@ rdcstr Reflector::Disassemble(const rdcstr &entryPoint, // scalar and vector constants are inlined when they're unnamed and not specialised, so only // declare others. case Op::ConstantTrue: - case Op::ConstantFalse: continue; + case Op::ConstantFalse: + case Op::ConstantNull: + case Op::Undef: continue; + case Op::SpecConstant: case Op::Constant: { @@ -1533,6 +1536,20 @@ rdcstr Reflector::StringiseConstant(rdcspv::Id id) const if(specConstants.find(id) != specConstants.end()) return rdcstr(); + // print NULL or Undef values specially + if(cit->second.op == Op::ConstantNull || cit->second.op == Op::Undef) + { + rdcstr ret = dataTypes[cit->second.type].name; + if(ret.empty()) + ret = StringFormat::Fmt("type%u", cit->second.type.value()); + + if(cit->second.op == Op::ConstantNull) + ret += "(Null)"; + else if(cit->second.op == Op::Undef) + ret += "(Undef)"; + return ret; + } + const DataType &type = dataTypes[cit->second.type]; const ShaderVariable &value = cit->second.value; diff --git a/renderdoc/driver/shaders/spirv/spirv_processor.cpp b/renderdoc/driver/shaders/spirv/spirv_processor.cpp index 6640ec946..50f7a1f5e 100644 --- a/renderdoc/driver/shaders/spirv/spirv_processor.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_processor.cpp @@ -496,7 +496,21 @@ void Processor::RegisterOp(Iter it) DataType &type = dataTypes[decoded.resultType]; - constants[decoded.result] = {decoded.resultType, decoded.result, MakeNULL(type)}; + ShaderVariable v = MakeNULL(type, 0ULL); + v.name = "NULL"; + + constants[decoded.result] = {decoded.resultType, decoded.result, v, {}, opdata.op}; + } + else if(opdata.op == Op::Undef) + { + OpUndef decoded(it); + + DataType &type = dataTypes[decoded.resultType]; + + ShaderVariable v = MakeNULL(type, 0xccccccccccccccccULL); + v.name = "Undef"; + + constants[decoded.result] = {decoded.resultType, decoded.result, v, {}, opdata.op}; } else if(opdata.op == Op::ConstantTrue || opdata.op == Op::SpecConstantTrue) { @@ -505,7 +519,7 @@ void Processor::RegisterOp(Iter it) ShaderVariable v("true", 1, 0, 0, 0); v.columns = 1; - constants[decoded.result] = {decoded.resultType, decoded.result, v}; + constants[decoded.result] = {decoded.resultType, decoded.result, v, {}, opdata.op}; if(opdata.op == Op::SpecConstantTrue) specConstants.insert(decoded.result); } @@ -516,7 +530,7 @@ void Processor::RegisterOp(Iter it) ShaderVariable v("true", 0, 0, 0, 0); v.columns = 1; - constants[decoded.result] = {decoded.resultType, decoded.result, v}; + constants[decoded.result] = {decoded.resultType, decoded.result, v, {}, opdata.op}; if(opdata.op == Op::SpecConstantFalse) specConstants.insert(decoded.result); } @@ -575,7 +589,8 @@ void Processor::RegisterOp(Iter it) v.members[i] = constants[decoded.constituents[i]].value; } - constants[decoded.result] = {decoded.resultType, decoded.result, v, decoded.constituents}; + constants[decoded.result] = {decoded.resultType, decoded.result, v, decoded.constituents, + opdata.op}; if(opdata.op == Op::SpecConstantComposite) specConstants.insert(decoded.result); } @@ -588,7 +603,7 @@ void Processor::RegisterOp(Iter it) specop.params.push_back(Id::fromWord(it.word(w))); specOps[opdata.result] = specop; - constants[opdata.result] = {opdata.resultType, opdata.result}; + constants[opdata.result] = {opdata.resultType, opdata.result, ShaderVariable(), {}, opdata.op}; specConstants.insert(opdata.result); } else if(opdata.op == Op::Constant || opdata.op == Op::SpecConstant) @@ -624,7 +639,7 @@ void Processor::RegisterOp(Iter it) } } - constants[opdata.result] = {opdata.resultType, opdata.result, v}; + constants[opdata.result] = {opdata.resultType, opdata.result, v, {}, opdata.op}; if(opdata.op == Op::SpecConstant) specConstants.insert(opdata.result); } @@ -787,11 +802,11 @@ void Processor::UnregisterOp(Iter it) globals.removeOneIf([result](const Variable &e) { return e.id == result; }); } - else if(opdata.op == Op::ConstantNull || opdata.op == Op::ConstantTrue || - opdata.op == Op::ConstantFalse || opdata.op == Op::ConstantComposite || - opdata.op == Op::Constant || opdata.op == Op::SpecConstantTrue || - opdata.op == Op::SpecConstantFalse || opdata.op == Op::SpecConstantComposite || - opdata.op == Op::SpecConstant) + else if(opdata.op == Op::ConstantNull || opdata.op == Op::Undef || + opdata.op == Op::ConstantTrue || opdata.op == Op::ConstantFalse || + opdata.op == Op::ConstantComposite || opdata.op == Op::Constant || + opdata.op == Op::SpecConstantTrue || opdata.op == Op::SpecConstantFalse || + opdata.op == Op::SpecConstantComposite || opdata.op == Op::SpecConstant) { constants.erase(opdata.result); specConstants.erase(opdata.result); @@ -878,12 +893,15 @@ void Processor::PostParse() m_MemberDecorations.clear(); } -ShaderVariable Processor::MakeNULL(const DataType &type) +ShaderVariable Processor::MakeNULL(const DataType &type, uint64_t value) { - ShaderVariable v("NULL", 0, 0, 0, 0); + ShaderVariable v("", 0, 0, 0, 0); v.rows = v.columns = 0; v.isStruct = (type.type == DataType::StructType); + for(uint8_t c = 0; c < 16; c++) + v.value.u64v[c] = value; + if(type.type == DataType::VectorType) { v.type = type.scalar().Type(); @@ -916,7 +934,7 @@ ShaderVariable Processor::MakeNULL(const DataType &type) v.members.resize(EvaluateConstant(type.length, {}).value.u.x); for(size_t i = 0; i < v.members.size(); i++) { - v.members[i] = MakeNULL(dataTypes[type.InnerType()]); + v.members[i] = MakeNULL(dataTypes[type.InnerType()], value); v.members[i].name = StringFormat::Fmt("[%zu]", i); } } @@ -925,7 +943,7 @@ ShaderVariable Processor::MakeNULL(const DataType &type) v.members.resize(type.children.size()); for(size_t i = 0; i < v.members.size(); i++) { - v.members[i] = MakeNULL(dataTypes[type.children[i].type]); + v.members[i] = MakeNULL(dataTypes[type.children[i].type], value); v.members[i].name = StringFormat::Fmt("_child%zu", i); } } diff --git a/renderdoc/driver/shaders/spirv/spirv_processor.h b/renderdoc/driver/shaders/spirv/spirv_processor.h index 6188e614e..d4d8eec76 100644 --- a/renderdoc/driver/shaders/spirv/spirv_processor.h +++ b/renderdoc/driver/shaders/spirv/spirv_processor.h @@ -457,6 +457,7 @@ struct Constant Id id; ShaderVariable value; rdcarray children; + Op op; }; struct SpecOp @@ -520,7 +521,7 @@ protected: // after parsing - e.g. to do any deferred post-processing virtual void PostParse(); - ShaderVariable MakeNULL(const DataType &type); + ShaderVariable MakeNULL(const DataType &type, uint64_t value); ShaderVariable EvaluateConstant(Id constID, const rdcarray &specInfo) const;