AuroraMiddleware/SPIRV-Tools
1
0
Fork 0
mirror of https://github.com/KhronosGroup/SPIRV-Tools synced 2024-11-26 05:10:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

Update validator diagnostics with "structurally dominated" (#4844)

Browse Source 
The updated rules in SPIR-V 1.6 Rev2 use structural dominance,
so update the messages to match
This commit is contained in:
David Neto 2022-07-06 15:10:29 -04:00 committed by GitHub
parent 5f4284aa78
commit dcee3a5de0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 37 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
source/val/validate_cfg.cpp
View File

@ -553,7 +553,7 @@ spv_result_t StructuredSwitchChecks(ValidationState_t& _, Function* function,
!header->structurally_dominates(*target_block)) { !header->structurally_dominates(*target_block)) {
return _.diag(SPV_ERROR_INVALID_CFG, header->label()) return _.diag(SPV_ERROR_INVALID_CFG, header->label())
<< "Selection header " << _.getIdName(header->id()) << "Selection header " << _.getIdName(header->id())
<< " does not dominate its case construct " << " does not structurally dominate its case construct "
<< _.getIdName(target); << _.getIdName(target);
} }
@ -742,15 +742,15 @@ spv_result_t StructuredControlFlowChecks(
return _.diag(SPV_ERROR_INVALID_CFG, _.FindDef(merge->id())) return _.diag(SPV_ERROR_INVALID_CFG, _.FindDef(merge->id()))
<< ConstructErrorString(construct, _.getIdName(header->id()), << ConstructErrorString(construct, _.getIdName(header->id()),
_.getIdName(merge->id()), _.getIdName(merge->id()),
"does not dominate"); "does not structurally dominate");
} }
// If it's really a merge block for a selection or loop, then it must be // If it's really a merge block for a selection or loop, then it must be
// *strictly* dominated by the header. // *strictly* structrually dominated by the header.
if (construct.ExitBlockIsMergeBlock() && (header == merge)) { if (construct.ExitBlockIsMergeBlock() && (header == merge)) {
return _.diag(SPV_ERROR_INVALID_CFG, _.FindDef(merge->id())) return _.diag(SPV_ERROR_INVALID_CFG, _.FindDef(merge->id()))
<< ConstructErrorString(construct, _.getIdName(header->id()), << ConstructErrorString(construct, _.getIdName(header->id()),
_.getIdName(merge->id()), _.getIdName(merge->id()),
"does not strictly dominate"); "does not strictly structurally dominate");
} }
// Check post-dominance for continue constructs. But dominance and // Check post-dominance for continue constructs. But dominance and
@ -760,7 +760,7 @@ spv_result_t StructuredControlFlowChecks(
return _.diag(SPV_ERROR_INVALID_CFG, _.FindDef(merge->id())) return _.diag(SPV_ERROR_INVALID_CFG, _.FindDef(merge->id()))
<< ConstructErrorString(construct, _.getIdName(header->id()), << ConstructErrorString(construct, _.getIdName(header->id()),
_.getIdName(merge->id()), _.getIdName(merge->id()),
"is not post dominated by"); "is not structurally post dominated by");
} }
} }

36
test/val/val_cfg_test.cpp
View File

