spirv-reader: CFG tests: make valid SPIR-V
Bug: tint:765
Change-Id: I84ea018478feafc4a5c03052832acae8cf3f15b9
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49940
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: Alan Baker <alanbaker@google.com>
diff --git a/src/reader/spirv/function_cfg_test.cc b/src/reader/spirv/function_cfg_test.cc
index 7217d3b..8889b2a 100644
--- a/src/reader/spirv/function_cfg_test.cc
+++ b/src/reader/spirv/function_cfg_test.cc
@@ -46,6 +46,8 @@
return R"(
OpCapability Shader
OpMemoryModel Logical Simple
+ OpEntryPoint Fragment %100 "main"
+ OpExecutionMode %100 OriginUpperLeft
OpName %var "var"
@@ -1078,13 +1080,17 @@
%100 = OpFunction %void None %voidfn
%10 = OpLabel
- OpBranch %20
+ OpSelectionMerge %99 None
+ OpBranchConditional %cond %20 %30
%30 = OpLabel
OpReturn
%20 = OpLabel
- OpBranch %30 ; backtrack
+ OpBranch %30 ; backtrack, but does dominate %30
+
+ %99 = OpLabel
+ OpReturn
OpFunctionEnd
)"));
@@ -1093,7 +1099,7 @@
fe.RegisterBasicBlocks();
fe.ComputeBlockOrderAndPositions();
- EXPECT_THAT(fe.block_order(), ElementsAre(10, 20, 30));
+ EXPECT_THAT(fe.block_order(), ElementsAre(10, 20, 30, 99));
const auto* bi10 = fe.GetBlockInfo(10);
ASSERT_NE(bi10, nullptr);
@@ -1104,6 +1110,9 @@
const auto* bi30 = fe.GetBlockInfo(30);
ASSERT_NE(bi30, nullptr);
EXPECT_EQ(bi30->pos, 2u);
+ const auto* bi99 = fe.GetBlockInfo(99);
+ ASSERT_NE(bi99, nullptr);
+ EXPECT_EQ(bi99->pos, 3u);
}
TEST_F(SpvParserCFGTest, ComputeBlockOrder_DupConditionalBranch) {
@@ -1114,12 +1123,12 @@
OpSelectionMerge %99 None
OpBranchConditional %cond %20 %20
- %99 = OpLabel
- OpReturn
-
%20 = OpLabel
OpBranch %99
+ %99 = OpLabel
+ OpReturn
+
OpFunctionEnd
)"));
ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
@@ -1138,15 +1147,15 @@
OpSelectionMerge %99 None
OpBranchConditional %cond %20 %30
- %99 = OpLabel
- OpReturn
-
%30 = OpLabel
OpReturn
%20 = OpLabel
OpBranch %99
+ %99 = OpLabel ; dominated by %20, so follow %20
+ OpReturn
+
OpFunctionEnd
)"));
ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
@@ -1299,7 +1308,9 @@
%10 = OpLabel
OpSelectionMerge %99 None
- OpSwitch %selector %99 20 %20 30 %30 40 %40 50 %50
+ ; SPIR-V validation requires a fallthrough destination to immediately
+ ; follow the source. So %20 -> %40, %30 -> %50
+ OpSwitch %selector %99 20 %20 40 %40 30 %30 50 %50
%50 = OpLabel
OpBranch %99
@@ -1373,9 +1384,6 @@
OpSelectionMerge %99 None
OpSwitch %selector %80 20 %20 30 %30
- %99 = OpLabel
- OpReturn
-
%20 = OpLabel
OpBranch %80 ; fallthrough to default
@@ -1385,6 +1393,9 @@
%30 = OpLabel
OpBranch %99
+ %99 = OpLabel ; dominated by %30, so follow %30
+ OpReturn
+
OpFunctionEnd
)";
auto p = parser(test::Assemble(assembly));
@@ -1432,6 +1443,9 @@
EXPECT_THAT(fe.block_order(), ElementsAre(10, 50, 40, 20, 30, 99))
<< assembly;
+
+ // We're deliberately testing a case that SPIR-V doesn't allow.
+ p->DeliberatelyInvalidSpirv();
}
TEST_F(SpvParserCFGTest,
@@ -1441,7 +1455,9 @@
%10 = OpLabel
OpSelectionMerge %99 None
- OpSwitch %selector %99 20 %20 30 %30 40 %40 50 %50
+ ; SPIR-V validation requires a fallthrough destination to immediately
+ ; follow the source. So %20 -> %40
+ OpSwitch %selector %99 20 %20 40 %40 30 %30 50 %50
%99 = OpLabel
OpReturn
@@ -1641,22 +1657,22 @@
OpSelectionMerge %49 None
OpBranchConditional %cond %99 %40 ; break-if
- %49 = OpLabel
- OpBranch %99
-
%40 = OpLabel
OpBranch %49
+ %49 = OpLabel
+ OpBranch %99
+
%50 = OpLabel
OpSelectionMerge %79 None
OpBranchConditional %cond %60 %99 ; break-unless
- %79 = OpLabel
- OpBranch %99
-
%60 = OpLabel
OpBranch %79
+ %79 = OpLabel ; dominated by 60, so must follow 60
+ OpBranch %99
+
OpFunctionEnd
)";
auto p = parser(test::Assemble(assembly));
@@ -2184,6 +2200,11 @@
EXPECT_THAT(fe.block_order(), ElementsAre(10, 20, 30, 45, 40, 49, 50, 99))
<< assembly;
+
+ // Fails SPIR-V validation:
+ // Branch from block 40 to block 99 is an invalid exit from construct starting
+ // at block 30; branch bypasses merge block 49
+ p->DeliberatelyInvalidSpirv();
}
TEST_F(SpvParserCFGTest, ComputeBlockOrder_Loop_Body_Switch_CaseContinues) {
@@ -2240,6 +2261,7 @@
OpBranchConditional %cond %30 %99
%30 = OpLabel
+ ; OpSwitch must be preceded by a selection merge
OpSwitch %selector %99 50 %50 ; default is break, 50 is continue
%40 = OpLabel
@@ -2254,12 +2276,10 @@
OpFunctionEnd
)";
auto p = parser(test::Assemble(assembly));
- ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
- auto fe = p->function_emitter(100);
- fe.RegisterBasicBlocks();
- fe.ComputeBlockOrderAndPositions();
-
- EXPECT_THAT(fe.block_order(), ElementsAre(10, 20, 30, 50, 99));
+ EXPECT_FALSE(p->Parse());
+ EXPECT_FALSE(p->success());
+ EXPECT_THAT(p->error(),
+ HasSubstr("OpSwitch must be preceeded by an OpSelectionMerge"));
}
TEST_F(SpvParserCFGTest, ComputeBlockOrder_Loop_Continue_Sequence) {
@@ -2414,7 +2434,9 @@
OpBranch %50
%50 = OpLabel
- OpSwitch %selector %20 99 %99 ; yes, this is obtuse but valid
+ ; Updated SPIR-V rule:
+ ; OpSwitch must be preceded by a selection.
+ OpSwitch %selector %20 99 %99
%99 = OpLabel
OpReturn
@@ -2422,12 +2444,10 @@
OpFunctionEnd
)";
auto p = parser(test::Assemble(assembly));
- ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
- auto fe = p->function_emitter(100);
- fe.RegisterBasicBlocks();
- fe.ComputeBlockOrderAndPositions();
-
- EXPECT_THAT(fe.block_order(), ElementsAre(10, 20, 30, 50, 99));
+ EXPECT_FALSE(p->Parse());
+ EXPECT_FALSE(p->success());
+ EXPECT_THAT(p->error(),
+ HasSubstr("OpSwitch must be preceeded by an OpSelectionMerge"));
}
TEST_F(SpvParserCFGTest, ComputeBlockOrder_Loop_Loop) {
@@ -2653,6 +2673,11 @@
EXPECT_THAT(fe.block_order(),
ElementsAre(10, 20, 30, 35, 37, 40, 49, 50, 99));
+
+ p->DeliberatelyInvalidSpirv();
+ // SPIR-V validation fails:
+ // block <ID> 40[%40] exits the continue headed by <ID> 40[%40], but not
+ // via a structured exit"
}
TEST_F(SpvParserCFGTest, ComputeBlockOrder_Loop_Loop_SwitchBackedgeBreakContinue) {
@@ -2703,6 +2728,11 @@
EXPECT_THAT(fe.block_order(),
ElementsAre(10, 20, 30, 35, 37, 40, 49, 50, 99));
+
+ p->DeliberatelyInvalidSpirv();
+ // SPIR-V validation fails:
+ // block <ID> 40[%40] exits the continue headed by <ID> 40[%40], but not
+ // via a structured exit"
}
TEST_F(SpvParserCFGTest, VerifyHeaderContinueMergeOrder_Selection_Good) {
@@ -3178,7 +3208,7 @@
%20 = OpLabel
OpLoopMerge %99 %20 None
- OpBranchConditional %cond %30 %99
+ OpBranch %30
%30 = OpLabel
OpBranch %40
@@ -3187,7 +3217,7 @@
OpBranch %50
%50 = OpLabel
- OpBranch %20
+ OpBranchConditional %cond %20 %99
%99 = OpLabel
OpReturn
@@ -3487,7 +3517,7 @@
%20 = OpLabel
OpLoopMerge %89 %50 None
- OpBranchConditional %cond %30 %99
+ OpBranchConditional %cond %30 %89
%30 = OpLabel ; single block loop
OpLoopMerge %40 %30 None
@@ -3650,7 +3680,7 @@
%20 = OpLabel
OpLoopMerge %89 %20 None
- OpBranchConditional %cond %20 %99
+ OpBranchConditional %cond %20 %89
%89 = OpLabel
OpBranch %99
@@ -3702,7 +3732,7 @@
OpBranch %20
%89 = OpLabel ; merge for the loop
- OpBranch %20
+ OpBranch %99
%99 = OpLabel ; merge for the if
OpReturn
@@ -4050,6 +4080,9 @@
fe.RegisterMerges();
fe.LabelControlFlowConstructs();
EXPECT_TRUE(fe.FindSwitchCaseHeaders());
+
+ // TODO(crbug.com/tint/774) Re-enable after codegen bug fixed.
+ p->DeliberatelyInvalidSpirv();
}
TEST_F(SpvParserCFGTest, FindSwitchCaseHeaders_CaseCantEscapeSwitch) {
@@ -7493,6 +7526,10 @@
EXPECT_EQ(bi20->succ_edge[99], EdgeKind::kIfBreak);
EXPECT_THAT(p->error(), Eq(""));
+
+ // TODO(crbug.com/tint/775): The SPIR-V reader errors out on this case.
+ // Remove this when it's fixed.
+ p->DeliberatelyInvalidSpirv();
}
TEST_F(
@@ -10460,7 +10497,7 @@
TEST_F(SpvParserCFGTest, EmitBody_ReturnValue_Loop) {
auto p = parser(test::Assemble(CommonTypes() + R"(
- %200 = OpFunction %void None %voidfn
+ %200 = OpFunction %uint None %uintfn
%210 = OpLabel
OpBranch %220
@@ -11002,8 +11039,9 @@
"(violates post-dominance rule)"));
}
-TEST_F(SpvParserCFGTest,
- EmitBody_Branch_LoopBreak_MultiBlockLoop_FromContinueConstructEnd) {
+TEST_F(
+ SpvParserCFGTest,
+ EmitBody_Branch_LoopBreak_MultiBlockLoop_FromContinueConstructEnd_Unconditional) {
auto p = parser(test::Assemble(CommonTypes() + R"(
%100 = OpFunction %void None %voidfn
@@ -11017,6 +11055,52 @@
%80 = OpLabel ; continue target
OpStore %var %uint_1
OpBranch %99 ; should be a backedge
+ ; This is invalid as there must be a backedge to the loop header.
+ ; The SPIR-V allows this and understands how to emit it, even if it's not
+ ; permitted by the SPIR-V validator.
+
+ %99 = OpLabel
+ OpReturn
+
+ OpFunctionEnd
+ )"));
+
+ p->DeliberatelyInvalidSpirv();
+
+ ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
+ auto fe = p->function_emitter(100);
+ EXPECT_TRUE(fe.EmitBody()) << p->error();
+ auto got = ToString(p->builder(), fe.ast_body());
+ auto* expect = R"(Loop{
+ continuing {
+ Assignment{
+ Identifier[not set]{var_1}
+ ScalarConstructor[not set]{1u}
+ }
+ Break{}
+ }
+}
+Return{}
+)";
+ ASSERT_EQ(expect, got);
+}
+
+TEST_F(
+ SpvParserCFGTest,
+ EmitBody_Branch_LoopBreak_MultiBlockLoop_FromContinueConstructEnd_Conditional) {
+ auto p = parser(test::Assemble(CommonTypes() + R"(
+ %100 = OpFunction %void None %voidfn
+
+ %10 = OpLabel
+ OpBranch %20
+
+ %20 = OpLabel
+ OpLoopMerge %99 %80 None
+ OpBranch %80
+
+ %80 = OpLabel ; continue target
+ OpStore %var %uint_1
+ OpBranchConditional %cond %20 %99 ; backedge, and exit
%99 = OpLabel
OpReturn
@@ -11033,7 +11117,18 @@
Identifier[not set]{var_1}
ScalarConstructor[not set]{1u}
}
- Break{}
+ If{
+ (
+ ScalarConstructor[not set]{false}
+ )
+ {
+ }
+ }
+ Else{
+ {
+ Break{}
+ }
+ }
}
}
Return{}
@@ -14198,10 +14293,10 @@
%20 = OpLabel
OpLoopMerge %99 %20 None ; continue target is also loop header
- OpBranchConditional %cond %30 %99
+ OpBranch %30
%30 = OpLabel
- OpBranch %20
+ OpBranchConditional %cond %20 %99
%99 = OpLabel
OpReturn
