Fix block depth rule priority (#2755)
Fixes #2743
* Continue depth calculation should take precedence over merge calculation
diff --git a/source/val/function.cpp b/source/val/function.cpp
index af1498d..876a608 100644
--- a/source/val/function.cpp
+++ b/source/val/function.cpp
@@ -317,13 +317,10 @@
if (!bb_dom || bb == bb_dom) {
// This block has no dominator, so it's at depth 0.
block_depth_[bb] = 0;
- } else if (bb->is_type(kBlockTypeMerge)) {
- // If this is a merge block, its depth is equal to the block before
- // branching.
- BasicBlock* header = merge_block_header_[bb];
- assert(header);
- block_depth_[bb] = GetBlockDepth(header);
} else if (bb->is_type(kBlockTypeContinue)) {
+ // This rule must precede the rule for merge blocks in order to set up
+ // depths correctly. If a block is both a merge and continue then the merge
+ // is nested within the continue's loop (or the graph is incorrect).
// The depth of the continue block entry point is 1 + loop header depth.
Construct* continue_construct =
entry_block_to_construct_[std::make_pair(bb, ConstructType::kContinue)];
@@ -341,6 +338,12 @@
} else {
block_depth_[bb] = 1 + GetBlockDepth(loop_header);
}
+ } else if (bb->is_type(kBlockTypeMerge)) {
+ // If this is a merge block, its depth is equal to the block before
+ // branching.
+ BasicBlock* header = merge_block_header_[bb];
+ assert(header);
+ block_depth_[bb] = GetBlockDepth(header);
} else if (bb_dom->is_type(kBlockTypeHeader) ||
bb_dom->is_type(kBlockTypeLoop)) {
// The dominator of the given block is a header block. So, the nesting
diff --git a/test/val/val_cfg_test.cpp b/test/val/val_cfg_test.cpp
index b1857cf..b22db06 100644
--- a/test/val/val_cfg_test.cpp
+++ b/test/val/val_cfg_test.cpp
@@ -3703,6 +3703,52 @@
"10[%10], but not via a structured exit"));
}
+TEST_F(ValidateCFG, ContinueFromNestedSelection) {
+ const std::string text = R"(
+OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+%void = OpTypeVoid
+%void_fn = OpTypeFunction %void
+%bool = OpTypeBool
+%undef = OpUndef %bool
+%4 = OpFunction %void None %void_fn
+%5 = OpLabel
+OpBranch %48
+%48 = OpLabel
+OpLoopMerge %47 %50 None
+OpBranch %10
+%10 = OpLabel
+OpLoopMerge %12 %37 None
+OpBranchConditional %undef %11 %12
+%11 = OpLabel
+OpSelectionMerge %31 None
+OpBranchConditional %undef %30 %31
+%30 = OpLabel
+OpSelectionMerge %37 None
+OpBranchConditional %undef %36 %37
+%36 = OpLabel
+OpBranch %37
+%37 = OpLabel
+OpBranch %10
+%31 = OpLabel
+OpBranch %12
+%12 = OpLabel
+OpSelectionMerge %55 None
+OpBranchConditional %undef %47 %55
+%55 = OpLabel
+OpBranch %47
+%50 = OpLabel
+OpBranch %48
+%47 = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(text);
+ EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
+}
+
/// TODO(umar): Nested CFG constructs
} // namespace
