spirv-reader: disallow OpSwitch without preceding OpSelectionMerge

This was updated/clarified by the SPIR WG.

Bug: tint:3
Change-Id: Ie4c503f0e5f80ffeabada9c526375588e81a5ceb
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45740
Auto-Submit: David Neto <dneto@google.com>
Commit-Queue: Alan Baker <alanbaker@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Baker <alanbaker@google.com>
This commit is contained in:
David Neto 2021-03-23 17:06:29 +00:00 committed by Commit Bot service account
parent f40330502d
commit 84ef13c84f
2 changed files with 44 additions and 15 deletions

View File

@ -2621,8 +2621,14 @@ bool FunctionEmitter::EmitNormalTerminator(const BlockInfo& block_info) {
return true; return true;
} }
case SpvOpSwitch: case SpvOpSwitch:
// TODO(dneto) // An OpSelectionMerge must precede an OpSwitch. That is clarified
break; // in the resolution to Khronos-internal SPIR-V issue 115.
// A new enough version of the SPIR-V validator checks this case.
// But issue an error in this case, as a defensive measure.
return Fail() << "invalid structured control flow: found an OpSwitch "
"that is not preceded by an "
"OpSelectionMerge: "
<< terminator.PrettyPrint();
default: default:
break; break;
} }

View File

@ -13542,9 +13542,31 @@ TEST_F(SpvParserTest,
"a structured header (it has no merge instruction)")); "a structured header (it has no merge instruction)"));
} }
TEST_F(SpvParserTest, Switch_NotAsSelectionHeader_Simple) {
auto assembly = CommonTypes() + R"(
%100 = OpFunction %void None %voidfn
%10 = OpLabel
OpSwitch %uint_0 %99
%99 = OpLabel
OpReturn
OpFunctionEnd
)";
auto p = parser(test::Assemble(assembly));
ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
FunctionEmitter fe(p.get(), *spirv_function(p.get(), 100));
EXPECT_FALSE(fe.EmitBody());
EXPECT_THAT(
p->error(),
HasSubstr("invalid structured control flow: found an OpSwitch that "
"is not preceded by an OpSelectionMerge:"));
}
TEST_F(SpvParserTest, TEST_F(SpvParserTest,
DISABLED_Switch_NotAsSelectionHeader_NonDefaultBranchesAreContinue) { Switch_NotAsSelectionHeader_NonDefaultBranchesAreContinue) {
// Adapted from SPIRV-Tools test MissingMergeOneUnseenTargetSwitchGood // Adapted from SPIRV-Tools test MissingMergeOneUnseenTargetSwitchBad
auto p = parser(test::Assemble(CommonTypes() + R"( auto p = parser(test::Assemble(CommonTypes() + R"(
%100 = OpFunction %void None %voidfn %100 = OpFunction %void None %voidfn
%entry = OpLabel %entry = OpLabel
@ -13571,15 +13593,15 @@ TEST_F(SpvParserTest,
)")); )"));
ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
FunctionEmitter fe(p.get(), *spirv_function(p.get(), 100)); FunctionEmitter fe(p.get(), *spirv_function(p.get(), 100));
EXPECT_TRUE(fe.EmitBody()) << p->error(); EXPECT_FALSE(fe.EmitBody());
auto got = ToString(p->builder(), fe.ast_body()); EXPECT_THAT(
auto* expect = "unhandled case"; p->error(),
ASSERT_EQ(expect, got); HasSubstr("invalid structured control flow: found an OpSwitch that "
"is not preceded by an OpSelectionMerge:"));
} }
TEST_F(SpvParserTest, TEST_F(SpvParserTest, Switch_NotAsSelectionHeader_DefaultBranchIsContinue) {
DISABLED_Switch_NotAsSelectionHeader_DefaultBranchIsContinue) { // Adapted from SPIRV-Tools test MissingMergeOneUnseenTargetSwitchBad
// Adapted from SPIRV-Tools test MissingMergeOneUnseenTargetSwitchGood
auto p = parser(test::Assemble(CommonTypes() + R"( auto p = parser(test::Assemble(CommonTypes() + R"(
%100 = OpFunction %void None %voidfn %100 = OpFunction %void None %voidfn
%entry = OpLabel %entry = OpLabel
@ -13606,10 +13628,11 @@ TEST_F(SpvParserTest,
)")); )"));
ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
FunctionEmitter fe(p.get(), *spirv_function(p.get(), 100)); FunctionEmitter fe(p.get(), *spirv_function(p.get(), 100));
EXPECT_TRUE(fe.EmitBody()) << p->error(); EXPECT_FALSE(fe.EmitBody());
auto got = ToString(p->builder(), fe.ast_body()); EXPECT_THAT(
auto* expect = "unhandled case"; p->error(),
ASSERT_EQ(expect, got); HasSubstr("invalid structured control flow: found an OpSwitch that "
"is not preceded by an OpSelectionMerge:"));
} }
TEST_F(SpvParserTest, SiblingLoopConstruct_Null) { TEST_F(SpvParserTest, SiblingLoopConstruct_Null) {