diff --git a/source/opt/debug_info_manager.cpp b/source/opt/debug_info_manager.cpp index 346134d4db..4941d648e6 100644 --- a/source/opt/debug_info_manager.cpp +++ b/source/opt/debug_info_manager.cpp @@ -40,6 +40,7 @@ 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 { @@ -440,15 +441,27 @@ bool DebugInfoManager::IsAncestorOfScope(uint32_t scope, uint32_t ancestor) { return false; } -bool DebugInfoManager::IsDeclareVisibleToInstr(Instruction* dbg_declare, - uint32_t instr_scope_id) { - if (instr_scope_id == kNoDebugScope) return false; - +Instruction* DebugInfoManager::GetDebugLocalVariableFromDeclare( + Instruction* dbg_declare) { + assert(dbg_declare); 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()); - uint32_t decl_scope_id = dbg_local_var_itr->second->GetSingleWordOperand( + 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( kDebugLocalVariableOperandParentIndex); // If the scope of DebugDeclare is an ancestor scope of the instruction's @@ -502,11 +515,20 @@ void DebugInfoManager::AddDebugValueIfVarDeclIsVisible( uint32_t instr_scope_id = scope_and_line->GetDebugScope().GetLexicalScope(); for (auto* dbg_decl_or_val : dbg_decl_itr->second) { - if (!IsDeclareVisibleToInstr(dbg_decl_or_val, instr_scope_id)) continue; + // 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; // Avoid inserting the new DebugValue between OpPhi or OpVariable // instructions. - Instruction* insert_before = insert_pos->NextNode(); + Instruction* insert_before = is_function_param ? dbg_decl_or_val->NextNode() + : insert_pos->NextNode(); while (insert_before->opcode() == SpvOpPhi || insert_before->opcode() == SpvOpVariable) { insert_before = insert_before->NextNode(); @@ -523,7 +545,8 @@ void DebugInfoManager::AddDebugValueIfVarDeclIsVisible( kDebugValueOperandLocalVariableIndex), value_id, 0, index_id, insert_before); assert(added_dbg_value != nullptr); - added_dbg_value->UpdateDebugInfoFrom(scope_and_line); + added_dbg_value->UpdateDebugInfoFrom(is_function_param ? dbg_decl_or_val + : scope_and_line); AnalyzeDebugInst(added_dbg_value); } } diff --git a/source/opt/debug_info_manager.h b/source/opt/debug_info_manager.h index 6e7373bc41..bf398b4672 100644 --- a/source/opt/debug_info_manager.h +++ b/source/opt/debug_info_manager.h @@ -215,10 +215,17 @@ class DebugInfoManager { // of |scope|. bool IsAncestorOfScope(uint32_t scope, uint32_t ancestor); - // 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 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 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 ebc89a6300..f1b98212cf 100644 --- a/test/opt/local_single_store_elim_test.cpp +++ b/test/opt/local_single_store_elim_test.cpp @@ -1211,6 +1211,80 @@ 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