diff --git a/source/opt/ir_loader.cpp b/source/opt/ir_loader.cpp index 70e5144aa..e443ebb5f 100644 --- a/source/opt/ir_loader.cpp +++ b/source/opt/ir_loader.cpp @@ -41,6 +41,7 @@ bool IrLoader::AddInstruction(const spv_parsed_instruction_t* inst) { ++inst_index_; const auto opcode = static_cast(inst->opcode); if (IsDebugLineInst(opcode)) { + module()->SetContainsDebugInfo(); last_line_inst_.reset(); dbg_line_info_.push_back( Instruction(module()->context(), *inst, last_dbg_scope_)); @@ -61,12 +62,12 @@ bool IrLoader::AddInstruction(const spv_parsed_instruction_t* inst) { inlined_at = inst->words[kInlinedAtIndex]; last_dbg_scope_ = DebugScope(inst->words[kLexicalScopeIndex], inlined_at); - module()->SetContainsDebugScope(); + module()->SetContainsDebugInfo(); return true; } if (ext_inst_key == OpenCLDebugInfo100DebugNoScope) { last_dbg_scope_ = DebugScope(kNoDebugScope, kNoInlinedAt); - module()->SetContainsDebugScope(); + module()->SetContainsDebugInfo(); return true; } } else { @@ -78,12 +79,12 @@ bool IrLoader::AddInstruction(const spv_parsed_instruction_t* inst) { inlined_at = inst->words[kInlinedAtIndex]; last_dbg_scope_ = DebugScope(inst->words[kLexicalScopeIndex], inlined_at); - module()->SetContainsDebugScope(); + module()->SetContainsDebugInfo(); return true; } if (ext_inst_key == DebugInfoDebugNoScope) { last_dbg_scope_ = DebugScope(kNoDebugScope, kNoInlinedAt); - module()->SetContainsDebugScope(); + module()->SetContainsDebugInfo(); return true; } } diff --git a/source/opt/module.h b/source/opt/module.h index 2c96f0295..d609b603a 100644 --- a/source/opt/module.h +++ b/source/opt/module.h @@ -49,7 +49,7 @@ class Module { using const_inst_iterator = InstructionList::const_iterator; // Creates an empty module with zero'd header. - Module() : header_({}), contains_debug_scope_(false) {} + Module() : header_({}), contains_debug_info_(false) {} // Sets the header to the given |header|. void SetHeader(const ModuleHeader& header) { header_ = header; } @@ -119,9 +119,9 @@ class Module { // Appends a function to this module. inline void AddFunction(std::unique_ptr f); - // Sets |contains_debug_scope_| as true. - inline void SetContainsDebugScope(); - inline bool ContainsDebugScope() { return contains_debug_scope_; } + // Sets |contains_debug_info_| as true. + inline void SetContainsDebugInfo(); + inline bool ContainsDebugInfo() { return contains_debug_info_; } // Returns a vector of pointers to type-declaration instructions in this // module. @@ -301,8 +301,8 @@ class Module { // any instruction. We record them here, so they will not be lost. std::vector trailing_dbg_line_info_; - // This module contains DebugScope or DebugNoScope. - bool contains_debug_scope_; + // This module contains DebugScope/DebugNoScope or OpLine/OpNoLine. + bool contains_debug_info_; }; // Pretty-prints |module| to |str|. Returns |str|. @@ -364,7 +364,7 @@ inline void Module::AddFunction(std::unique_ptr f) { functions_.emplace_back(std::move(f)); } -inline void Module::SetContainsDebugScope() { contains_debug_scope_ = true; } +inline void Module::SetContainsDebugInfo() { contains_debug_info_ = true; } inline Module::inst_iterator Module::capability_begin() { return capabilities_.begin(); diff --git a/source/opt/optimizer.cpp b/source/opt/optimizer.cpp index 8726ff93d..c2a21f2ce 100644 --- a/source/opt/optimizer.cpp +++ b/source/opt/optimizer.cpp @@ -583,10 +583,12 @@ bool Optimizer::Run(const uint32_t* original_binary, #ifndef NDEBUG // We do not keep the result id of DebugScope in struct DebugScope. // Instead, we assign random ids for them, which results in integrity + // check failures. In addition, propagating the OpLine/OpNoLine to preserve + // the debug information through transformations results in integrity // check failures. We want to skip the integrity check when the module - // contains DebugScope instructions. + // contains DebugScope or OpLine/OpNoLine instructions. if (status == opt::Pass::Status::SuccessWithoutChange && - !context->module()->ContainsDebugScope()) { + !context->module()->ContainsDebugInfo()) { std::vector optimized_binary_with_nop; context->module()->ToBinary(&optimized_binary_with_nop, /* skip_nop = */ false); diff --git a/test/opt/optimizer_test.cpp b/test/opt/optimizer_test.cpp index 945aa7826..de79536e4 100644 --- a/test/opt/optimizer_test.cpp +++ b/test/opt/optimizer_test.cpp @@ -762,6 +762,124 @@ OpFunctionEnd << "Was expecting the OpNop to have been removed."; } +TEST(Optimizer, AvoidIntegrityCheckForExtraLineInfo) { + // Test that it avoids the integrity check when no optimizations are run and + // OpLines are propagated. + const std::string before = R"(OpCapability Shader +OpCapability Linkage +OpMemoryModel Logical GLSL450 +%1 = OpString "Test" +%void = OpTypeVoid +%3 = OpTypeFunction %void +%uint = OpTypeInt 32 0 +%_ptr_Function_uint = OpTypePointer Function %uint +%6 = OpFunction %void None %3 +%7 = OpLabel +OpLine %1 10 0 +%8 = OpVariable %_ptr_Function_uint Function +OpLine %1 10 0 +%9 = OpVariable %_ptr_Function_uint Function +OpLine %1 20 0 +OpReturn +OpFunctionEnd +)"; + + const std::string after = R"(OpCapability Shader +OpCapability Linkage +OpMemoryModel Logical GLSL450 +%1 = OpString "Test" +%void = OpTypeVoid +%3 = OpTypeFunction %void +%uint = OpTypeInt 32 0 +%_ptr_Function_uint = OpTypePointer Function %uint +%6 = OpFunction %void None %3 +%7 = OpLabel +OpLine %1 10 0 +%8 = OpVariable %_ptr_Function_uint Function +%9 = OpVariable %_ptr_Function_uint Function +OpLine %1 20 0 +OpReturn +OpFunctionEnd +)"; + + std::vector binary; + SpirvTools tools(SPV_ENV_VULKAN_1_1); + tools.Assemble(before, &binary); + + Optimizer opt(SPV_ENV_VULKAN_1_1); + + std::vector optimized; + class ValidatorOptions validator_options; + ASSERT_TRUE(opt.Run(binary.data(), binary.size(), &optimized, + validator_options, true)) + << before << "\n"; + + std::string disassembly; + tools.Disassemble(optimized.data(), optimized.size(), &disassembly); + + EXPECT_EQ(after, disassembly) + << "Was expecting the OpLine to have been propagated."; +} + +TEST(Optimizer, AvoidIntegrityCheckForDebugScope) { + // Test that it avoids the integrity check when the code contains DebugScope. + const std::string before = R"(OpCapability Shader +%1 = OpExtInstImport "OpenCL.DebugInfo.100" +OpMemoryModel Logical GLSL450 +OpEntryPoint Fragment %main "main" +OpExecutionMode %main OriginUpperLeft +%3 = OpString "simple_vs.hlsl" +OpSource HLSL 600 %3 +OpName %main "main" +%void = OpTypeVoid +%5 = OpTypeFunction %void +%6 = OpExtInst %void %1 DebugSource %3 +%7 = OpExtInst %void %1 DebugCompilationUnit 2 4 %6 HLSL +%main = OpFunction %void None %5 +%14 = OpLabel +%26 = OpExtInst %void %1 DebugScope %7 +OpReturn +%27 = OpExtInst %void %1 DebugNoScope +OpFunctionEnd +)"; + + const std::string after = R"(OpCapability Shader +%1 = OpExtInstImport "OpenCL.DebugInfo.100" +OpMemoryModel Logical GLSL450 +OpEntryPoint Fragment %main "main" +OpExecutionMode %main OriginUpperLeft +%3 = OpString "simple_vs.hlsl" +OpSource HLSL 600 %3 +OpName %main "main" +%void = OpTypeVoid +%5 = OpTypeFunction %void +%6 = OpExtInst %void %1 DebugSource %3 +%7 = OpExtInst %void %1 DebugCompilationUnit 2 4 %6 HLSL +%main = OpFunction %void None %5 +%8 = OpLabel +%11 = OpExtInst %void %1 DebugScope %7 +OpReturn +%12 = OpExtInst %void %1 DebugNoScope +OpFunctionEnd +)"; + + std::vector binary; + SpirvTools tools(SPV_ENV_VULKAN_1_1); + tools.Assemble(before, &binary); + + Optimizer opt(SPV_ENV_VULKAN_1_1); + + std::vector optimized; + ASSERT_TRUE(opt.Run(binary.data(), binary.size(), &optimized)) + << before << "\n"; + + std::string disassembly; + tools.Disassemble(optimized.data(), optimized.size(), &disassembly); + + EXPECT_EQ(after, disassembly) + << "Was expecting the result id of DebugScope to have been changed."; +} + } // namespace } // namespace opt } // namespace spvtools