spirv-opt: Add specific handling of vulkan debug info differences (#4398

) Co-authored-by: baldurk <baldurk@baldurk.org>
KhronosGroup · Jul 29, 2021 · 983ee23 · 983ee23
1 parent 9c44814
commit 983ee23
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 5 deletions.
diff --git a/source/operand.cpp b/source/operand.cpp
@@ -578,6 +578,12 @@ std::function<bool(unsigned)> spvOperandCanBeForwardDeclaredFunction(
 
 std::function<bool(unsigned)> spvDbgInfoExtOperandCanBeForwardDeclaredFunction(
     spv_ext_inst_type_t ext_type, uint32_t key) {
+  // The Vulkan debug info extended instruction set is non-semantic so allows no
+  // forward references ever.
+  if (ext_type == SPV_EXT_INST_TYPE_NONSEMANTIC_VULKAN_DEBUGINFO_100) {
+    return [](unsigned) { return false; };
+  }
+
   // TODO(https://gitlab.khronos.org/spirv/SPIR-V/issues/532): Forward
   // references for debug info instructions are still in discussion. We must
   // update the following lines of code when we conclude the spec.

diff --git a/source/opt/inline_pass.cpp b/source/opt/inline_pass.cpp
@@ -403,6 +403,14 @@ bool InlinePass::InlineEntryBlock(
       callee2caller, inlined_at_ctx, new_blk_ptr, callee_first_block);
 
   while (callee_inst_itr != callee_first_block->end()) {
+    // Don't inline function definition links, the calling function is not a
+    // definition.
+    if (callee_inst_itr->GetVulkan100DebugOpcode() ==
+        NonSemanticVulkanDebugInfo100DebugFunctionDefinition) {
+      ++callee_inst_itr;
+      continue;
+    }
+
     if (!InlineSingleInstruction(
             callee2caller, new_blk_ptr->get(), &*callee_inst_itr,
             context()->get_debug_info_mgr()->BuildDebugInlinedAtChain(

diff --git a/source/opt/ir_loader.cpp b/source/opt/ir_loader.cpp
@@ -17,6 +17,7 @@
 #include <utility>
 
 #include "DebugInfo.h"
+#include "NonSemanticVulkanDebugInfo100.h"
 #include "OpenCLDebugInfo100.h"
 #include "source/ext_inst.h"
 #include "source/opt/log.h"
@@ -232,6 +233,35 @@ bool IrLoader::AddInstruction(const spv_parsed_instruction_t* inst) {
                 block_->AddInstruction(std::move(spv_inst));
               break;
             }
+            default: {
+              Errorf(consumer_, src, loc,
+                     "Debug info extension instruction other than DebugScope, "
+                     "DebugNoScope, DebugFunctionDefinition, DebugDeclare, and "
+                     "DebugValue found inside function",
+                     opcode);
+              return false;
+            }
+          }
+        } else if (inst->ext_inst_type ==
+                   SPV_EXT_INST_TYPE_NONSEMANTIC_VULKAN_DEBUGINFO_100) {
+          const NonSemanticVulkanDebugInfo100Instructions ext_inst_key =
+              NonSemanticVulkanDebugInfo100Instructions(ext_inst_index);
+          switch (ext_inst_key) {
+            case NonSemanticVulkanDebugInfo100DebugDeclare:
+            case NonSemanticVulkanDebugInfo100DebugValue:
+            case NonSemanticVulkanDebugInfo100DebugScope:
+            case NonSemanticVulkanDebugInfo100DebugNoScope:
+            case NonSemanticVulkanDebugInfo100DebugFunctionDefinition: {
+              if (block_ == nullptr) {  // Inside function but outside blocks
+                Errorf(consumer_, src, loc,
+                       "Debug info extension instruction found inside function "
+                       "but outside block",
+                       opcode);
+              } else {
+                block_->AddInstruction(std::move(spv_inst));
+              }
+              break;
+            }
             default: {
               Errorf(consumer_, src, loc,
                      "Debug info extension instruction other than DebugScope, "

diff --git a/source/opt/module.cpp b/source/opt/module.cpp
@@ -145,8 +145,10 @@ void Module::ToBinary(std::vector<uint32_t>* binary, bool skip_nop) const {
   DebugScope last_scope(kNoDebugScope, kNoInlinedAt);
   const Instruction* last_line_inst = nullptr;
   bool between_merge_and_branch = false;
+  bool between_label_and_phi_var = false;
   auto write_inst = [binary, skip_nop, &last_scope, &last_line_inst,
-                     &between_merge_and_branch, this](const Instruction* i) {
+                     &between_merge_and_branch, &between_label_and_phi_var,
+                     this](const Instruction* i) {
     // Skip emitting line instructions between merge and branch instructions.
     auto opcode = i->opcode();
     if (between_merge_and_branch &&
@@ -175,13 +177,29 @@ void Module::ToBinary(std::vector<uint32_t>* binary, bool skip_nop) const {
         last_line_inst = nullptr;
       }
     }
+
+    if (opcode == SpvOpLabel) {
+      between_label_and_phi_var = true;
+    } else if (opcode != SpvOpVariable && opcode != SpvOpPhi &&
+               opcode != SpvOpLine && opcode != SpvOpNoLine) {
+      between_label_and_phi_var = false;
+    }
+
     if (!(skip_nop && i->IsNop())) {
       const auto& scope = i->GetDebugScope();
       if (scope != last_scope) {
-        // Emit DebugScope |scope| to |binary|.
-        auto dbg_inst = ext_inst_debuginfo_.begin();
-        scope.ToBinary(dbg_inst->type_id(), context()->TakeNextId(),
-                       dbg_inst->GetSingleWordOperand(2), binary);
+        // Can only emit nonsemantic instructions after all phi instructions
+        // in a block so don't emit scope instructions before phi instructions
+        // for Vulkan.NonSemantic.DebugInfo.100.
+        if (!between_label_and_phi_var ||
+            context()
+                ->get_feature_mgr()
+                ->GetExtInstImportId_OpenCL100DebugInfo()) {
+          // Emit DebugScope |scope| to |binary|.
+          auto dbg_inst = ext_inst_debuginfo_.begin();
+          scope.ToBinary(dbg_inst->type_id(), context()->TakeNextId(),
+                         dbg_inst->GetSingleWordOperand(2), binary);
+        }
         last_scope = scope;
       }