diff --git a/renderdoc/driver/shaders/spirv/spirv_debug.h b/renderdoc/driver/shaders/spirv/spirv_debug.h index 331d370c8..372109a94 100644 --- a/renderdoc/driver/shaders/spirv/spirv_debug.h +++ b/renderdoc/driver/shaders/spirv/spirv_debug.h @@ -313,6 +313,20 @@ struct ScopeData rdcarray locals; rdcarray localMappings; + + bool HasAncestor(const ScopeData *check) const + { + const ScopeData *cur = this; + + while(cur) + { + if(cur == check) + return true; + cur = cur->parent; + } + + return false; + } }; struct InlineData @@ -462,7 +476,7 @@ private: ScopeData *curScope = NULL; InlineData *curInline = NULL; - rdcarray scopelessMappings; + rdcarray> pendingMappings; rdcarray globals; rdcarray constants; diff --git a/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp b/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp index b5100820c..644d1b00d 100644 --- a/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp @@ -1473,20 +1473,33 @@ void Debugger::FillDebugSourceVars(rdcarray &instInfo) // start and end points for each mapping and update the end points, but this is simple and // should be limited since it's only per-scope bool supercede = false; - for(size_t n = m + 1; n < scope->localMappings.size(); n++) + const ScopeData *supercedeScope = scope; + while(supercedeScope) { - const LocalMapping &laterMapping = scope->localMappings[n]; - - // if this mapping is past the current instruction, stop here. - if(laterMapping.instIndex > i.instruction) - break; - - // if this mapping will supercede - if(laterMapping.isSourceSupersetOf(mapping)) + for(size_t n = m + 1; n < supercedeScope->localMappings.size(); n++) { - supercede = true; - break; + const LocalMapping &laterMapping = supercedeScope->localMappings[n]; + + // if this mapping is past the current instruction, stop here. + if(laterMapping.instIndex > i.instruction) + break; + + // if this mapping will supercede and starts later + if(laterMapping.isSourceSupersetOf(mapping) && laterMapping.instIndex > mapping.instIndex) + { + supercede = true; + break; + } } + + if(supercede) + break; + + // if we reach a function scope, don't go up any further. + if(supercedeScope->type == DebugScope::Function) + break; + + supercedeScope = supercedeScope->parent; } for(size_t n = 0; n < processed.size(); n++) @@ -3305,8 +3318,21 @@ void Debugger::RegisterOp(Iter it) m_DebugInfo.curScope = &m_DebugInfo.scopes[dbg.arg(0)]; - m_DebugInfo.curScope->localMappings.append(std::move(m_DebugInfo.scopelessMappings)); - m_DebugInfo.scopelessMappings.clear(); + // pick up any pending mappings for this function if we just entered into a new function. + // See the comment below in Value for this workaround + for(size_t i = 0; i < m_DebugInfo.pendingMappings.size(); i++) + { + rdcpair &cur = m_DebugInfo.pendingMappings[i]; + if(m_DebugInfo.curScope->HasAncestor(cur.first)) + { + m_DebugInfo.curScope->localMappings.push_back(std::move(cur.second)); + + // the array isn't sorted so we can just swap the last one into this spot to avoid + // moving everything + std::swap(cur, m_DebugInfo.pendingMappings.back()); + m_DebugInfo.pendingMappings.pop_back(); + } + } if(dbg.params.size() >= 2) m_DebugInfo.curInline = &m_DebugInfo.inlined[dbg.arg(1)]; @@ -3354,12 +3380,16 @@ void Debugger::RegisterOp(Iter it) Id sourceVarId = dbg.arg(0); Id debugVarId = dbg.arg(1); - rdcarray &mappings = m_DebugInfo.curScope - ? m_DebugInfo.curScope->localMappings - : m_DebugInfo.scopelessMappings; + // check the function this variable is scoped inside of at declaration time. + const ScopeData *varDeclScope = m_DebugInfo.locals[sourceVarId].scope; - mappings.push_back({curInstIndex, sourceVarId, debugVarId, dbg.inst == ShaderDbg::Declare}); - LocalMapping &mapping = mappings.back(); + // bit of a hack - only process declares/values for variables inside a scope that is + // within that function. If we see a declare/value in another function we defer it hoping + // that we will encounter a scope later that's valid for it. + const bool insideValidScope = m_DebugInfo.curScope->HasAncestor(varDeclScope); + + LocalMapping mapping = {curInstIndex, sourceVarId, debugVarId, + dbg.inst == ShaderDbg::Declare}; if(constants.find(debugVarId) != constants.end() && !m_DebugInfo.constants.contains(debugVarId)) @@ -3388,6 +3418,22 @@ void Debugger::RegisterOp(Iter it) } } + if(insideValidScope) + { + m_DebugInfo.curScope->localMappings.push_back(mapping); + } + else + { + // remove any pending mapping that already exists for this variable, without any way to + // meaningfully know which one to use when we pick the latter. + m_DebugInfo.pendingMappings.removeIf( + [&mapping](const rdcpair &m) { + return mapping.isSourceSupersetOf(m.second); + }); + + m_DebugInfo.pendingMappings.push_back({varDeclScope, mapping}); + } + break; } case ShaderDbg::InlinedAt: