diff --git a/source/opt/debug_info_manager.cpp b/source/opt/debug_info_manager.cpp index 4941d648e6..346134d4db 100644 --- a/source/opt/debug_info_manager.cpp +++ b/source/opt/debug_info_manager.cpp @@ -40,7 +40,6 @@ static const uint32_t kDebugLocalVariableOperandParentIndex = 9; static const uint32_t kExtInstInstructionInIdx = 1; static const uint32_t kDebugGlobalVariableOperandFlagsIndex = 12; static const uint32_t kDebugLocalVariableOperandFlagsIndex = 10; -static const uint32_t kDebugLocalVariableOperandArgNumberIndex = 11; namespace spvtools { namespace opt { @@ -441,27 +440,15 @@ bool DebugInfoManager::IsAncestorOfScope(uint32_t scope, uint32_t ancestor) { return false; } -Instruction* DebugInfoManager::GetDebugLocalVariableFromDeclare( - Instruction* dbg_declare) { - assert(dbg_declare); +bool DebugInfoManager::IsDeclareVisibleToInstr(Instruction* dbg_declare, + uint32_t instr_scope_id) { + if (instr_scope_id == kNoDebugScope) return false; + uint32_t dbg_local_var_id = dbg_declare->GetSingleWordOperand(kDebugDeclareOperandLocalVariableIndex); auto dbg_local_var_itr = id_to_dbg_inst_.find(dbg_local_var_id); assert(dbg_local_var_itr != id_to_dbg_inst_.end()); - return dbg_local_var_itr->second; -} - -bool DebugInfoManager::IsFunctionParameter(Instruction* dbg_local_var) const { - // If a DebugLocalVariable has ArgNumber operand, it is a function parameter. - return dbg_local_var->NumOperands() > - kDebugLocalVariableOperandArgNumberIndex; -} - -bool DebugInfoManager::IsLocalVariableVisibleToInstr(Instruction* dbg_local_var, - uint32_t instr_scope_id) { - if (instr_scope_id == kNoDebugScope) return false; - - uint32_t decl_scope_id = dbg_local_var->GetSingleWordOperand( + uint32_t decl_scope_id = dbg_local_var_itr->second->GetSingleWordOperand( kDebugLocalVariableOperandParentIndex); // If the scope of DebugDeclare is an ancestor scope of the instruction's @@ -515,20 +502,11 @@ void DebugInfoManager::AddDebugValueIfVarDeclIsVisible( uint32_t instr_scope_id = scope_and_line->GetDebugScope().GetLexicalScope(); for (auto* dbg_decl_or_val : dbg_decl_itr->second) { - // If it declares a function parameter, the store instruction for the - // function parameter can exist out of the function parameter's scope - // because of the function inlining. We always add DebugValue for a - // function parameter next to the DebugDeclare regardless of the scope. - auto* dbg_local_var = GetDebugLocalVariableFromDeclare(dbg_decl_or_val); - bool is_function_param = IsFunctionParameter(dbg_local_var); - if (!is_function_param && - !IsLocalVariableVisibleToInstr(dbg_local_var, instr_scope_id)) - continue; + if (!IsDeclareVisibleToInstr(dbg_decl_or_val, instr_scope_id)) continue; // Avoid inserting the new DebugValue between OpPhi or OpVariable // instructions. - Instruction* insert_before = is_function_param ? dbg_decl_or_val->NextNode() - : insert_pos->NextNode(); + Instruction* insert_before = insert_pos->NextNode(); while (insert_before->opcode() == SpvOpPhi || insert_before->opcode() == SpvOpVariable) { insert_before = insert_before->NextNode(); @@ -545,8 +523,7 @@ void DebugInfoManager::AddDebugValueIfVarDeclIsVisible( kDebugValueOperandLocalVariableIndex), value_id, 0, index_id, insert_before); assert(added_dbg_value != nullptr); - added_dbg_value->UpdateDebugInfoFrom(is_function_param ? dbg_decl_or_val - : scope_and_line); + added_dbg_value->UpdateDebugInfoFrom(scope_and_line); AnalyzeDebugInst(added_dbg_value); } } diff --git a/source/opt/debug_info_manager.h b/source/opt/debug_info_manager.h index bf398b4672..6e7373bc41 100644 --- a/source/opt/debug_info_manager.h +++ b/source/opt/debug_info_manager.h @@ -215,17 +215,10 @@ class DebugInfoManager { // of |scope|. bool IsAncestorOfScope(uint32_t scope, uint32_t ancestor); - // Returns the DebugLocalVariable declared by |dbg_declare|. - Instruction* GetDebugLocalVariableFromDeclare(Instruction* dbg_declare); - - // Returns true if the DebugLocalVariable |dbg_local_var| is a function - // parameter. - bool IsFunctionParameter(Instruction* dbg_local_var) const; - - // Returns true if the DebugLocalVariable |dbg_local_var| is visible - // in the scope of an instruction |instr_scope_id|. - bool IsLocalVariableVisibleToInstr(Instruction* dbg_local_var, - uint32_t instr_scope_id); + // Returns true if the declaration of a local variable |dbg_declare| + // is visible in the scope of an instruction |instr_scope_id|. + bool IsDeclareVisibleToInstr(Instruction* dbg_declare, + uint32_t instr_scope_id); // Returns the parent scope of the scope |child_scope|. uint32_t GetParentScope(uint32_t child_scope); diff --git a/test/opt/local_single_store_elim_test.cpp b/test/opt/local_single_store_elim_test.cpp index f1b98212cf..ebc89a6300 100644 --- a/test/opt/local_single_store_elim_test.cpp +++ b/test/opt/local_single_store_elim_test.cpp @@ -1211,80 +1211,6 @@ TEST_F(LocalSingleStoreElimTest, DebugValueTest) { SinglePassRunAndMatch(text, false); } -TEST_F(LocalSingleStoreElimTest, DebugDeclareForFunctionParameter) { - // We have to add DebugValue for DebugDeclare regardless of the scope - // if it declares a function parameter. - const std::string text = R"( - OpCapability Shader - %1 = OpExtInstImport "OpenCL.DebugInfo.100" - OpMemoryModel Logical GLSL450 - OpEntryPoint GLCompute %main "main" - OpExecutionMode %main LocalSize 1 1 1 - %3 = OpString "fn_call.hlsl" - %4 = OpString "int" - %5 = OpString "" - %6 = OpString "x" - %7 = OpString "A" - %8 = OpString "main" - OpName %type_StructuredBuffer_int "type.StructuredBuffer.int" - OpName %data "data" - OpName %main "main" - OpDecorate %data DescriptorSet 0 - OpDecorate %data Binding 0 - OpDecorate %_runtimearr_int ArrayStride 4 - OpMemberDecorate %type_StructuredBuffer_int 0 Offset 0 - OpMemberDecorate %type_StructuredBuffer_int 0 NonWritable - OpDecorate %type_StructuredBuffer_int BufferBlock - %int = OpTypeInt 32 1 - %uint = OpTypeInt 32 0 - %uint_0 = OpConstant %uint 0 - %uint_32 = OpConstant %uint 32 -%_runtimearr_int = OpTypeRuntimeArray %int -%type_StructuredBuffer_int = OpTypeStruct %_runtimearr_int -%_ptr_Uniform_type_StructuredBuffer_int = OpTypePointer Uniform %type_StructuredBuffer_int - %void = OpTypeVoid - %18 = OpTypeFunction %void -%_ptr_Function_int = OpTypePointer Function %int -%_ptr_Uniform_int = OpTypePointer Uniform %int - %data = OpVariable %_ptr_Uniform_type_StructuredBuffer_int Uniform - %21 = OpExtInst %void %1 DebugInfoNone - %22 = OpExtInst %void %1 DebugExpression - %23 = OpExtInst %void %1 DebugTypeBasic %4 %uint_32 Signed - %24 = OpExtInst %void %1 DebugTypeFunction FlagIsProtected|FlagIsPrivate %23 %23 - %25 = OpExtInst %void %1 DebugSource %3 - %26 = OpExtInst %void %1 DebugCompilationUnit 1 4 %25 HLSL - %27 = OpExtInst %void %1 DebugFunction %7 %24 %25 17 1 %26 %5 FlagIsProtected|FlagIsPrivate 18 %21 - %28 = OpExtInst %void %1 DebugLocalVariable %6 %23 %25 17 11 %27 FlagIsLocal 1 - %29 = OpExtInst %void %1 DebugTypeFunction FlagIsProtected|FlagIsPrivate %void - %30 = OpExtInst %void %1 DebugFunction %8 %29 %25 25 1 %26 %5 FlagIsProtected|FlagIsPrivate 25 %21 - %31 = OpExtInst %void %1 DebugLexicalBlock %25 25 13 %30 - %32 = OpExtInst %void %1 DebugInlinedAt 26 %31 -;CHECK: [[fn_param:%\w+]] = OpExtInst %void [[ext:%\w+]] DebugLocalVariable -;CHECK: [[bb:%\w+]] = OpExtInst %void [[ext]] DebugLexicalBlock -;CHECK: [[inlined_at:%\w+]] = OpExtInst %void [[ext]] DebugInlinedAt 26 [[bb]] - %main = OpFunction %void None %18 - %bb = OpLabel - %33 = OpExtInst %void %1 DebugScope %31 - %34 = OpVariable %_ptr_Function_int Function - OpLine %3 26 15 - %35 = OpAccessChain %_ptr_Uniform_int %data %uint_0 %uint_0 - %36 = OpLoad %int %35 -;CHECK: DebugScope [[bb]] -;CHECK: OpLine {{%\w+}} 26 15 -;CHECK: OpStore {{%\w+}} [[val:%\w+]] - OpStore %34 %36 - %37 = OpExtInst %void %1 DebugScope %27 %32 - %38 = OpExtInst %void %1 DebugDeclare %28 %34 %22 -;CHECK: DebugScope {{%\w+}} [[inlined_at:%\w+]] -;CHECK: DebugValue [[fn_param]] [[val]] - OpReturn - OpFunctionEnd - )"; - - SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS); - SinglePassRunAndMatch(text, false); -} - // TODO(greg-lunarg): Add tests to verify handling of these cases: // // Other types