openharmony/third_party_spirv-tools
1
0
Fork 0
mirror of https://gitee.com/openharmony/third_party_spirv-tools synced 2024-11-23 23:39:40 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Fix dead branch elim infinite loop. (#2009)

Browse Source 
When looking for a break from a selection construct, we do not realize
that a jump to the continue target of a loop containing the selection
is a break.  This causes and infinit loop, or possibly other failures.

Fixes #2004.
This commit is contained in:
Steven Perron 2018-10-24 09:10:30 -04:00 committed by GitHub
parent 07d0f9dff3
commit 18fe6d59e5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 335 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
source/opt/dead_branch_elim_pass.cpp
View File

@ -164,7 +164,8 @@ bool DeadBranchElimPass::MarkLiveBlocks(
if (mergeInst && mergeInst->opcode() == SpvOpSelectionMerge) {
Instruction* first_break = FindFirstExitFromSelectionMerge(
live_lab_id, mergeInst->GetSingleWordInOperand(0),
cfgAnalysis->LoopMergeBlock(live_lab_id));
cfgAnalysis->LoopMergeBlock(live_lab_id),
cfgAnalysis->LoopContinueBlock(live_lab_id));
if (first_break == nullptr) {
context()->KillInst(mergeInst);
} else {
@ -440,12 +441,14 @@ Pass::Status DeadBranchElimPass::Process() {
}
Instruction* DeadBranchElimPass::FindFirstExitFromSelectionMerge(
uint32_t start_block_id, uint32_t merge_block_id, uint32_t loop_merge_id) {
uint32_t start_block_id, uint32_t merge_block_id, uint32_t loop_merge_id,
uint32_t loop_continue_id) {
// To find the "first" exit, we follow branches looking for a conditional
// branch that is not in a nested construct and is not the header of a new
// construct. We follow the control flow from |start_block_id| to find the
// first one.
while (start_block_id != merge_block_id) {
while (start_block_id != merge_block_id && start_block_id != loop_merge_id &&
start_block_id != loop_continue_id) {
BasicBlock* start_block = context()->get_instr_block(start_block_id);
Instruction* branch = start_block->terminator();
uint32_t next_block_id = 0;
@ -453,15 +456,20 @@ Instruction* DeadBranchElimPass::FindFirstExitFromSelectionMerge(
case SpvOpBranchConditional:
next_block_id = start_block->MergeBlockIdIfAny();
if (next_block_id == 0) {
// If a possible target is the |loop_merge_id|, which is not the
// current merge node, then we have to continue the search with the
// other target.
// If a possible target is the |loop_merge_id| or |loop_continue_id|,
// which are not the current merge node, then we continue the search
// with the other target.
for (uint32_t i = 1; i < 3; i++) {
if (branch->GetSingleWordInOperand(i) == loop_merge_id &&
loop_merge_id != merge_block_id) {
next_block_id = branch->GetSingleWordInOperand(3 - i);
break;
}
if (branch->GetSingleWordInOperand(i) == loop_continue_id &&
loop_continue_id != merge_block_id) {
next_block_id = branch->GetSingleWordInOperand(3 - i);
break;
}
}
if (next_block_id == 0) {
@ -472,10 +480,11 @@ Instruction* DeadBranchElimPass::FindFirstExitFromSelectionMerge(
case SpvOpSwitch:
next_block_id = start_block->MergeBlockIdIfAny();
if (next_block_id == 0) {
// A switch with no merge instructions can have at most 3 targets:
// a. merge_block_id
// b. loop_merge_id
// c. 1 block inside the current region.
// A switch with no merge instructions can have at most 4 targets:
// a. |merge_block_id|
// b. |loop_merge_id|
// c. |loop_continue_id|
// d. 1 block inside the current region.
//
// This leads to a number of cases of what to do.
//
@ -495,7 +504,7 @@ Instruction* DeadBranchElimPass::FindFirstExitFromSelectionMerge(
uint32_t target = branch->GetSingleWordInOperand(i);
if (target == merge_block_id) {
found_break = true;
} else if (target != loop_merge_id) {
} else if (target != loop_merge_id && target != loop_continue_id) {
next_block_id = branch->GetSingleWordInOperand(i);
}
}
@ -519,12 +528,6 @@ Instruction* DeadBranchElimPass::FindFirstExitFromSelectionMerge(
next_block_id = start_block->MergeBlockIdIfAny();
if (next_block_id == 0) {
next_block_id = branch->GetSingleWordInOperand(0);
if (next_block_id == loop_merge_id) {
// We break out of the selection, but to the merge of a loop
// containing the selection. There is no break to the merge of the
// selection construct.
return nullptr;
}
}
break;
default:

7
source/opt/dead_branch_elim_pass.h
View File

@ -140,11 +140,12 @@ class DeadBranchElimPass : public MemPass {
// |start_block_id| must be a block whose innermost containing merge construct
// has |merge_block_id| as the merge block.
//
// |loop_merge_id| is the merge block id of the innermost loop containing
// |start_block_id|.
// |loop_merge_id| and |loop_continue_id| are the merge and continue block ids
// of the innermost loop containing |start_block_id|.
Instruction* FindFirstExitFromSelectionMerge(uint32_t start_block_id,
uint32_t merge_block_id,
uint32_t loop_merge_id);
uint32_t loop_merge_id,
uint32_t loop_continue_id);
};
} // namespace opt

313
test/opt/dead_branch_elim_test.cpp
View File

@ -2315,8 +2315,71 @@ OpFunctionEnd
SinglePassRunAndMatch<DeadBranchElimPass>(predefs + body, true);
}
TEST_F(DeadBranchElimTest, SelectionMergeWithExitToLoopContinue) {
// Checks that if a selection merge construct contains a conditional branch
// to continue of a loop surrounding the selection merge, then we do not keep
// the OpSelectionMerge instruction.
const std::string predefs = R"(
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %main "main"
OpExecutionMode %main OriginUpperLeft
OpSource GLSL 140
%void = OpTypeVoid
%func_type = OpTypeFunction %void
%bool = OpTypeBool
%true = OpConstantTrue %bool
%undef_bool = OpUndef %bool
)";
const std::string body =
R"(;
; CHECK: OpLabel
; CHECK: [[loop_header:%\w+]] = OpLabel
; CHECK: OpLoopMerge [[loop_merge:%\w+]] [[loop_cont:%\w+]]
; CHECK-NEXT: OpBranch [[bb1:%\w+]]
; CHECK: [[bb1]] = OpLabel
; CHECK-NEXT: OpBranch [[bb2:%\w+]]
; CHECK: [[bb2]] = OpLabel
; CHECK-NEXT: OpBranchConditional {{%\w+}} [[bb3:%\w+]] [[loop_cont]]
; CHECK: [[bb3]] = OpLabel
; CHECK-NEXT: OpBranch [[sel_merge:%\w+]]
; CHECK: [[sel_merge]] = OpLabel
; CHECK-NEXT: OpBranch [[loop_merge]]
; CHECK: [[loop_cont]] = OpLabel
; CHECK-NEXT: OpBranch [[loop_header]]
; CHECK: [[loop_merge]] = OpLabel
; CHECK-NEXT: OpReturn
%main = OpFunction %void None %func_type
%entry_bb = OpLabel
OpBranch %loop_header
%loop_header = OpLabel
OpLoopMerge %loop_merge %cont None
OpBranch %bb1
%bb1 = OpLabel
OpSelectionMerge %sel_merge None
OpBranchConditional %true %bb2 %bb4
%bb2 = OpLabel
OpBranchConditional %undef_bool %bb3 %cont
%bb3 = OpLabel
OpBranch %sel_merge
%bb4 = OpLabel
OpBranch %sel_merge
%sel_merge = OpLabel
OpBranch %loop_merge
%cont = OpLabel
OpBranch %loop_header
%loop_merge = OpLabel
OpReturn
OpFunctionEnd
)";
SinglePassRunAndMatch<DeadBranchElimPass>(predefs + body, true);
}
TEST_F(DeadBranchElimTest, SelectionMergeWithExitToLoop2) {
// Same as |SelectionMergeWithExitToLoop|, except the swith goes to the loop
// Same as |SelectionMergeWithExitToLoop|, except the switch goes to the loop
// merge or the selection merge. In this case, we do not need an
// OpSelectionMerge either.
const std::string predefs = R"(
@ -2370,6 +2433,65 @@ OpFunctionEnd
SinglePassRunAndMatch<DeadBranchElimPass>(predefs + body, true);
}
TEST_F(DeadBranchElimTest, SelectionMergeWithExitToLoopContinue2) {
// Same as |SelectionMergeWithExitToLoopContinue|, except the branch goes to
// the loop continue or the selection merge. In this case, we do not need an
// OpSelectionMerge either.
const std::string predefs = R"(
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %main "main"
OpExecutionMode %main OriginUpperLeft
OpSource GLSL 140
%void = OpTypeVoid
%func_type = OpTypeFunction %void
%bool = OpTypeBool
%true = OpConstantTrue %bool
%undef_bool = OpUndef %bool
)";
const std::string body =
R"(
; CHECK: OpLabel
; CHECK: [[loop_header:%\w+]] = OpLabel
; CHECK: OpLoopMerge [[loop_merge:%\w+]] [[loop_cont:%\w+]]
; CHECK-NEXT: OpBranch [[bb1:%\w+]]
; CHECK: [[bb1]] = OpLabel
; CHECK-NEXT: OpBranch [[bb2:%\w+]]
; CHECK: [[bb2]] = OpLabel
; CHECK-NEXT: OpBranchConditional {{%\w+}} [[sel_merge:%\w+]] [[loop_cont]]
; CHECK: [[sel_merge]] = OpLabel
; CHECK-NEXT: OpBranch [[loop_merge]]
; CHECK: [[loop_cont]] = OpLabel
; CHECK: OpBranch [[loop_header]]
; CHECK: [[loop_merge]] = OpLabel
; CHECK-NEXT: OpReturn
%main = OpFunction %void None %func_type
%entry_bb = OpLabel
OpBranch %loop_header
%loop_header = OpLabel
OpLoopMerge %loop_merge %cont None
OpBranch %bb1
%bb1 = OpLabel
OpSelectionMerge %sel_merge None
OpBranchConditional %true %bb2 %bb4
%bb2 = OpLabel
OpBranchConditional %undef_bool %sel_merge %cont
%bb4 = OpLabel
OpBranch %sel_merge
%sel_merge = OpLabel
OpBranch %loop_merge
%cont = OpLabel
OpBranch %loop_header
%loop_merge = OpLabel
OpReturn
OpFunctionEnd
)";
SinglePassRunAndMatch<DeadBranchElimPass>(predefs + body, true);
}
TEST_F(DeadBranchElimTest, SelectionMergeWithExitToLoop3) {
// Checks that if a selection merge construct contains a conditional branch
// to the merge of a surrounding loop, the selection merge, and another block
@ -2432,8 +2554,74 @@ OpFunctionEnd
SinglePassRunAndMatch<DeadBranchElimPass>(predefs + body, true);
}
TEST_F(DeadBranchElimTest, SelectionMergeWithExitToLoopContinue3) {
// Checks that if a selection merge construct contains a conditional branch
// to the merge of a surrounding loop, the selection merge, and another block
// inside the selection merge, then we must keep the OpSelectionMerge
// instruction on that branch.
const std::string predefs = R"(
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %main "main"
OpExecutionMode %main OriginUpperLeft
OpSource GLSL 140
%void = OpTypeVoid
%func_type = OpTypeFunction %void
%bool = OpTypeBool
%true = OpConstantTrue %bool
%uint = OpTypeInt 32 0
%undef_int = OpUndef %uint
)";
const std::string body =
R"(
; CHECK: OpLabel
; CHECK: [[loop_header:%\w+]] = OpLabel
; CHECK: OpLoopMerge [[loop_merge:%\w+]] [[loop_continue:%\w+]]
; CHECK-NEXT: OpBranch [[bb1:%\w+]]
; CHECK: [[bb1]] = OpLabel
; CHECK-NEXT: OpBranch [[bb2:%\w+]]
; CHECK: [[bb2]] = OpLabel
; CHECK-NEXT: OpSelectionMerge [[sel_merge:%\w+]] None
; CHECK-NEXT: OpSwitch {{%\w+}} [[sel_merge]] 0 [[loop_continue]] 1 [[bb3:%\w+]]
; CHECK: [[bb3]] = OpLabel
; CHECK-NEXT: OpBranch [[sel_merge]]
; CHECK: [[sel_merge]] = OpLabel
; CHECK-NEXT: OpBranch [[loop_merge]]
; CHECK: [[loop_continue]] = OpLabel
; CHECK-NEXT: OpBranch [[loop_header]]
; CHECK: [[loop_merge]] = OpLabel
; CHECK-NEXT: OpReturn
%main = OpFunction %void None %func_type
%entry_bb = OpLabel
OpBranch %loop_header
%loop_header = OpLabel
OpLoopMerge %loop_merge %cont None
OpBranch %bb1
%bb1 = OpLabel
OpSelectionMerge %sel_merge None
OpBranchConditional %true %bb2 %bb4
%bb2 = OpLabel
OpSwitch %undef_int %sel_merge 0 %cont 1 %bb3
%bb3 = OpLabel
OpBranch %sel_merge
%bb4 = OpLabel
OpBranch %sel_merge
%sel_merge = OpLabel
OpBranch %loop_merge
%cont = OpLabel
OpBranch %loop_header
%loop_merge = OpLabel
OpReturn
OpFunctionEnd
)";
SinglePassRunAndMatch<DeadBranchElimPass>(predefs + body, true);
}
TEST_F(DeadBranchElimTest, SelectionMergeWithExitToLoop4) {
// Same as |SelectionMergeWithExitToLoop|, execept the branch in the selection
// Same as |SelectionMergeWithExitToLoop|, except the branch in the selection
// construct is an |OpSwitch| instead of an |OpConditionalBranch|. The
// OpSelectionMerge instruction is not needed in this case either.
const std::string predefs = R"(
@ -2492,6 +2680,127 @@ OpFunctionEnd
SinglePassRunAndMatch<DeadBranchElimPass>(predefs + body, true);
}
TEST_F(DeadBranchElimTest, SelectionMergeWithExitToLoopContinue4) {
// Same as |SelectionMergeWithExitToLoopContinue|, except the branch in the
// selection construct is an |OpSwitch| instead of an |OpConditionalBranch|.
// The OpSelectionMerge instruction is not needed in this case either.
const std::string predefs = R"(
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %main "main"
OpExecutionMode %main OriginUpperLeft
OpSource GLSL 140
%void = OpTypeVoid
%func_type = OpTypeFunction %void
%bool = OpTypeBool
%true = OpConstantTrue %bool
%uint = OpTypeInt 32 0
%undef_int = OpUndef %uint
)";
const std::string body =
R"(
; CHECK: OpLoopMerge [[loop_merge:%\w+]] [[loop_cont:%\w+]]
; CHECK-NEXT: OpBranch [[bb1:%\w+]]
; CHECK: [[bb1]] = OpLabel
; CHECK-NEXT: OpBranch [[bb2:%\w+]]
; CHECK: [[bb2]] = OpLabel
; CHECK-NEXT: OpSwitch {{%\w+}} [[bb3:%\w+]] 0 [[loop_cont]] 1 [[bb3:%\w+]]
; CHECK: [[bb3]] = OpLabel
; CHECK-NEXT: OpBranch [[sel_merge:%\w+]]
; CHECK: [[sel_merge]] = OpLabel
; CHECK-NEXT: OpBranch [[loop_merge]]
; CHECK: [[loop_merge]] = OpLabel
; CHECK-NEXT: OpReturn
%main = OpFunction %void None %func_type
%entry_bb = OpLabel
OpBranch %loop_header
%loop_header = OpLabel
OpLoopMerge %loop_merge %cont None
OpBranch %bb1
%bb1 = OpLabel
OpSelectionMerge %sel_merge None
OpBranchConditional %true %bb2 %bb4
%bb2 = OpLabel
OpSwitch %undef_int %bb3 0 %cont 1 %bb3
%bb3 = OpLabel
OpBranch %sel_merge
%bb4 = OpLabel
OpBranch %sel_merge
%sel_merge = OpLabel
OpBranch %loop_merge
%cont = OpLabel
OpBranch %loop_header
%loop_merge = OpLabel
OpReturn
OpFunctionEnd
)";
SinglePassRunAndMatch<DeadBranchElimPass>(predefs + body, true);
}
TEST_F(DeadBranchElimTest, SelectionMergeSameAsLoopContinue) {
// Same as |SelectionMergeWithExitToLoopContinue|, except the branch in the
// selection construct is an |OpSwitch| instead of an |OpConditionalBranch|.
// The OpSelectionMerge instruction is not needed in this case either.
const std::string predefs = R"(
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %main "main"
OpExecutionMode %main OriginUpperLeft
OpSource GLSL 140
%void = OpTypeVoid
%func_type = OpTypeFunction %void
%bool = OpTypeBool
%true = OpConstantTrue %bool
%uint = OpTypeInt 32 0
%undef_bool = OpUndef %bool
)";
const std::string body =
R"(
; CHECK: OpLabel
; CHECK: [[loop_header:%\w+]] = OpLabel
; CHECK: OpLoopMerge [[loop_merge:%\w+]] [[loop_cont:%\w+]]
; CHECK-NEXT: OpBranch [[bb1:%\w+]]
; CHECK: [[bb1]] = OpLabel
; CHECK-NEXT: OpBranch [[bb2:%\w+]]
; CHECK: [[bb2]] = OpLabel
; CHECK-NEXT: OpSelectionMerge [[loop_cont]]
; CHECK-NEXT: OpBranchConditional {{%\w+}} [[bb3:%\w+]] [[loop_cont]]
; CHECK: [[bb3]] = OpLabel
; CHECK-NEXT: OpBranch [[loop_cont]]
; CHECK: [[loop_cont]] = OpLabel
; CHECK-NEXT: OpBranchConditional {{%\w+}} [[loop_header]] [[loop_merge]]
; CHECK: [[loop_merge]] = OpLabel
; CHECK-NEXT: OpReturn
%main = OpFunction %void None %func_type
%entry_bb = OpLabel
OpBranch %loop_header
%loop_header = OpLabel
OpLoopMerge %loop_merge %cont None
OpBranch %bb1
%bb1 = OpLabel
OpSelectionMerge %cont None
OpBranchConditional %true %bb2 %bb4
%bb2 = OpLabel
OpBranchConditional %undef_bool %bb3 %cont
%bb3 = OpLabel
OpBranch %cont
%bb4 = OpLabel
OpBranch %cont
%cont = OpLabel
OpBranchConditional %undef_bool %loop_header %loop_merge
%loop_merge = OpLabel
OpReturn
OpFunctionEnd
)";
SinglePassRunAndMatch<DeadBranchElimPass>(predefs + body, true);
}
TEST_F(DeadBranchElimTest, SelectionMergeWithNestedLoop) {
const std::string body =
R"(