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

Avoid confusing short-circuiting (#3404)

Browse Source 
When checking the OpBranchConditional for selection headers,
we intend to register both the true and false targets.
Short circuiting was getting in the way.

Co-authored-by: Steven Perron <stevenperron@google.com>
This commit is contained in:
David Neto 2021-10-28 15:57:36 -04:00 committed by GitHub
parent 7c5b17d379
commit 2feb7074d4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 62 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
source/val/validate_cfg.cpp
View File

@ -660,7 +660,7 @@ spv_result_t ValidateStructuredSelections(
// was missing a merge instruction and both labels hadn't been seen
// previously.
const bool both_unseen =
seen.insert(true_label).second && seen.insert(false_label).second;
seen.insert(true_label).second & seen.insert(false_label).second;
if (!merge && both_unseen) {
return _.diag(SPV_ERROR_INVALID_CFG, terminator)
<< "Selection must be structured";

61
test/val/val_cfg_test.cpp
View File

@ -4229,6 +4229,67 @@ OpFunctionEnd
ASSERT_EQ(SPV_SUCCESS, ValidateInstructions());
}
TEST_F(ValidateCFG, StructuredSelections_RegisterBothTrueAndFalse) {
// In this test, we try to make a case where the false branches
// to %20 and %60 from blocks %10 and %50 must be registered
// during the validity check for sturctured selections.
// However, an error is caught earlier in the flow, that the
// branches from %100 to %20 and %60 violate dominance.
const std::string text = R"(
OpCapability Shader
OpMemoryModel Logical Simple
OpEntryPoint Fragment %main "main"
OpExecutionMode %main OriginUpperLeft
%void = OpTypeVoid
%void_fn = OpTypeFunction %void
%bool = OpTypeBool
%cond = OpUndef %bool
%main = OpFunction %void None %void_fn
%1 = OpLabel
OpSelectionMerge %999 None
OpBranchConditional %cond %10 %100
%10 = OpLabel
OpSelectionMerge %30 None ; force registration of %30
OpBranchConditional %cond %30 %20 ; %20 should be registered too
%20 = OpLabel
OpBranch %30
%30 = OpLabel ; merge for first if
OpBranch %50
%50 = OpLabel
OpSelectionMerge %70 None ; force registration of %70
OpBranchConditional %cond %70 %60 ; %60 should be registered
%60 = OpLabel
OpBranch %70
%70 = OpLabel ; merge for second if
OpBranch %999
%100 = OpLabel
OpBranchConditional %cond %20 %60 ; should require a merge
%999 = OpLabel
OpReturn
OpFunctionEnd
)";
CompileSuccessfully(text);
EXPECT_NE(SPV_SUCCESS, ValidateInstructions());
EXPECT_THAT(getDiagnosticString(),
HasSubstr("The selection construct with the selection header "
"8[%8] does not dominate the merge block 10[%10]\n"));
}
TEST_F(ValidateCFG, UnreachableIsStaticallyReachable) {
const std::string text = R"(
OpCapability Shader