Skip to content

Commit

Permalink
[OPT] Fix generating debugLocalVariable from debugGlobalVariable (Khr…
Browse files Browse the repository at this point in the history
…onosGroup#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 KhronosGroup#5776
  • Loading branch information
s-perron authored and Keenuts committed Nov 12, 2024
1 parent 235394b commit 29bf25d
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 3 deletions.
20 changes: 17 additions & 3 deletions source/opt/debug_info_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
78 changes: 78 additions & 0 deletions test/opt/debug_info_manager_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<IRContext> 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<Operand> 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
Expand Down

0 comments on commit 29bf25d

Please sign in to comment.