Use DebugSource and DebugLine/DebugNoLine for line-number info

renderdoc/driver/shaders/spirv/spirv_debug.cpp

@@ -3779,7 +3779,7 @@ void ThreadState::StepNext(ShaderDebugState *state, const rdcarray<ThreadState>
   }
 
   // skip over any degenerate branches
-  while(true)
+  while(!debugger.HasDebugInfo())
   {
     it = debugger.GetIterForInstruction(nextInstruction);
     if(it.opcode() == Op::Branch)

renderdoc/driver/shaders/spirv/spirv_debug.h

@@ -401,7 +401,7 @@ private:
 
     SparseIdMap<LocalData> locals;
 
-    std::map<rdcstr, int32_t> files;
+    SparseIdMap<int32_t> sources;
 
     std::map<size_t, ScopeData *> lineScope;
     std::map<size_t, LocalMapping> localMappings;

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<Id>(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<Id>(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<Id>(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<Id>(1), {}).value.u32v[0];
+        m_CurLineCol.lineEnd = EvaluateConstant(dbg.arg<Id>(2), {}).value.u32v[0];
+        if(Vulkan_Debug_UseDebugColumnInformation())
+        {
+          m_CurLineCol.colStart = EvaluateConstant(dbg.arg<Id>(3), {}).value.u32v[0];
+          m_CurLineCol.colEnd = EvaluateConstant(dbg.arg<Id>(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<Id>(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)

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<Id>(2)));
+        debugSources[dbg.result] = sources.size();
         sources.push_back({
-            (SourceLanguage)EvaluateConstant(dbg.arg<Id>(3), {}).value.u32v[0],
-            strings[src.arg<Id>(0)], src.params.size() > 1 ? strings[src.arg<Id>(1)] : rdcstr(),
+            SourceLanguage::Unknown, strings[dbg.arg<Id>(0)],
+            dbg.params.size() > 1 ? strings[dbg.arg<Id>(1)] : rdcstr(),
         });
       }
+      else if(dbg.inst == ShaderDbg::CompilationUnit)
+      {
+        sources[debugSources[dbg.arg<Id>(2)]].lang =
+            (SourceLanguage)EvaluateConstant(dbg.arg<Id>(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,

renderdoc/driver/shaders/spirv/spirv_reflect.h

@@ -125,6 +125,7 @@ private:
   rdcstr cmdline;
   DenseIdMap<rdcstr> strings;
   rdcarray<SourceFile> sources;
+  SparseIdMap<size_t> debugSources;
 
   Id curBlock;
   std::set<Id> loopBlocks;