Prevent eliminating case constructs in block merging (#4976)
Fixes #4918
* Prevent block merging from producing an invalid case construct by
merging a switch target/default with another construct's merge or
continue block
* This is to satisfy the structural dominance requirement between the
switch header and the case constructs
diff --git a/source/opt/block_merge_util.cpp b/source/opt/block_merge_util.cpp
index 8ae8020..83c702c 100644
--- a/source/opt/block_merge_util.cpp
+++ b/source/opt/block_merge_util.cpp
@@ -125,6 +125,26 @@
return false;
}
}
+
+ if (succ_is_merge || IsContinue(context, lab_id)) {
+ auto* struct_cfg = context->GetStructuredCFGAnalysis();
+ auto switch_block_id = struct_cfg->ContainingSwitch(block->id());
+ if (switch_block_id) {
+ auto switch_merge_id = struct_cfg->SwitchMergeBlock(switch_block_id);
+ const auto* switch_inst =
+ &*block->GetParent()->FindBlock(switch_block_id)->tail();
+ for (uint32_t i = 1; i < switch_inst->NumInOperands(); i += 2) {
+ auto target_id = switch_inst->GetSingleWordInOperand(i);
+ if (target_id == block->id() && target_id != switch_merge_id) {
+ // Case constructs must be structurally dominated by the OpSwitch.
+ // Since the successor is the merge/continue for another construct,
+ // merging the blocks would break that requirement.
+ return false;
+ }
+ }
+ }
+ }
+
return true;
}
diff --git a/test/opt/block_merge_test.cpp b/test/opt/block_merge_test.cpp
index 9698fed..6129bb2 100644
--- a/test/opt/block_merge_test.cpp
+++ b/test/opt/block_merge_test.cpp
@@ -1201,6 +1201,92 @@
SinglePassRunAndMatch<BlockMergePass>(text, true);
}
+TEST_F(BlockMergeTest, DontLoseCaseConstruct) {
+ const std::string text = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint GLCompute %func "func"
+OpExecutionMode %func LocalSize 1 1 1
+OpName %entry "entry";
+OpName %loop "loop"
+OpName %loop_merge "loop_merge"
+OpName %loop_cont "loop_cont"
+OpName %switch "switch"
+OpName %switch_merge "switch_merge"
+%void = OpTypeVoid
+%bool = OpTypeBool
+%int = OpTypeInt 32 0
+%void_fn = OpTypeFunction %void
+%bool_undef = OpUndef %bool
+%int_undef = OpUndef %int
+%func = OpFunction %void None %void_fn
+%entry = OpLabel
+OpBranch %loop
+%loop = OpLabel
+OpLoopMerge %loop_merge %loop_cont None
+OpBranch %switch
+%switch = OpLabel
+OpSelectionMerge %switch_merge None
+OpSwitch %int_undef %switch_merge 0 %break 1 %break
+%break = OpLabel
+OpBranch %loop_merge
+%switch_merge = OpLabel
+OpBranch %loop_cont
+%loop_cont = OpLabel
+OpBranch %loop
+%loop_merge = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+ auto result = SinglePassRunAndDisassemble<opt::BlockMergePass>(
+ text, /* skip_nop = */ true, /* do_validation = */ true);
+ EXPECT_EQ(opt::Pass::Status::SuccessWithoutChange, std::get<1>(result));
+}
+
+TEST_F(BlockMergeTest, DontLoseDefaultCaseConstruct) {
+ const std::string text = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint GLCompute %func "func"
+OpExecutionMode %func LocalSize 1 1 1
+OpName %entry "entry";
+OpName %loop "loop"
+OpName %loop_merge "loop_merge"
+OpName %loop_cont "loop_cont"
+OpName %switch "switch"
+OpName %switch_merge "switch_merge"
+%void = OpTypeVoid
+%bool = OpTypeBool
+%int = OpTypeInt 32 0
+%void_fn = OpTypeFunction %void
+%bool_undef = OpUndef %bool
+%int_undef = OpUndef %int
+%func = OpFunction %void None %void_fn
+%entry = OpLabel
+OpBranch %loop
+%loop = OpLabel
+OpLoopMerge %loop_merge %loop_cont None
+OpBranch %switch
+%switch = OpLabel
+OpSelectionMerge %switch_merge None
+OpSwitch %int_undef %cont 0 %switch_merge 1 %switch_merge
+%cont = OpLabel
+OpBranch %loop_cont
+%switch_merge = OpLabel
+OpBranch %loop_merge
+%loop_cont = OpLabel
+OpBranch %loop
+%loop_merge = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+ auto result = SinglePassRunAndDisassemble<opt::BlockMergePass>(
+ text, /* skip_nop = */ true, /* do_validation = */ true);
+ EXPECT_EQ(opt::Pass::Status::SuccessWithoutChange, std::get<1>(result));
+}
+
// TODO(greg-lunarg): Add tests to verify handling of these cases:
//
// More complex control flow
