From d85446fd7eec314e50ed373bf91a7661653bc49a Mon Sep 17 00:00:00 2001 From: Steven Perron Date: Mon, 16 Sep 2024 10:11:22 -0400 Subject: [PATCH] [OPT] Fix generating debugLocalVariable from debugGlobalVariable (#5803) The code converting the global to local was generating an extra operand that was incorrect. Fixed the code, and added a unit test. Fixes #5776 --- source/opt/debug_info_manager.cpp | 20 +++++-- test/opt/debug_info_manager_test.cpp | 78 ++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 3 deletions(-) diff --git a/source/opt/debug_info_manager.cpp b/source/opt/debug_info_manager.cpp index 1e614c6ff..24094b36e 100644 --- a/source/opt/debug_info_manager.cpp +++ b/source/opt/debug_info_manager.cpp @@ -768,15 +768,29 @@ void DebugInfoManager::ConvertDebugGlobalToLocalVariable( local_var->opcode() == spv::Op::OpFunctionParameter); // Convert |dbg_global_var| to DebugLocalVariable + // All of the operands up to the scope operand are the same for the type + // instructions. The flag operand needs to move from operand + // kDebugGlobalVariableOperandFlagsIndex to + // kDebugLocalVariableOperandFlagsIndex. No other operands are needed to + // define the DebugLocalVariable. + + // Modify the opcode. dbg_global_var->SetInOperand(kExtInstInstructionInIdx, {CommonDebugInfoDebugLocalVariable}); + + // Move the flags operand. auto flags = dbg_global_var->GetSingleWordOperand( kDebugGlobalVariableOperandFlagsIndex); - for (uint32_t i = dbg_global_var->NumInOperands() - 1; - i >= kDebugLocalVariableOperandFlagsIndex; --i) { + dbg_global_var->SetOperand(kDebugLocalVariableOperandFlagsIndex, {flags}); + + // Remove the extra operands. Starting at the end to avoid copying too much + // data. + for (uint32_t i = dbg_global_var->NumOperands() - 1; + i > kDebugLocalVariableOperandFlagsIndex; --i) { dbg_global_var->RemoveOperand(i); } - dbg_global_var->SetOperand(kDebugLocalVariableOperandFlagsIndex, {flags}); + + // Update the def-use manager. context()->ForgetUses(dbg_global_var); context()->AnalyzeUses(dbg_global_var); diff --git a/test/opt/debug_info_manager_test.cpp b/test/opt/debug_info_manager_test.cpp index 3df26a976..9c7572803 100644 --- a/test/opt/debug_info_manager_test.cpp +++ b/test/opt/debug_info_manager_test.cpp @@ -799,6 +799,84 @@ void main(float in_var_color : COLOR) { 7); } +TEST(DebugInfoManager, ConvertGlobalToLocal) { + const std::string text = R"( + OpCapability Shader + %1 = OpExtInstImport "NonSemantic.Shader.DebugInfo.100" + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %2 "PSMain" %3 + OpExecutionMode %2 OriginUpperLeft + %4 = OpString "C:\\local\\Temp\\2528091a-6811-4e62-9ed5-02f1547c2016.hlsl" + %5 = OpString "float" + %6 = OpString "Pi" + %float = OpTypeFloat 32 +%float_3_1415 = OpConstant %float 3.1415 + %uint = OpTypeInt 32 0 + %uint_32 = OpConstant %uint 32 +%_ptr_Private_float = OpTypePointer Private %float +%_ptr_Function_float = OpTypePointer Function %float + %void = OpTypeVoid + %uint_3 = OpConstant %uint 3 + %uint_0 = OpConstant %uint 0 + %uint_4 = OpConstant %uint 4 + %uint_1 = OpConstant %uint 1 + %uint_5 = OpConstant %uint 5 + %uint_8 = OpConstant %uint 8 + %uint_6 = OpConstant %uint 6 + %uint_20 = OpConstant %uint 20 + %25 = OpTypeFunction %void + %uint_11 = OpConstant %uint 11 + %3 = OpVariable %_ptr_Private_float Private + %8 = OpExtInst %void %1 DebugTypeBasic %5 %uint_32 %uint_3 %uint_0 + %12 = OpExtInst %void %1 DebugSource %4 + %13 = OpExtInst %void %1 DebugCompilationUnit %uint_1 %uint_4 %12 %uint_5 + %17 = OpExtInst %void %1 DebugGlobalVariable %6 %8 %12 %uint_6 %uint_20 %13 %6 %3 %uint_8 + %2 = OpFunction %void None %25 + %27 = OpLabel + %29 = OpVariable %_ptr_Function_float Function + OpStore %3 %float_3_1415 + %28 = OpExtInst %void %1 DebugLine %12 %uint_11 %uint_11 %uint_1 %uint_1 + OpReturn + OpFunctionEnd + )"; + + std::unique_ptr context = + BuildModule(SPV_ENV_UNIVERSAL_1_1, nullptr, text, + SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS); + auto* def_use_mgr = context->get_def_use_mgr(); + auto* dbg_var = def_use_mgr->GetDef(17); + EXPECT_EQ(dbg_var->GetCommonDebugOpcode(), + OpenCLDebugInfo100DebugGlobalVariable); + EXPECT_EQ(dbg_var->NumInOperands(), 11); + + std::vector originalOperands; + for (uint32_t i = 0; i < dbg_var->NumInOperands(); ++i) { + originalOperands.emplace_back(dbg_var->GetInOperand((i))); + } + + auto* local_var = def_use_mgr->GetDef(29); + auto* dbg_info_mgr = context->get_debug_info_mgr(); + dbg_info_mgr->ConvertDebugGlobalToLocalVariable(dbg_var, local_var); + + EXPECT_EQ(dbg_var->NumInOperands(), 9); + + // This checks that the first two inoperands are correct. + EXPECT_EQ(dbg_var->GetCommonDebugOpcode(), + OpenCLDebugInfo100DebugLocalVariable); + + // Then next 6 operands should be the same as the original instruction. + EXPECT_EQ(dbg_var->GetInOperand(2), originalOperands[2]); + EXPECT_EQ(dbg_var->GetInOperand(3), originalOperands[3]); + EXPECT_EQ(dbg_var->GetInOperand(4), originalOperands[4]); + EXPECT_EQ(dbg_var->GetInOperand(5), originalOperands[5]); + EXPECT_EQ(dbg_var->GetInOperand(6), originalOperands[6]); + EXPECT_EQ(dbg_var->GetInOperand(7), originalOperands[7]); + + // The flags operand should have shifted because operand 8 and 9 in the global + // instruction are not relevant. + EXPECT_EQ(dbg_var->GetInOperand(8), originalOperands[10]); +} + } // namespace } // namespace analysis } // namespace opt