@ -663,7 +663,8 @@ TEST_P(ValidateCFG, HeaderDoesntStrictlyDominateMergeBad) {
EXPECT_THAT( EXPECT_THAT(
getDiagnosticString(), getDiagnosticString(),
MatchesRegex("The selection construct with the selection header " MatchesRegex("The selection construct with the selection header "
".\\[%head\\] does not strictly dominate the merge block " ".\\[%head\\] does not strictly structurally dominate the "
"merge block "
".\\[%head\\]\n %head = OpLabel\n")); ".\\[%head\\]\n %head = OpLabel\n"));
} else { } else {
ASSERT_EQ(SPV_SUCCESS, ValidateInstructions()) << str; ASSERT_EQ(SPV_SUCCESS, ValidateInstructions()) << str;
@ -1345,9 +1346,11 @@ TEST_P(ValidateCFG, BackEdgeBlockDoesntPostDominateContinueTargetBad) {
CompileSuccessfully(str); CompileSuccessfully(str);
if (GetParam() == SpvCapabilityShader) { if (GetParam() == SpvCapabilityShader) {
ASSERT_EQ(SPV_ERROR_INVALID_CFG, ValidateInstructions()); ASSERT_EQ(SPV_ERROR_INVALID_CFG, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(), EXPECT_THAT(
MatchesRegex("The continue construct with the continue target " getDiagnosticString(),
".\\[%loop1_cont\\] is not post dominated by the " MatchesRegex(
"The continue construct with the continue target "
".\\[%loop1_cont\\] is not structurally post dominated by the "
"back-edge block .\\[%be_block\\]\n" "back-edge block .\\[%be_block\\]\n"
" %be_block = OpLabel\n")); " %be_block = OpLabel\n"));
} else { } else {
@ -1485,8 +1488,9 @@ TEST_P(ValidateCFG, ContinueTargetMustBePostDominatedByBackEdge) {
if (is_shader) { if (is_shader) {
ASSERT_EQ(SPV_ERROR_INVALID_CFG, ValidateInstructions()); ASSERT_EQ(SPV_ERROR_INVALID_CFG, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(), EXPECT_THAT(getDiagnosticString(),
MatchesRegex("The continue construct with the continue target " MatchesRegex(
".\\[%cheader\\] is not post dominated by the " "The continue construct with the continue target "
".\\[%cheader\\] is not structurally post dominated by the "
"back-edge block .\\[%be_block\\]\n" "back-edge block .\\[%be_block\\]\n"
" %be_block = OpLabel\n")); " %be_block = OpLabel\n"));
} else { } else {
@ -1517,9 +1521,10 @@ TEST_P(ValidateCFG, BranchOutOfConstructToMergeBad) {
CompileSuccessfully(str); CompileSuccessfully(str);
if (is_shader) { if (is_shader) {
ASSERT_EQ(SPV_ERROR_INVALID_CFG, ValidateInstructions()); ASSERT_EQ(SPV_ERROR_INVALID_CFG, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(), EXPECT_THAT(
getDiagnosticString(),
MatchesRegex("The continue construct with the continue target " MatchesRegex("The continue construct with the continue target "
".\\[%loop\\] is not post dominated by the " ".\\[%loop\\] is not structurally post dominated by the "
"back-edge block .\\[%cont\\]\n" "back-edge block .\\[%cont\\]\n"
" %cont = OpLabel\n")) " %cont = OpLabel\n"))
<< str; << str;
@ -1553,9 +1558,10 @@ TEST_P(ValidateCFG, BranchOutOfConstructBad) {
CompileSuccessfully(str); CompileSuccessfully(str);
if (is_shader) { if (is_shader) {
ASSERT_EQ(SPV_ERROR_INVALID_CFG, ValidateInstructions()); ASSERT_EQ(SPV_ERROR_INVALID_CFG, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(), EXPECT_THAT(
getDiagnosticString(),
MatchesRegex("The continue construct with the continue target " MatchesRegex("The continue construct with the continue target "
".\\[%loop\\] is not post dominated by the " ".\\[%loop\\] is not structurally post dominated by the "
"back-edge block .\\[%cont\\]\n" "back-edge block .\\[%cont\\]\n"
" %cont = OpLabel\n")); " %cont = OpLabel\n"));
} else { } else {
@ -3200,7 +3206,7 @@ OpFunctionEnd
EXPECT_THAT( EXPECT_THAT(
getDiagnosticString(), getDiagnosticString(),
HasSubstr("The continue construct with the continue target 9[%9] is not " HasSubstr("The continue construct with the continue target 9[%9] is not "
"post dominated by the back-edge block 13[%13]")); "structurally post dominated by the back-edge block 13[%13]"));
} }
TEST_F(ValidateCFG, BreakFromSwitch) { TEST_F(ValidateCFG, BreakFromSwitch) {
@ -4208,9 +4214,11 @@ TEST_F(ValidateCFG, StructuredSelections_RegisterBothTrueAndFalse) {
CompileSuccessfully(text); CompileSuccessfully(text);
EXPECT_NE(SPV_SUCCESS, ValidateInstructions()); EXPECT_NE(SPV_SUCCESS, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(), EXPECT_THAT(
HasSubstr("The selection construct with the selection header " getDiagnosticString(),
"8[%8] does not dominate the merge block 10[%10]\n")); HasSubstr(
"The selection construct with the selection header "
"8[%8] does not structurally dominate the merge block 10[%10]\n"));
} }
TEST_F(ValidateCFG, UnreachableIsStaticallyReachable) { TEST_F(ValidateCFG, UnreachableIsStaticallyReachable) {