From 983ee2313c6818d72dc0692b6a7824ca080357fc Mon Sep 17 00:00:00 2001 From: Greg Fischer Date: Wed, 28 Jul 2021 19:35:32 -0600 Subject: [PATCH] spirv-opt: Add specific handling of vulkan debug info differences (#4398) Co-authored-by: baldurk --- source/operand.cpp | 6 ++++++ source/opt/inline_pass.cpp | 8 ++++++++ source/opt/ir_loader.cpp | 30 ++++++++++++++++++++++++++++++ source/opt/module.cpp | 28 +++++++++++++++++++++++----- 4 files changed, 67 insertions(+), 5 deletions(-) diff --git a/source/operand.cpp b/source/operand.cpp index c00c9b64ed..bff36a2699 100644 --- a/source/operand.cpp +++ b/source/operand.cpp @@ -578,6 +578,12 @@ std::function spvOperandCanBeForwardDeclaredFunction( std::function 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 index ce3bf7f379..a6bdaaff0c 100644 --- 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 index 77f90f1924..bfdd59b3d7 100644 --- a/source/opt/ir_loader.cpp +++ b/source/opt/ir_loader.cpp @@ -17,6 +17,7 @@ #include #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 index 0c88601097..6585012f2e 100644 --- a/source/opt/module.cpp +++ b/source/opt/module.cpp @@ -145,8 +145,10 @@ void Module::ToBinary(std::vector* 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* 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; }