Fix bug in construct block calculation (#1964)
Fixes #1960
* Only allows blocks that are dominated by the header
* Fixed a bad loop fusion test
* Added a test derived from the reported bug
diff --git a/source/val/construct.cpp b/source/val/construct.cpp
index c11a065..7b0cb2d 100644
--- a/source/val/construct.cpp
+++ b/source/val/construct.cpp
@@ -116,7 +116,10 @@
if (merge != block) {
for (auto succ : *block->successors()) {
- stack.push_back(succ);
+ // All blocks in the construct must be dominated by the header.
+ if (header->dominates(*succ)) {
+ stack.push_back(succ);
+ }
}
}
}
diff --git a/test/opt/loop_optimizations/fusion_legal.cpp b/test/opt/loop_optimizations/fusion_legal.cpp
index db7f61c..41d796f 100644
--- a/test/opt/loop_optimizations/fusion_legal.cpp
+++ b/test/opt/loop_optimizations/fusion_legal.cpp
@@ -3177,7 +3177,7 @@
%21 = OpLabel
%29 = OpSMod %6 %96 %28
%30 = OpIEqual %17 %29 %9
- OpSelectionMerge %32 None
+ OpSelectionMerge %23 None
OpBranchConditional %30 %31 %48
%31 = OpLabel
%44 = OpAccessChain %7 %41 %91 %96
@@ -3224,7 +3224,7 @@
%72 = OpSMod %6 %93 %28
%73 = OpIEqual %17 %72 %9
OpSelectionMerge %75 None
- OpBranchConditional %73 %74 %85
+ OpBranchConditional %73 %74 %66
%74 = OpLabel
%81 = OpAccessChain %7 %38 %92 %93
%82 = OpLoad %6 %81
@@ -3232,8 +3232,6 @@
%84 = OpAccessChain %7 %76 %92 %93
OpStore %84 %83
OpBranch %75
- %85 = OpLabel
- OpBranch %66
%75 = OpLabel
OpBranch %67
%67 = OpLabel
diff --git a/test/val/val_cfg_test.cpp b/test/val/val_cfg_test.cpp
index bdfb366..d45fd10 100644
--- a/test/val/val_cfg_test.cpp
+++ b/test/val/val_cfg_test.cpp
@@ -1846,6 +1846,55 @@
ASSERT_EQ(SPV_SUCCESS, ValidateInstructions());
}
+TEST_F(ValidateCFG, GoodUnreachableSelection) {
+ const std::string text = R"(
+OpCapability Shader
+%1 = OpExtInstImport "GLSL.std.450"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main"
+OpExecutionMode %main OriginUpperLeft
+%void = OpTypeVoid
+%8 = OpTypeFunction %void
+%bool = OpTypeBool
+%false = OpConstantFalse %bool
+%main = OpFunction %void None %8
+%15 = OpLabel
+OpBranch %16
+%16 = OpLabel
+OpLoopMerge %17 %18 None
+OpBranch %19
+%19 = OpLabel
+OpBranchConditional %false %21 %17
+%21 = OpLabel
+OpSelectionMerge %22 None
+OpBranchConditional %false %23 %22
+%23 = OpLabel
+OpBranch %24
+%24 = OpLabel
+OpLoopMerge %25 %26 None
+OpBranch %27
+%27 = OpLabel
+OpReturn
+%26 = OpLabel
+OpBranchConditional %false %24 %25
+%25 = OpLabel
+OpSelectionMerge %28 None
+OpBranchConditional %false %18 %28
+%28 = OpLabel
+OpBranch %22
+%22 = OpLabel
+OpBranch %18
+%18 = OpLabel
+OpBranch %16
+%17 = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(text);
+ ASSERT_EQ(SPV_SUCCESS, ValidateInstructions());
+}
+
/// TODO(umar): Nested CFG constructs
} // namespace
