Skip to content

Commit d85446f

Browse files
authored
[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
1 parent a2c9c23 commit d85446f

File tree

2 files changed

+95
-3
lines changed

2 files changed

+95
-3
lines changed

source/opt/debug_info_manager.cpp

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -768,15 +768,29 @@ void DebugInfoManager::ConvertDebugGlobalToLocalVariable(
768768
local_var->opcode() == spv::Op::OpFunctionParameter);
769769

770770
// Convert |dbg_global_var| to DebugLocalVariable
771+
// All of the operands up to the scope operand are the same for the type
772+
// instructions. The flag operand needs to move from operand
773+
// kDebugGlobalVariableOperandFlagsIndex to
774+
// kDebugLocalVariableOperandFlagsIndex. No other operands are needed to
775+
// define the DebugLocalVariable.
776+
777+
// Modify the opcode.
771778
dbg_global_var->SetInOperand(kExtInstInstructionInIdx,
772779
{CommonDebugInfoDebugLocalVariable});
780+
781+
// Move the flags operand.
773782
auto flags = dbg_global_var->GetSingleWordOperand(
774783
kDebugGlobalVariableOperandFlagsIndex);
775-
for (uint32_t i = dbg_global_var->NumInOperands() - 1;
776-
i >= kDebugLocalVariableOperandFlagsIndex; --i) {
784+
dbg_global_var->SetOperand(kDebugLocalVariableOperandFlagsIndex, {flags});
785+
786+
// Remove the extra operands. Starting at the end to avoid copying too much
787+
// data.
788+
for (uint32_t i = dbg_global_var->NumOperands() - 1;
789+
i > kDebugLocalVariableOperandFlagsIndex; --i) {
777790
dbg_global_var->RemoveOperand(i);
778791
}
779-
dbg_global_var->SetOperand(kDebugLocalVariableOperandFlagsIndex, {flags});
792+
793+
// Update the def-use manager.
780794
context()->ForgetUses(dbg_global_var);
781795
context()->AnalyzeUses(dbg_global_var);
782796

test/opt/debug_info_manager_test.cpp

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -799,6 +799,84 @@ void main(float in_var_color : COLOR) {
799799
7);
800800
}
801801

802+
TEST(DebugInfoManager, ConvertGlobalToLocal) {
803+
const std::string text = R"(
804+
OpCapability Shader
805+
%1 = OpExtInstImport "NonSemantic.Shader.DebugInfo.100"
806+
OpMemoryModel Logical GLSL450
807+
OpEntryPoint Fragment %2 "PSMain" %3
808+
OpExecutionMode %2 OriginUpperLeft
809+
%4 = OpString "C:\\local\\Temp\\2528091a-6811-4e62-9ed5-02f1547c2016.hlsl"
810+
%5 = OpString "float"
811+
%6 = OpString "Pi"
812+
%float = OpTypeFloat 32
813+
%float_3_1415 = OpConstant %float 3.1415
814+
%uint = OpTypeInt 32 0
815+
%uint_32 = OpConstant %uint 32
816+
%_ptr_Private_float = OpTypePointer Private %float
817+
%_ptr_Function_float = OpTypePointer Function %float
818+
%void = OpTypeVoid
819+
%uint_3 = OpConstant %uint 3
820+
%uint_0 = OpConstant %uint 0
821+
%uint_4 = OpConstant %uint 4
822+
%uint_1 = OpConstant %uint 1
823+
%uint_5 = OpConstant %uint 5
824+
%uint_8 = OpConstant %uint 8
825+
%uint_6 = OpConstant %uint 6
826+
%uint_20 = OpConstant %uint 20
827+
%25 = OpTypeFunction %void
828+
%uint_11 = OpConstant %uint 11
829+
%3 = OpVariable %_ptr_Private_float Private
830+
%8 = OpExtInst %void %1 DebugTypeBasic %5 %uint_32 %uint_3 %uint_0
831+
%12 = OpExtInst %void %1 DebugSource %4
832+
%13 = OpExtInst %void %1 DebugCompilationUnit %uint_1 %uint_4 %12 %uint_5
833+
%17 = OpExtInst %void %1 DebugGlobalVariable %6 %8 %12 %uint_6 %uint_20 %13 %6 %3 %uint_8
834+
%2 = OpFunction %void None %25
835+
%27 = OpLabel
836+
%29 = OpVariable %_ptr_Function_float Function
837+
OpStore %3 %float_3_1415
838+
%28 = OpExtInst %void %1 DebugLine %12 %uint_11 %uint_11 %uint_1 %uint_1
839+
OpReturn
840+
OpFunctionEnd
841+
)";
842+
843+
std::unique_ptr<IRContext> context =
844+
BuildModule(SPV_ENV_UNIVERSAL_1_1, nullptr, text,
845+
SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
846+
auto* def_use_mgr = context->get_def_use_mgr();
847+
auto* dbg_var = def_use_mgr->GetDef(17);
848+
EXPECT_EQ(dbg_var->GetCommonDebugOpcode(),
849+
OpenCLDebugInfo100DebugGlobalVariable);
850+
EXPECT_EQ(dbg_var->NumInOperands(), 11);
851+
852+
std::vector<Operand> originalOperands;
853+
for (uint32_t i = 0; i < dbg_var->NumInOperands(); ++i) {
854+
originalOperands.emplace_back(dbg_var->GetInOperand((i)));
855+
}
856+
857+
auto* local_var = def_use_mgr->GetDef(29);
858+
auto* dbg_info_mgr = context->get_debug_info_mgr();
859+
dbg_info_mgr->ConvertDebugGlobalToLocalVariable(dbg_var, local_var);
860+
861+
EXPECT_EQ(dbg_var->NumInOperands(), 9);
862+
863+
// This checks that the first two inoperands are correct.
864+
EXPECT_EQ(dbg_var->GetCommonDebugOpcode(),
865+
OpenCLDebugInfo100DebugLocalVariable);
866+
867+
// Then next 6 operands should be the same as the original instruction.
868+
EXPECT_EQ(dbg_var->GetInOperand(2), originalOperands[2]);
869+
EXPECT_EQ(dbg_var->GetInOperand(3), originalOperands[3]);
870+
EXPECT_EQ(dbg_var->GetInOperand(4), originalOperands[4]);
871+
EXPECT_EQ(dbg_var->GetInOperand(5), originalOperands[5]);
872+
EXPECT_EQ(dbg_var->GetInOperand(6), originalOperands[6]);
873+
EXPECT_EQ(dbg_var->GetInOperand(7), originalOperands[7]);
874+
875+
// The flags operand should have shifted because operand 8 and 9 in the global
876+
// instruction are not relevant.
877+
EXPECT_EQ(dbg_var->GetInOperand(8), originalOperands[10]);
878+
}
879+
802880
} // namespace
803881
} // namespace analysis
804882
} // namespace opt

0 commit comments

Comments
 (0)