Skip to content

Commit bc7c60e

Browse files
authored
Fix the wrong location of DebugFunctionDefinition inst (KhronosGroup#6198)
* Fix the wrong location of DebugFunctionDefinition inst DebugFunctionDefinition has to be kept in the entry block of a function, however merge_return_pass put this instruction in the case-switch block during splitting the entry block. This PR fixes this issue. * add a unit test * address comment * format and fix mem leak issue
1 parent f657d2c commit bc7c60e

File tree

2 files changed

+78
-1
lines changed

2 files changed

+78
-1
lines changed

source/opt/merge_return_pass.cpp

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -828,7 +828,8 @@ BasicBlock* MergeReturnPass::CreateContinueTarget(uint32_t header_label_id) {
828828

829829
bool MergeReturnPass::CreateSingleCaseSwitch(BasicBlock* merge_target) {
830830
// Insert the switch before any code is run. We have to split the entry
831-
// block to make sure the OpVariable instructions remain in the entry block.
831+
// block to make sure the OpVariable instructions and DebugFunctionDefinition
832+
// instructions remain in the entry block.
832833
BasicBlock* start_block = &*function_->begin();
833834
auto split_pos = start_block->begin();
834835
while (split_pos->opcode() == spv::Op::OpVariable) {
@@ -838,6 +839,18 @@ bool MergeReturnPass::CreateSingleCaseSwitch(BasicBlock* merge_target) {
838839
BasicBlock* old_block =
839840
start_block->SplitBasicBlock(context(), TakeNextId(), split_pos);
840841

842+
// Find DebugFunctionDefinition inst in the old block, and if we can find it,
843+
// move it to the entry block. Since DebugFunctionDefinition is not necessary
844+
// after OpVariable inst, we have to traverse the whole block to find it.
845+
for (auto pos = old_block->begin(); pos != old_block->end(); ++pos) {
846+
if (pos->GetShader100DebugOpcode() ==
847+
NonSemanticShaderDebugInfo100DebugFunctionDefinition) {
848+
start_block->AddInstruction(MakeUnique<Instruction>(*pos));
849+
pos.Erase();
850+
break;
851+
}
852+
}
853+
841854
// Add the switch to the end of the entry block.
842855
InstructionBuilder builder(
843856
context(), start_block,

test/opt/pass_merge_return_test.cpp

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2598,6 +2598,70 @@ TEST_F(MergeReturnPassTest, OverflowTest1) {
25982598
EXPECT_EQ(Pass::Status::Failure, std::get<1>(result));
25992599
}
26002600

2601+
TEST_F(MergeReturnPassTest, DebugFunctionDefinitionStillInEntryBlock) {
2602+
// Make sure that the DebugFunctionDefinition instruction is still in the
2603+
// entry block
2604+
const std::string text =
2605+
R"(
2606+
; CHECK: OpFunction
2607+
; CHECK: OpLabel
2608+
; CHECK: DebugFunctionDefinition
2609+
; CHECK: OpSelectionMerge
2610+
; CHECK: OpCompositeExtract
2611+
; CHECK: OpUGreaterThan
2612+
OpCapability Shader
2613+
OpExtension "SPV_KHR_non_semantic_info"
2614+
%2 = OpExtInstImport "NonSemantic.Shader.DebugInfo.100"
2615+
OpMemoryModel Logical GLSL450
2616+
OpEntryPoint GLCompute %main "main" %entryPointParam_main %gl_GlobalInvocationID
2617+
%1 = OpString "test"
2618+
OpDecorate %gl_GlobalInvocationID BuiltIn GlobalInvocationId
2619+
OpDecorate %entryPointParam_main Location 0
2620+
%void = OpTypeVoid
2621+
%4 = OpExtInst %void %2 DebugSource %1
2622+
%uint = OpTypeInt 32 0
2623+
%uint_100 = OpConstant %uint 100
2624+
%uint_5 = OpConstant %uint 5
2625+
%uint_11 = OpConstant %uint 11
2626+
%10 = OpExtInst %void %2 DebugCompilationUnit %uint_100 %uint_5 %4 %uint_11
2627+
%12 = OpTypeFunction %void
2628+
%uint_0 = OpConstant %uint 0
2629+
%14 = OpExtInst %void %2 DebugTypeFunction %uint_0 %void
2630+
%uint_2 = OpConstant %uint 2
2631+
%16 = OpExtInst %void %2 DebugFunction %1 %14 %4 %uint_2 %uint_5 %10 %1 %uint_0 %uint_2
2632+
%v3uint = OpTypeVector %uint 3
2633+
%_ptr_Input_v3uint = OpTypePointer Input %v3uint
2634+
%bool = OpTypeBool
2635+
%uint_3 = OpConstant %uint 3
2636+
%int = OpTypeInt 32 1
2637+
%_ptr_Output_int = OpTypePointer Output %int
2638+
%int_1 = OpConstant %int 1
2639+
%int_0 = OpConstant %int 0
2640+
%gl_GlobalInvocationID = OpVariable %_ptr_Input_v3uint Input ; BuiltIn GlobalInvocationId
2641+
%entryPointParam_main = OpVariable %_ptr_Output_int Output ; Location 0
2642+
2643+
; Function main
2644+
%main = OpFunction %void None %12
2645+
%13 = OpLabel
2646+
%20 = OpExtInst %void %2 DebugScope %16
2647+
%29 = OpLoad %v3uint %gl_GlobalInvocationID
2648+
%37 = OpCompositeExtract %uint %29 0
2649+
%39 = OpUGreaterThan %bool %37 %uint_3
2650+
%19 = OpExtInst %void %2 DebugFunctionDefinition %16 %main
2651+
OpSelectionMerge %21 None
2652+
OpBranchConditional %39 %23 %21
2653+
%21 = OpLabel
2654+
OpStore %entryPointParam_main %int_1
2655+
OpReturn
2656+
%23 = OpLabel
2657+
OpStore %entryPointParam_main %int_0
2658+
OpReturn
2659+
OpFunctionEnd
2660+
)";
2661+
2662+
SinglePassRunAndMatch<MergeReturnPass>(text, true);
2663+
}
2664+
26012665
} // namespace
26022666
} // namespace opt
26032667
} // namespace spvtools

0 commit comments

Comments
 (0)