diff --git a/source/operand.cpp b/source/operand.cpp index 52a839f3af..304260668d 100644 --- a/source/operand.cpp +++ b/source/operand.cpp @@ -19,6 +19,8 @@ #include +#include "DebugInfo.h" +#include "OpenCLDebugInfo100.h" #include "source/macro.h" #include "source/spirv_constant.h" #include "source/spirv_target_env.h" @@ -514,21 +516,35 @@ std::function spvOperandCanBeForwardDeclaredFunction( } std::function spvDbgInfoExtOperandCanBeForwardDeclaredFunction( - OpenCLDebugInfo100Instructions key) { - std::function out; + spv_ext_inst_type_t ext_type, uint32_t key) { // 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. - switch (key) { - case OpenCLDebugInfo100DebugFunction: - out = [](unsigned index) { return index == 13; }; - break; - case OpenCLDebugInfo100DebugTypeComposite: - out = [](unsigned index) { return index >= 13; }; - break; - default: - out = [](unsigned) { return false; }; - break; + std::function out; + if (ext_type == SPV_EXT_INST_TYPE_OPENCL_DEBUGINFO_100) { + switch (OpenCLDebugInfo100Instructions(key)) { + case OpenCLDebugInfo100DebugFunction: + out = [](unsigned index) { return index == 13; }; + break; + case OpenCLDebugInfo100DebugTypeComposite: + out = [](unsigned index) { return index >= 13; }; + break; + default: + out = [](unsigned) { return false; }; + break; + } + } else { + switch (DebugInfoInstructions(key)) { + case DebugInfoDebugFunction: + out = [](unsigned index) { return index == 13; }; + break; + case DebugInfoDebugTypeComposite: + out = [](unsigned index) { return index >= 12; }; + break; + default: + out = [](unsigned) { return false; }; + break; + } } return out; } diff --git a/source/operand.h b/source/operand.h index 282554a53c..7c73c6f560 100644 --- a/source/operand.h +++ b/source/operand.h @@ -18,7 +18,6 @@ #include #include -#include "OpenCLDebugInfo100.h" #include "source/table.h" #include "spirv-tools/libspirv.h" @@ -147,6 +146,6 @@ std::function spvOperandCanBeForwardDeclaredFunction( // of the operand can be forward declared. This function will // used in the SSA validation stage of the pipeline std::function spvDbgInfoExtOperandCanBeForwardDeclaredFunction( - OpenCLDebugInfo100Instructions key); + spv_ext_inst_type_t ext_type, uint32_t key); #endif // SOURCE_OPERAND_H_ diff --git a/source/opt/ir_loader.cpp b/source/opt/ir_loader.cpp index 790cbf0559..836012f171 100644 --- a/source/opt/ir_loader.cpp +++ b/source/opt/ir_loader.cpp @@ -16,6 +16,7 @@ #include +#include "DebugInfo.h" #include "OpenCLDebugInfo100.h" #include "source/ext_inst.h" #include "source/opt/log.h" @@ -143,18 +144,34 @@ bool IrLoader::AddInstruction(const spv_parsed_instruction_t* inst) { if (opcode == SpvOpExtInst && spvExtInstIsDebugInfo(inst->ext_inst_type)) { const uint32_t ext_inst_index = inst->words[4]; - const OpenCLDebugInfo100Instructions ext_inst_key = - OpenCLDebugInfo100Instructions(ext_inst_index); - if (ext_inst_key != OpenCLDebugInfo100DebugScope && - ext_inst_key != OpenCLDebugInfo100DebugNoScope && - ext_inst_key != OpenCLDebugInfo100DebugDeclare && - ext_inst_key != OpenCLDebugInfo100DebugValue) { - Errorf(consumer_, src, loc, - "Debug info extension instruction other than DebugScope, " - "DebugNoScope, DebugDeclare, and DebugValue found inside " - "function", - opcode); - return false; + if (inst->ext_inst_type == SPV_EXT_INST_TYPE_OPENCL_DEBUGINFO_100) { + const OpenCLDebugInfo100Instructions ext_inst_key = + OpenCLDebugInfo100Instructions(ext_inst_index); + if (ext_inst_key != OpenCLDebugInfo100DebugScope && + ext_inst_key != OpenCLDebugInfo100DebugNoScope && + ext_inst_key != OpenCLDebugInfo100DebugDeclare && + ext_inst_key != OpenCLDebugInfo100DebugValue) { + Errorf(consumer_, src, loc, + "Debug info extension instruction other than DebugScope, " + "DebugNoScope, DebugDeclare, and DebugValue found inside " + "function", + opcode); + return false; + } + } else { + const DebugInfoInstructions ext_inst_key = + DebugInfoInstructions(ext_inst_index); + if (ext_inst_key != DebugInfoDebugScope && + ext_inst_key != DebugInfoDebugNoScope && + ext_inst_key != DebugInfoDebugDeclare && + ext_inst_key != DebugInfoDebugValue) { + Errorf(consumer_, src, loc, + "Debug info extension instruction other than DebugScope, " + "DebugNoScope, DebugDeclare, and DebugValue found inside " + "function", + opcode); + return false; + } } } block_->AddInstruction(std::move(spv_inst)); diff --git a/source/val/validate_id.cpp b/source/val/validate_id.cpp index 646e7c85a8..c171d310d6 100644 --- a/source/val/validate_id.cpp +++ b/source/val/validate_id.cpp @@ -134,7 +134,7 @@ spv_result_t IdPass(ValidationState_t& _, Instruction* inst) { inst->opcode() == SpvOpExtInst && spvExtInstIsDebugInfo(inst->ext_inst_type()) ? spvDbgInfoExtOperandCanBeForwardDeclaredFunction( - OpenCLDebugInfo100Instructions(inst->word(4))) + inst->ext_inst_type(), inst->word(4)) : spvOperandCanBeForwardDeclaredFunction(inst->opcode()); // Keep track of a result id defined by this instruction. 0 means it diff --git a/source/val/validate_layout.cpp b/source/val/validate_layout.cpp index f2644779ed..2eb7241470 100644 --- a/source/val/validate_layout.cpp +++ b/source/val/validate_layout.cpp @@ -16,6 +16,7 @@ #include +#include "DebugInfo.h" #include "OpenCLDebugInfo100.h" #include "source/diagnostic.h" #include "source/opcode.h" @@ -197,18 +198,34 @@ spv_result_t FunctionScopedInstructions(ValidationState_t& _, } } else if (spvExtInstIsDebugInfo(inst->ext_inst_type())) { const uint32_t ext_inst_index = inst->word(4); - const OpenCLDebugInfo100Instructions ext_inst_key = - OpenCLDebugInfo100Instructions(ext_inst_index); - if (ext_inst_key == OpenCLDebugInfo100DebugScope || - ext_inst_key == OpenCLDebugInfo100DebugNoScope || - ext_inst_key == OpenCLDebugInfo100DebugDeclare || - ext_inst_key == OpenCLDebugInfo100DebugValue) { + bool local_debug_info = false; + if (inst->ext_inst_type() == SPV_EXT_INST_TYPE_OPENCL_DEBUGINFO_100) { + const OpenCLDebugInfo100Instructions ext_inst_key = + OpenCLDebugInfo100Instructions(ext_inst_index); + if (ext_inst_key == OpenCLDebugInfo100DebugScope || + ext_inst_key == OpenCLDebugInfo100DebugNoScope || + ext_inst_key == OpenCLDebugInfo100DebugDeclare || + ext_inst_key == OpenCLDebugInfo100DebugValue) { + local_debug_info = true; + } + } else { + const DebugInfoInstructions ext_inst_key = + DebugInfoInstructions(ext_inst_index); + if (ext_inst_key == DebugInfoDebugScope || + ext_inst_key == DebugInfoDebugNoScope || + ext_inst_key == DebugInfoDebugDeclare || + ext_inst_key == DebugInfoDebugValue) { + local_debug_info = true; + } + } + + if (local_debug_info) { if (_.in_function_body() == false) { // DebugScope, DebugNoScope, DebugDeclare, DebugValue must // appear in a function body. return _.diag(SPV_ERROR_INVALID_LAYOUT, inst) - << "OpenCL.DebugInfo.100 DebugScope, DebugNoScope, " - << "DebugDeclare, DebugValue must appear in a function " + << "DebugScope, DebugNoScope, DebugDeclare, DebugValue " + << "of debug info extension must appear in a function " << "body"; } } else { @@ -219,7 +236,7 @@ spv_result_t FunctionScopedInstructions(ValidationState_t& _, if (_.current_layout_section() < kLayoutTypes || _.current_layout_section() >= kLayoutFunctionDeclarations) { return _.diag(SPV_ERROR_INVALID_LAYOUT, inst) - << "OpenCL.DebugInfo.100 instructions other than " + << "Debug info extension instructions other than " << "DebugScope, DebugNoScope, DebugDeclare, DebugValue " << "must appear between section 9 (types, constants, " << "global variables) and section 10 (function " diff --git a/test/opt/ir_loader_test.cpp b/test/opt/ir_loader_test.cpp index 9bf91cb858..c60e8537ad 100644 --- a/test/opt/ir_loader_test.cpp +++ b/test/opt/ir_loader_test.cpp @@ -223,16 +223,16 @@ OpLine %7 3 18 %out_var_COLOR = OpVariable %_ptr_Output_v4float Output %34 = OpExtInst %void %1 DebugSource %7 %8 %35 = OpExtInst %void %1 DebugCompilationUnit 2 4 %34 HLSL -%36 = OpExtInst %void %1 DebugTypeComposite %9 Structure %34 1 1 %35 %13 %int_128 FlagIsProtected|FlagIsPrivate %34 %34 -%37 = OpExtInst %void %1 DebugTypeBasic %10 %int_32 Float -%38 = OpExtInst %void %1 DebugTypeVector %37 4 -%39 = OpExtInst %void %1 DebugTypeMember %11 %38 %34 2 3 %36 %int_0 %int_128 FlagIsProtected|FlagIsPrivate -%40 = OpExtInst %void %1 DebugTypeMember %12 %38 %34 3 3 %36 %int_128 %int_128 FlagIsProtected|FlagIsPrivate -%41 = OpExtInst %void %1 DebugTypeFunction FlagIsProtected|FlagIsPrivate %36 %38 %38 +%36 = OpExtInst %void %1 DebugTypeComposite %9 Structure %34 1 1 %35 %13 %int_128 FlagIsProtected|FlagIsPrivate %37 %38 +%39 = OpExtInst %void %1 DebugTypeBasic %10 %int_32 Float +%40 = OpExtInst %void %1 DebugTypeVector %39 4 +%37 = OpExtInst %void %1 DebugTypeMember %11 %40 %34 2 3 %36 %int_0 %int_128 FlagIsProtected|FlagIsPrivate +%38 = OpExtInst %void %1 DebugTypeMember %12 %40 %34 3 3 %36 %int_128 %int_128 FlagIsProtected|FlagIsPrivate +%41 = OpExtInst %void %1 DebugTypeFunction FlagIsProtected|FlagIsPrivate %36 %40 %40 %42 = OpExtInst %void %1 DebugExpression %43 = OpExtInst %void %1 DebugFunction %14 %41 %34 6 1 %35 %15 FlagIsProtected|FlagIsPrivate 7 %main -%44 = OpExtInst %void %1 DebugLocalVariable %16 %38 %34 6 16 %43 FlagIsLocal 0 -%45 = OpExtInst %void %1 DebugLocalVariable %17 %38 %34 7 16 %43 FlagIsLocal 1 +%44 = OpExtInst %void %1 DebugLocalVariable %16 %40 %34 6 16 %43 FlagIsLocal 0 +%45 = OpExtInst %void %1 DebugLocalVariable %17 %40 %34 7 16 %43 FlagIsLocal 1 %46 = OpExtInst %void %1 DebugLocalVariable %18 %36 %34 8 3 %43 FlagIsLocal %47 = OpExtInst %void %1 DebugDeclare %44 %pos %42 %48 = OpExtInst %void %1 DebugDeclare %45 %color %42 diff --git a/test/val/val_ext_inst_test.cpp b/test/val/val_ext_inst_test.cpp index 346932194f..664fc1ec58 100644 --- a/test/val/val_ext_inst_test.cpp +++ b/test/val/val_ext_inst_test.cpp @@ -33,6 +33,7 @@ using ::testing::HasSubstr; using ::testing::Not; using ValidateExtInst = spvtest::ValidateBase; +using ValidateOldDebugInfo = spvtest::ValidateBase; using ValidateOpenCL100DebugInfo = spvtest::ValidateBase; using ValidateGlslStd450SqrtLike = spvtest::ValidateBase; using ValidateGlslStd450FMinLike = spvtest::ValidateBase; @@ -655,6 +656,24 @@ OpFunctionEnd)"; return ss.str(); } +TEST_F(ValidateOldDebugInfo, UseDebugInstructionOutOfFunction) { + const std::string src = R"( +%code = OpString "main() {}" +)"; + + const std::string dbg_inst = R"( +%cu = OpExtInst %void %DbgExt DebugCompilationUnit %code 1 1 +)"; + + const std::string extension = R"( +%DbgExt = OpExtInstImport "DebugInfo" +)"; + + CompileSuccessfully(GenerateShaderCodeForDebugInfo(src, "", dbg_inst, "", + extension, "Vertex")); + ASSERT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + TEST_F(ValidateOpenCL100DebugInfo, UseDebugInstructionOutOfFunction) { const std::string src = R"( %src = OpString "simple.hlsl" @@ -693,7 +712,7 @@ TEST_F(ValidateOpenCL100DebugInfo, DebugSourceInFunction) { ASSERT_EQ(SPV_ERROR_INVALID_LAYOUT, ValidateInstructions()); EXPECT_THAT( getDiagnosticString(), - HasSubstr("OpenCL.DebugInfo.100 instructions other than DebugScope, " + HasSubstr("Debug info extension instructions other than DebugScope, " "DebugNoScope, DebugDeclare, DebugValue must appear between " "section 9 (types, constants, global variables) and section 10 " "(function declarations)"));