From 2f0be387c450153242081722003b0e14dd09c944 Mon Sep 17 00:00:00 2001 From: baldurk Date: Tue, 23 Feb 2021 11:35:16 +0000 Subject: [PATCH] Use DebugSource and DebugLine/DebugNoLine for line-number info --- .../driver/shaders/spirv/spirv_debug.cpp | 2 +- renderdoc/driver/shaders/spirv/spirv_debug.h | 2 +- .../shaders/spirv/spirv_debug_setup.cpp | 63 +++++++++++++------ .../driver/shaders/spirv/spirv_reflect.cpp | 13 ++-- .../driver/shaders/spirv/spirv_reflect.h | 1 + 5 files changed, 57 insertions(+), 24 deletions(-) diff --git a/renderdoc/driver/shaders/spirv/spirv_debug.cpp b/renderdoc/driver/shaders/spirv/spirv_debug.cpp index 6c5b1d658..a0a7b3a72 100644 --- a/renderdoc/driver/shaders/spirv/spirv_debug.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_debug.cpp @@ -3779,7 +3779,7 @@ void ThreadState::StepNext(ShaderDebugState *state, const rdcarray } // skip over any degenerate branches - while(true) + while(!debugger.HasDebugInfo()) { it = debugger.GetIterForInstruction(nextInstruction); if(it.opcode() == Op::Branch) diff --git a/renderdoc/driver/shaders/spirv/spirv_debug.h b/renderdoc/driver/shaders/spirv/spirv_debug.h index 5f2707694..83c7e7e6f 100644 --- a/renderdoc/driver/shaders/spirv/spirv_debug.h +++ b/renderdoc/driver/shaders/spirv/spirv_debug.h @@ -401,7 +401,7 @@ private: SparseIdMap locals; - std::map files; + SparseIdMap sources; std::map lineScope; std::map localMappings; diff --git a/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp b/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp index dad7e85ee..23929d893 100644 --- a/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp @@ -24,10 +24,14 @@ #include "spirv_debug.h" #include "common/formatting.h" +#include "core/settings.h" #include "spirv_op_helpers.h" #include "spirv_reflect.h" #include "var_dispatch_helpers.h" +RDOC_CONFIG(bool, Vulkan_Debug_UseDebugColumnInformation, false, + "Control whether column information should be read from vulkan debug info."); + // this could be cleaner if ShaderVariable wasn't a very public struct, but it's not worth it so // we just reserve value slots that we know won't be used in opaque variables static const uint32_t PointerVariableSlot = 0; @@ -2615,15 +2619,16 @@ void Debugger::RegisterOp(Iter it) // the types are identical just with different accessors OpShaderDbg &dbg = (OpShaderDbg &)extinst; - if(dbg.inst == ShaderDbg::CompilationUnit) + if(dbg.inst == ShaderDbg::Source) { - OpShaderDbg src(GetID(dbg.arg(2))); + int32_t fileIndex = (int32_t)m_DebugInfo.sources.size(); - int32_t fileIndex = (int32_t)m_DebugInfo.files.size(); - - m_DebugInfo.files[strings[src.arg(0)]] = fileIndex; - - m_DebugInfo.scopes[dbg.result] = {DebugScope::CompilationUnit, NULL, 1, 1, fileIndex, 0}; + m_DebugInfo.sources[dbg.result] = fileIndex; + } + else if(dbg.inst == ShaderDbg::CompilationUnit) + { + m_DebugInfo.scopes[dbg.result] = {DebugScope::CompilationUnit, NULL, 1, 1, + m_DebugInfo.sources[dbg.arg(2)], 0}; } else if(dbg.inst == ShaderDbg::LexicalBlock) { @@ -2705,6 +2710,25 @@ void Debugger::RegisterOp(Iter it) { // TODO inline information } + else if(dbg.inst == ShaderDbg::Line) + { + OpShaderDbg line(it); + + m_CurLineCol.lineStart = EvaluateConstant(dbg.arg(1), {}).value.u32v[0]; + m_CurLineCol.lineEnd = EvaluateConstant(dbg.arg(2), {}).value.u32v[0]; + if(Vulkan_Debug_UseDebugColumnInformation()) + { + m_CurLineCol.colStart = EvaluateConstant(dbg.arg(3), {}).value.u32v[0]; + m_CurLineCol.colEnd = EvaluateConstant(dbg.arg(4), {}).value.u32v[0]; + } + + // find file index by filename matching, this would be nice to improve as it's brittle + m_CurLineCol.fileIndex = m_DebugInfo.sources[dbg.arg(0)]; + } + else if(dbg.inst == ShaderDbg::NoLine) + { + m_CurLineCol = LineColumnInfo(); + } } } else if(opdata.op == Op::ExtInstImport) @@ -2732,15 +2756,7 @@ void Debugger::RegisterOp(Iter it) if(m_DebugInfo.valid) { - m_CurLineCol.lineStart = line.line; - m_CurLineCol.lineEnd = line.line; - m_CurLineCol.colStart = line.column; - // find file index by filename matching, this would be nice to improve as it's brittle - auto file = m_DebugInfo.files.find(strings[line.file]); - if(file != m_DebugInfo.files.end()) - m_CurLineCol.fileIndex = file->second; - else - m_CurLineCol.fileIndex = -1; + // ignore any OpLine when we have proper debug info } else { @@ -2752,11 +2768,22 @@ void Debugger::RegisterOp(Iter it) } else if(opdata.op == Op::NoLine) { - m_CurLineCol = LineColumnInfo(); + if(!m_DebugInfo.valid) + m_CurLineCol = LineColumnInfo(); } else { - m_LineColInfo[it.offs()] = m_CurLineCol; + // for debug info, only apply line info if we're in a scope. Otherwise the line info may not + // apply to this instruction. This means OpPhi's will never be line mapped + if(m_DebugInfo.valid) + { + if(m_DebugInfo.curScope) + m_LineColInfo[it.offs()] = m_CurLineCol; + } + else + { + m_LineColInfo[it.offs()] = m_CurLineCol; + } } if(m_DebugInfo.valid) diff --git a/renderdoc/driver/shaders/spirv/spirv_reflect.cpp b/renderdoc/driver/shaders/spirv/spirv_reflect.cpp index a87b397cd..69a499bac 100644 --- a/renderdoc/driver/shaders/spirv/spirv_reflect.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_reflect.cpp @@ -454,14 +454,19 @@ void Reflector::RegisterOp(Iter it) // and potential names of global variables that might be missing. if(dbg.set == knownExtSet[ExtSet_ShaderDbg]) { - if(dbg.inst == ShaderDbg::CompilationUnit) + if(dbg.inst == ShaderDbg::Source) { - OpShaderDbg src(GetID(dbg.arg(2))); + debugSources[dbg.result] = sources.size(); sources.push_back({ - (SourceLanguage)EvaluateConstant(dbg.arg(3), {}).value.u32v[0], - strings[src.arg(0)], src.params.size() > 1 ? strings[src.arg(1)] : rdcstr(), + SourceLanguage::Unknown, strings[dbg.arg(0)], + dbg.params.size() > 1 ? strings[dbg.arg(1)] : rdcstr(), }); } + else if(dbg.inst == ShaderDbg::CompilationUnit) + { + sources[debugSources[dbg.arg(2)]].lang = + (SourceLanguage)EvaluateConstant(dbg.arg(3), {}).value.u32v[0]; + } else if(dbg.inst == ShaderDbg::GlobalVariable) { // copy the name string to the variable string only if it's empty. If it has a name already, diff --git a/renderdoc/driver/shaders/spirv/spirv_reflect.h b/renderdoc/driver/shaders/spirv/spirv_reflect.h index af1064f15..7e15f0799 100644 --- a/renderdoc/driver/shaders/spirv/spirv_reflect.h +++ b/renderdoc/driver/shaders/spirv/spirv_reflect.h @@ -125,6 +125,7 @@ private: rdcstr cmdline; DenseIdMap strings; rdcarray sources; + SparseIdMap debugSources; Id curBlock; std::set loopBlocks;