Allows breaks selection breaks to switches (#2605)
Fixes #2604
* Allow selection constructs to branch to the nearest selection merge
whose header is terminated by an OpSwitch
* Cleanup break and continue checks generally
* add tests
diff --git a/source/val/construct.cpp b/source/val/construct.cpp
index 7e106e6..1564449 100644
--- a/source/val/construct.cpp
+++ b/source/val/construct.cpp
@@ -133,6 +133,7 @@
// - Selection:
// - branch to its merge
// - branch to nearest enclosing loop merge or continue
+ // - branch to nearest enclosing switch selection merge
// - Loop:
// - branch to its merge
// - branch to its continue
@@ -168,13 +169,17 @@
return true;
}
+ bool seen_switch = false;
auto header = entry_block();
- auto block = header;
+ auto block = header->immediate_dominator();
while (block) {
auto terminator = block->terminator();
auto index = terminator - &_.ordered_instructions()[0];
auto merge_inst = &_.ordered_instructions()[index - 1];
- if (merge_inst->opcode() == SpvOpLoopMerge) {
+ if (merge_inst->opcode() == SpvOpLoopMerge ||
+ (header->terminator()->opcode() != SpvOpSwitch &&
+ merge_inst->opcode() == SpvOpSelectionMerge &&
+ terminator->opcode() == SpvOpSwitch)) {
auto merge_target = merge_inst->GetOperandAs<uint32_t>(0u);
auto merge_block = merge_inst->function()->GetBlock(merge_target).first;
if (merge_block->dominates(*header)) {
@@ -182,10 +187,20 @@
continue;
}
- auto continue_target = merge_inst->GetOperandAs<uint32_t>(1u);
- if (dest->id() == merge_target || dest->id() == continue_target) {
+ if ((!seen_switch || merge_inst->opcode() == SpvOpLoopMerge) &&
+ dest->id() == merge_target) {
return true;
+ } else if (merge_inst->opcode() == SpvOpLoopMerge) {
+ auto continue_target = merge_inst->GetOperandAs<uint32_t>(1u);
+ if (dest->id() == continue_target) {
+ return true;
+ }
}
+
+ if (terminator->opcode() == SpvOpSwitch) seen_switch = true;
+
+ // Hit an enclosing loop and didn't break or continue.
+ if (merge_inst->opcode() == SpvOpLoopMerge) return false;
}
block = block->immediate_dominator();
diff --git a/source/val/construct.h b/source/val/construct.h
index 172976d..9476760 100644
--- a/source/val/construct.h
+++ b/source/val/construct.h
@@ -116,7 +116,9 @@
// * branch to the associated merge
// * branch to the merge or continue of the innermost loop containing the
// selection
- // Loop:
+ // * branch to the merge block of the innermost switch containing the
+ // selection
+ // Loop:
// * branch to the associated merge or continue
// Continue:
// * back-edge to the associated loop header
diff --git a/test/val/val_cfg_test.cpp b/test/val/val_cfg_test.cpp
index 0bb8fec..73b2bbc 100644
--- a/test/val/val_cfg_test.cpp
+++ b/test/val/val_cfg_test.cpp
@@ -3403,6 +3403,149 @@
"9[%9], but not via a structured exit"));
}
+TEST_F(ValidateCFG, BreakFromSwitch) {
+ const std::string text = R"(
+OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+%1 = OpTypeVoid
+%2 = OpTypeBool
+%3 = OpTypeInt 32 0
+%4 = OpUndef %2
+%5 = OpUndef %3
+%6 = OpTypeFunction %1
+%7 = OpFunction %1 None %6
+%8 = OpLabel
+OpSelectionMerge %9 None
+OpSwitch %5 %9 0 %10
+%10 = OpLabel
+OpSelectionMerge %11 None
+OpBranchConditional %4 %11 %12
+%12 = OpLabel
+OpBranch %9
+%11 = OpLabel
+OpBranch %9
+%9 = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(text);
+ EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
+}
+
+TEST_F(ValidateCFG, InvalidBreakFromSwitch) {
+ const std::string text = R"(
+OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+%1 = OpTypeVoid
+%2 = OpTypeBool
+%3 = OpTypeInt 32 0
+%4 = OpUndef %2
+%5 = OpUndef %3
+%6 = OpTypeFunction %1
+%7 = OpFunction %1 None %6
+%8 = OpLabel
+OpSelectionMerge %9 None
+OpSwitch %5 %9 0 %10
+%10 = OpLabel
+OpSelectionMerge %11 None
+OpSwitch %5 %11 0 %12
+%12 = OpLabel
+OpBranch %9
+%11 = OpLabel
+OpBranch %9
+%9 = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(text);
+ EXPECT_EQ(SPV_ERROR_INVALID_CFG, ValidateInstructions());
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("block <ID> 12[%12] exits the selection headed by <ID> "
+ "10[%10], but not via a structured exit"));
+}
+
+TEST_F(ValidateCFG, BreakToOuterSwitch) {
+ const std::string text = R"(
+OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+%1 = OpTypeVoid
+%2 = OpTypeBool
+%3 = OpTypeInt 32 0
+%4 = OpUndef %2
+%5 = OpUndef %3
+%6 = OpTypeFunction %1
+%7 = OpFunction %1 None %6
+%8 = OpLabel
+OpSelectionMerge %9 None
+OpSwitch %5 %9 0 %10
+%10 = OpLabel
+OpSelectionMerge %11 None
+OpSwitch %5 %11 0 %12
+%12 = OpLabel
+OpSelectionMerge %13 None
+OpBranchConditional %4 %13 %14
+%14 = OpLabel
+OpBranch %9
+%13 = OpLabel
+OpBranch %11
+%11 = OpLabel
+OpBranch %9
+%9 = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(text);
+ EXPECT_EQ(SPV_ERROR_INVALID_CFG, ValidateInstructions());
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("block <ID> 14[%14] exits the selection headed by <ID> "
+ "10[%10], but not via a structured exit"));
+}
+
+TEST_F(ValidateCFG, BreakToOuterLoop) {
+ const std::string text = R"(
+OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+%1 = OpTypeVoid
+%2 = OpTypeBool
+%3 = OpUndef %2
+%4 = OpTypeFunction %1
+%5 = OpFunction %1 None %4
+%6 = OpLabel
+OpBranch %7
+%7 = OpLabel
+OpLoopMerge %8 %9 None
+OpBranch %10
+%10 = OpLabel
+OpLoopMerge %9 %11 None
+OpBranch %12
+%12 = OpLabel
+OpSelectionMerge %11 None
+OpBranchConditional %3 %11 %13
+%13 = OpLabel
+OpBranch %8
+%11 = OpLabel
+OpBranchConditional %3 %9 %10
+%9 = OpLabel
+OpBranchConditional %3 %8 %7
+%8 = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(text);
+ EXPECT_EQ(SPV_ERROR_INVALID_CFG, ValidateInstructions());
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("block <ID> 13[%13] exits the loop headed by <ID> "
+ "10[%10], but not via a structured exit"));
+}
+
/// TODO(umar): Nested CFG constructs
} // namespace
