[spirv-reader] Systematic bad construct exit tests
Do so systmatically. Before we had tested some as a side effect
of other objectives.
Fix the error message for when we have a bad exit from a loop construct
that bypasses not only the continue construct but the loop merge block
itself.
Bug: tint:3
Change-Id: Iaf8fc9bcd3162002aa906efa90a244ef5f439911
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/21580
Reviewed-by: dan sinclair <dsinclair@google.com>
diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc
index b3f0c81..94aa686 100644
--- a/src/reader/spirv/function.cc
+++ b/src/reader/spirv/function.cc
@@ -1139,11 +1139,30 @@
(edge_kind == EdgeKind::kCaseFallThrough)) {
// Check for an invalid forward exit out of this construct.
if (dest_info->pos >= src_construct.end_pos) {
+ // In most cases we're bypassing the merge block for the source
+ // construct.
+ auto end_block = src_construct.end_id;
+ const char* end_block_desc = "merge block";
+ if (src_construct.kind == Construct::kLoop) {
+ // For a loop construct, we have two valid places to go: the
+ // continue target or the merge for the loop header, which is
+ // further down.
+ const auto loop_merge =
+ GetBlockInfo(src_construct.begin_id)->merge_for_header;
+ if (dest_info->pos >= GetBlockInfo(loop_merge)->pos) {
+ // We're bypassing the loop's merge block.
+ end_block = loop_merge;
+ } else {
+ // We're bypassing the loop's continue target, and going into
+ // the middle of the continue construct.
+ end_block_desc = "continue target";
+ }
+ }
return Fail()
<< "Branch from block " << src << " to block " << dest
<< " is an invalid exit from construct starting at block "
- << src_construct.begin_id << "; branch bypasses block "
- << src_construct.end_id;
+ << src_construct.begin_id << "; branch bypasses "
+ << end_block_desc << " " << end_block;
}
normal_forward_edges.push_back(dest);
diff --git a/src/reader/spirv/function_cfg_test.cc b/src/reader/spirv/function_cfg_test.cc
index 6026c75..cae468f 100644
--- a/src/reader/spirv/function_cfg_test.cc
+++ b/src/reader/spirv/function_cfg_test.cc
@@ -4066,7 +4066,7 @@
EXPECT_THAT(*(bi20->case_values.get()), UnorderedElementsAre(200));
}
-TEST_F(SpvParserTest, FindSwitchCaseHeaders_ManyCasesWithSameValue_Error) {
+TEST_F(SpvParserTest, FindSwitchCaseHeaders_ManyCasesWithSameValue_IsError) {
auto assembly = CommonTypes() + R"(
%100 = OpFunction %void None %voidfn
@@ -4618,6 +4618,32 @@
EXPECT_EQ(bi50->succ_edge[99], EdgeKind::kIfBreak);
}
+TEST_F(SpvParserTest, ClassifyCFGEdge_IfBreak_BypassesMerge_IsError) {
+ auto assembly = CommonTypes() + R"(
+ %100 = OpFunction %void None %voidfn
+
+ %10 = OpLabel
+ OpSelectionMerge %50 None
+ OpBranchConditional %cond %20 %50
+
+ %20 = OpLabel
+ OpBranch %99
+
+ %50 = OpLabel ; merge
+ OpBranch %99
+
+ %99 = OpLabel
+ OpReturn
+)";
+ auto* p = parser(test::Assemble(assembly));
+ ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
+ FunctionEmitter fe(p, *spirv_function(100));
+ EXPECT_FALSE(FlowClassifyCFGEdges(&fe));
+ EXPECT_THAT(p->error(),
+ Eq("Branch from block 20 to block 99 is an invalid exit from "
+ "construct starting at block 10; branch bypasses merge block 50"));
+}
+
TEST_F(SpvParserTest, ClassifyCFGEdges_SwitchBreak_FromSwitchCaseDirect) {
auto assembly = CommonTypes() + R"(
%100 = OpFunction %void None %voidfn
@@ -4785,6 +4811,32 @@
EXPECT_EQ(bi->succ_edge[99], EdgeKind::kSwitchBreak);
}
+TEST_F(SpvParserTest, ClassifyCFGEdges_SwitchBreak_BypassesMerge_IsError) {
+ auto assembly = CommonTypes() + R"(
+ %100 = OpFunction %void None %voidfn
+
+ %10 = OpLabel
+ OpSelectionMerge %50 None
+ OpSwitch %selector %50 20 %20
+
+ %20 = OpLabel
+ OpBranch %99 ; invalid exit
+
+ %50 = OpLabel ; switch merge
+ OpBranch %99
+
+ %99 = OpLabel
+ OpReturn
+)";
+ auto* p = parser(test::Assemble(assembly));
+ ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
+ FunctionEmitter fe(p, *spirv_function(100));
+ EXPECT_FALSE(FlowClassifyCFGEdges(&fe));
+ EXPECT_THAT(p->error(),
+ Eq("Branch from block 20 to block 99 is an invalid exit from "
+ "construct starting at block 10; branch bypasses merge block 50"));
+}
+
TEST_F(SpvParserTest, ClassifyCFGEdges_SwitchBreak_FromNestedLoop_IsError) {
// It's an error because the break can only go as far as the loop.
auto assembly = CommonTypes() + R"(
@@ -4816,7 +4868,7 @@
EXPECT_FALSE(FlowClassifyCFGEdges(&fe));
EXPECT_THAT(p->error(),
Eq("Branch from block 30 to block 99 is an invalid exit from "
- "construct starting at block 20; branch bypasses block 70"));
+ "construct starting at block 20; branch bypasses merge block 80"));
}
TEST_F(SpvParserTest, ClassifyCFGEdges_SwitchBreak_FromNestedSwitch_IsError) {
@@ -4847,7 +4899,7 @@
EXPECT_FALSE(FlowClassifyCFGEdges(&fe));
EXPECT_THAT(p->error(),
Eq("Branch from block 30 to block 99 is an invalid exit from "
- "construct starting at block 20; branch bypasses block 80"));
+ "construct starting at block 20; branch bypasses merge block 80"));
}
TEST_F(SpvParserTest, ClassifyCFGEdges_LoopBreak_FromLoopBody) {
@@ -5065,6 +5117,75 @@
"(violates post-dominance rule)"));
}
+TEST_F(SpvParserTest,
+ ClassifyCFGEdges_LoopBreak_FromLoopBypassesMerge_IsError) {
+ auto assembly = CommonTypes() + R"(
+ %100 = OpFunction %void None %voidfn
+
+ %10 = OpLabel
+ OpBranch %20
+
+ %20 = OpLabel
+ OpLoopMerge %50 %40 None
+ OpBranchConditional %cond %30 %50
+
+ %30 = OpLabel
+ OpBranch %99 ; bad exit
+
+ %40 = OpLabel ; continue construct
+ OpBranch %20
+
+ %50 = OpLabel
+ OpBranch %99
+
+ %99 = OpLabel
+ OpReturn
+)";
+ auto* p = parser(test::Assemble(assembly));
+ ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
+ FunctionEmitter fe(p, *spirv_function(100));
+ EXPECT_FALSE(FlowClassifyCFGEdges(&fe));
+ EXPECT_THAT(p->error(),
+ Eq("Branch from block 30 to block 99 is an invalid exit from "
+ "construct starting at block 20; branch bypasses merge block 50"));
+}
+
+TEST_F(SpvParserTest,
+ ClassifyCFGEdges_LoopBreak_FromContinueBypassesMerge_IsError) {
+ auto assembly = CommonTypes() + R"(
+ %100 = OpFunction %void None %voidfn
+
+ %10 = OpLabel
+ OpBranch %20
+
+ %20 = OpLabel
+ OpLoopMerge %50 %40 None
+ OpBranchConditional %cond %30 %50
+
+ %30 = OpLabel
+ OpBranch %40
+
+ %40 = OpLabel ; continue construct
+ OpBranch %45
+
+ %45 = OpLabel
+ OpBranchConditional %cond2 %20 %99 ; branch to %99 is bad exit
+
+ %50 = OpLabel
+ OpBranch %99
+
+ %99 = OpLabel
+ OpReturn
+)";
+ auto* p = parser(test::Assemble(assembly));
+ ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
+ FunctionEmitter fe(p, *spirv_function(100));
+ EXPECT_FALSE(FlowClassifyCFGEdges(&fe));
+ EXPECT_THAT(p->error(),
+ Eq("Branch from block 45 to block 99 is an invalid exit from "
+ "construct starting at block 40; branch bypasses merge block 50"));
+}
+
TEST_F(SpvParserTest, ClassifyCFGEdges_LoopContinue_LoopBodyToContinue) {
auto assembly = CommonTypes() + R"(
%100 = OpFunction %void None %voidfn
@@ -5212,7 +5333,7 @@
}
TEST_F(SpvParserTest,
- ClassifyCFGEdges_LoopContinue_FromNestedSwitchCaseDirect_Error) {
+ ClassifyCFGEdges_LoopContinue_FromNestedSwitchCaseDirect_IsError) {
auto assembly = CommonTypes() + R"(
%100 = OpFunction %void None %voidfn
@@ -5250,7 +5371,7 @@
}
TEST_F(SpvParserTest,
- ClassifyCFGEdges_LoopContinue_FromNestedSwitchDefaultDirect_Error) {
+ ClassifyCFGEdges_LoopContinue_FromNestedSwitchDefaultDirect_IsError) {
auto assembly = CommonTypes() + R"(
%100 = OpFunction %void None %voidfn
@@ -5798,7 +5919,7 @@
%30 = OpLabel
OpBranch %60
- %50 = OpLabel
+ %50 = OpLabel ; continue target
OpBranch %60
%60 = OpLabel
@@ -5816,7 +5937,7 @@
EXPECT_FALSE(FlowClassifyCFGEdges(&fe));
EXPECT_THAT(p->error(),
Eq("Branch from block 30 to block 60 is an invalid exit from "
- "construct starting at block 20; branch bypasses block 50"));
+ "construct starting at block 20; branch bypasses continue target 50"));
}
TEST_F(SpvParserTest,
@@ -5849,7 +5970,7 @@
EXPECT_FALSE(FlowClassifyCFGEdges(&fe));
EXPECT_THAT(p->error(),
Eq("Branch from block 50 to block 60 is an invalid exit from "
- "construct starting at block 50; branch bypasses block 80"));
+ "construct starting at block 50; branch bypasses merge block 80"));
}
TEST_F(SpvParserTest, ClassifyCFGEdges_TooManyBackedges) {
@@ -6351,7 +6472,7 @@
}
TEST_F(SpvParserTest,
- FindIfSelectionInternalHeaders_Premerge_MultiCandidate_Error) {
+ FindIfSelectionInternalHeaders_Premerge_MultiCandidate_IsError) {
auto assembly = CommonTypes() + R"(
%100 = OpFunction %void None %voidfn
@@ -6491,7 +6612,7 @@
}
TEST_F(SpvParserTest,
- FindIfSelectionInternalHeaders_IfBreak_WithForwardToPremerge_Error) {
+ FindIfSelectionInternalHeaders_IfBreak_WithForwardToPremerge_IsError) {
auto assembly = CommonTypes() + R"(
%100 = OpFunction %void None %voidfn