Have dead-branch-elim handle conditional exits from selections. (#1850)

When dead-branch-elim folds a conditional branch, it also deletes the
OpSelectionMerge instruction.  If that construct contains a
conditional branch to the merge node, it will not have its own
OpSelectionMerge.  When the headers merge instruction is deleted, the
the inner conditional branch will no longer be legal.  It will be a
selection to a node that is not a merge node.

We fix this up by moving the OpSelectionMerge to a new location if it is
still needed.
This commit is contained in:
Steven Perron 2018-08-21 11:49:56 -04:00 committed by GitHub
parent 87f363636e
commit 45c235d41f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 149 additions and 1 deletions

View File

@ -159,7 +159,16 @@ bool DeadBranchElimPass::MarkLiveBlocks(
context()->KillInst(terminator);
Instruction* mergeInst = block->GetMergeInst();
if (mergeInst && mergeInst->opcode() == SpvOpSelectionMerge) {
context()->KillInst(mergeInst);
Instruction* first_break = FindFirstExitFromSelectionMerge(
live_lab_id, mergeInst->GetSingleWordInOperand(0));
if (first_break == nullptr) {
context()->KillInst(mergeInst);
} else {
mergeInst->RemoveFromList();
first_break->InsertBefore(std::unique_ptr<Instruction>(mergeInst));
context()->set_instr_block(mergeInst,
context()->get_instr_block(first_break));
}
}
stack.push_back(GetParentBlock(live_lab_id));
} else {
@ -425,5 +434,34 @@ Pass::Status DeadBranchElimPass::Process() {
return modified ? Status::SuccessWithChange : Status::SuccessWithoutChange;
}
Instruction* DeadBranchElimPass::FindFirstExitFromSelectionMerge(
uint32_t start_block_id, uint32_t merge_block_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) {
BasicBlock* start_block = context()->get_instr_block(start_block_id);
Instruction* branch = start_block->terminator();
uint32_t next_block_id = 0;
switch (branch->opcode()) {
case SpvOpBranchConditional:
case SpvOpSwitch:
next_block_id = start_block->MergeBlockIdIfAny();
if (next_block_id == 0) {
return branch;
}
break;
case SpvOpBranch:
next_block_id = branch->GetSingleWordInOperand(0);
break;
default:
return nullptr;
}
start_block_id = next_block_id;
}
return nullptr;
}
} // namespace opt
} // namespace spvtools

View File

@ -130,6 +130,17 @@ class DeadBranchElimPass : public MemPass {
// Reorders blocks in reachable functions so that they satisfy dominator
// block ordering rules.
void FixBlockOrder();
// Return the first branch instruction that is a conditional branch to
// |merge_block_id|. Returns |nullptr| if not such branch exists. If there are
// multiple such branches, the first one is the one that would be executed
// first when running the code. That is, the one that dominates all of the
// others.
//
// |start_block_id| must be a block whose innermost containing merge construct
// has |merge_block_id| as the merge block.
Instruction* FindFirstExitFromSelectionMerge(uint32_t start_block_id,
uint32_t merge_block_id);
};
} // namespace opt

View File

@ -2094,6 +2094,105 @@ OpFunctionEnd
SinglePassRunAndMatch<DeadBranchElimPass>(text, true);
}
TEST_F(DeadBranchElimTest, SelectionMergeWithEarlyExit1) {
// Checks that if a selection merge construct contains a conditional branch
// to the merge node, then the OpSelectionMerge instruction is positioned
// correctly.
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: OpFunction
; CHECK-NEXT: OpLabel
; CHECK-NEXT: OpBranch [[taken_branch:%\w+]]
; CHECK-NEXT: [[taken_branch]] = OpLabel
; CHECK-NEXT: OpSelectionMerge [[merge:%\w+]]
; CHECK-NEXT: OpBranchConditional {{%\w+}} [[merge]] {{%\w+}}
%main = OpFunction %void None %func_type
%entry_bb = OpLabel
OpSelectionMerge %outer_merge None
OpBranchConditional %true %bb1 %bb3
%bb1 = OpLabel
OpBranchConditional %undef_bool %outer_merge %bb2
%bb2 = OpLabel
OpBranch %outer_merge
%bb3 = OpLabel
OpBranch %outer_merge
%outer_merge = OpLabel
OpReturn
OpFunctionEnd
)";
SinglePassRunAndMatch<DeadBranchElimPass>(predefs + body, true);
}
TEST_F(DeadBranchElimTest, SelectionMergeWithEarlyExit2) {
// Checks that if a selection merge construct contains a conditional branch
// to the merge node, then the OpSelectionMerge instruction is positioned
// correctly.
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: OpFunction
; CHECK-NEXT: OpLabel
; CHECK-NEXT: OpBranch [[bb1:%\w+]]
; CHECK-NEXT: [[bb1]] = OpLabel
; CHECK-NEXT: OpSelectionMerge [[inner_merge:%\w+]]
; CHECK: [[inner_merge]] = OpLabel
; CHECK-NEXT: OpSelectionMerge [[outer_merge:%\w+]]
; CHECK-NEXT: OpBranchConditional {{%\w+}} [[outer_merge]:%\w+]] {{%\w+}}
; CHECK: [[outer_merge]] = OpLabel
; CHECK-NEXT: OpReturn
%main = OpFunction %void None %func_type
%entry_bb = OpLabel
OpSelectionMerge %outer_merge None
OpBranchConditional %true %bb1 %bb5
%bb1 = OpLabel
OpSelectionMerge %inner_merge None
OpBranchConditional %undef_bool %bb2 %bb3
%bb2 = OpLabel
OpBranch %inner_merge
%bb3 = OpLabel
OpBranch %inner_merge
%inner_merge = OpLabel
OpBranchConditional %undef_bool %outer_merge %bb4
%bb4 = OpLabel
OpBranch %outer_merge
%bb5 = OpLabel
OpBranch %outer_merge
%outer_merge = OpLabel
OpReturn
OpFunctionEnd
)";
SinglePassRunAndMatch<DeadBranchElimPass>(predefs + body, true);
}
#endif
// TODO(greg-lunarg): Add tests to verify handling of these cases: