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

val: interface struct with builtins must be Block (#4665)

Browse Source 
For a shader input/output interface variable of structure type:
If the structure has BuiltIn members, then the structure type
must be Block decorated.

Otherwise, the variable, or the struct members must have locations,
but nothing can have both a location be a BuiltIn.

Implements validation needed to reject the example in
https://github.com/KhronosGroup/SPIRV-Registry/issues/134
This commit is contained in:
David Neto 2021-12-16 16:48:12 -05:00 committed by GitHub
parent 5d0e3240f0
commit df2aad68b9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 83 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
source/val/validate_decorations.cpp
View File

@ -98,6 +98,14 @@ bool isBuiltInStruct(uint32_t struct_id, ValidationState_t& vstate) {
});
}
// Returns true if the given structure type has a Block decoration.
bool isBlock(uint32_t struct_id, ValidationState_t& vstate) {
const auto& decorations = vstate.id_decorations(struct_id);
return std::any_of(
decorations.begin(), decorations.end(),
[](const Decoration& d) { return SpvDecorationBlock == d.dec_type(); });
}
// Returns true if the given ID has the Import LinkageAttributes decoration.
bool hasImportLinkageAttribute(uint32_t id, ValidationState_t& vstate) {
const auto& decorations = vstate.id_decorations(id);
@ -716,8 +724,8 @@ spv_result_t CheckBuiltInVariable(uint32_t var_id, ValidationState_t& vstate) {
spv_result_t CheckDecorationsOfEntryPoints(ValidationState_t& vstate) {
for (uint32_t entry_point : vstate.entry_points()) {
const auto& descs = vstate.entry_point_descriptions(entry_point);
int num_builtin_inputs = 0;
int num_builtin_outputs = 0;
int num_builtin_block_inputs = 0;
int num_builtin_block_outputs = 0;
int num_workgroup_variables = 0;
int num_workgroup_variables_with_block = 0;
int num_workgroup_variables_with_aliased = 0;
@ -768,9 +776,20 @@ spv_result_t CheckDecorationsOfEntryPoints(ValidationState_t& vstate) {
Instruction* type_instr = vstate.FindDef(type_id);
if (type_instr && SpvOpTypeStruct == type_instr->opcode() &&
isBuiltInStruct(type_id, vstate)) {
if (storage_class == SpvStorageClassInput) ++num_builtin_inputs;
if (storage_class == SpvStorageClassOutput) ++num_builtin_outputs;
if (num_builtin_inputs > 1 || num_builtin_outputs > 1) break;
if (!isBlock(type_id, vstate)) {
return vstate.diag(SPV_ERROR_INVALID_DATA, vstate.FindDef(type_id))
<< vstate.VkErrorID(4919)
<< "Interface struct has no Block decoration but has "
"BuiltIn members. "
"Location decorations must be used on each member of "
"OpVariable with a structure type that is a block not "
"decorated with Location.";
}
if (storage_class == SpvStorageClassInput) ++num_builtin_block_inputs;
if (storage_class == SpvStorageClassOutput)
++num_builtin_block_outputs;
if (num_builtin_block_inputs > 1 || num_builtin_block_outputs > 1)
break;
if (auto error = CheckBuiltInVariable(interface, vstate))
return error;
} else if (isBuiltInVar(interface, vstate)) {
@ -788,7 +807,7 @@ spv_result_t CheckDecorationsOfEntryPoints(ValidationState_t& vstate) {
}
}
}
if (num_builtin_inputs > 1 || num_builtin_outputs > 1) {
if (num_builtin_block_inputs > 1 || num_builtin_block_outputs > 1) {
return vstate.diag(SPV_ERROR_INVALID_BINARY,
vstate.FindDef(entry_point))
<< "There must be at most one object per Storage Class that can "

17
test/val/val_builtins_test.cpp
View File

@ -93,7 +93,8 @@ CodeGenerator GetInMainCodeGenerator(const char* const built_in,
generator.extensions_ += extensions;
}
generator.before_types_ = "OpMemberDecorate %built_in_type 0 BuiltIn ";
generator.before_types_ = R"(OpDecorate %built_in_type Block
OpMemberDecorate %built_in_type 0 BuiltIn )";
generator.before_types_ += built_in;
generator.before_types_ += "\n";
@ -251,7 +252,8 @@ CodeGenerator GetInFunctionCodeGenerator(const char* const built_in,
generator.extensions_ += extensions;
}
generator.before_types_ = "OpMemberDecorate %built_in_type 0 BuiltIn ";
generator.before_types_ = R"(OpDecorate %built_in_type Block
OpMemberDecorate %built_in_type 0 BuiltIn )";
generator.before_types_ += built_in;
generator.before_types_ += "\n";
@ -3098,6 +3100,8 @@ TEST_F(ValidateBuiltIns, TwoBuiltInsFirstFails) {
CodeGenerator generator = CodeGenerator::GetDefaultShaderCodeGenerator();
generator.before_types_ = R"(
OpDecorate %input_type Block
OpDecorate %output_type Block
OpMemberDecorate %input_type 0 BuiltIn FragCoord
OpMemberDecorate %output_type 0 BuiltIn Position
)";
@ -3138,6 +3142,8 @@ TEST_F(ValidateBuiltIns, TwoBuiltInsSecondFails) {
CodeGenerator generator = CodeGenerator::GetDefaultShaderCodeGenerator();
generator.before_types_ = R"(
OpDecorate %input_type Block
OpDecorate %output_type Block
OpMemberDecorate %input_type 0 BuiltIn Position
OpMemberDecorate %output_type 0 BuiltIn FragCoord
)";
@ -3201,6 +3207,7 @@ OpStore %position %f32vec4_0123
TEST_F(ValidateBuiltIns, FragmentPositionTwoEntryPoints) {
CodeGenerator generator = CodeGenerator::GetDefaultShaderCodeGenerator();
generator.before_types_ = R"(
OpDecorate %output_type Block
OpMemberDecorate %output_type 0 BuiltIn Position
)";
@ -3252,6 +3259,7 @@ CodeGenerator GetNoDepthReplacingGenerator() {
CodeGenerator generator = CodeGenerator::GetDefaultShaderCodeGenerator();
generator.before_types_ = R"(
OpDecorate %output_type Block
OpMemberDecorate %output_type 0 BuiltIn FragDepth
)";
@ -3303,6 +3311,7 @@ CodeGenerator GetOneMainHasDepthReplacingOtherHasntGenerator() {
CodeGenerator generator = CodeGenerator::GetDefaultShaderCodeGenerator();
generator.before_types_ = R"(
OpDecorate %output_type Block
OpMemberDecorate %output_type 0 BuiltIn FragDepth
)";
@ -3374,6 +3383,7 @@ OpExtension "SPV_NV_ray_tracing"
)";
generator.before_types_ = R"(
OpDecorate %input_type Block
OpMemberDecorate %input_type 0 BuiltIn InstanceId
)";
@ -3609,6 +3619,7 @@ OpCapability GroupNonUniformBallot
OpMemoryModel Logical GLSL450
OpEntryPoint GLCompute %foo "foo"
OpExecutionMode %foo LocalSize 1 1 1
OpDecorate %struct Block
OpMemberDecorate %struct 0 BuiltIn SubgroupEqMask
%void = OpTypeVoid
%int = OpTypeInt 32 0
@ -3663,6 +3674,7 @@ OpCapability GroupNonUniform
OpMemoryModel Logical GLSL450
OpEntryPoint GLCompute %foo "foo"
OpExecutionMode %foo LocalSize 1 1 1
OpDecorate %struct Block
OpMemberDecorate %struct 0 BuiltIn SubgroupSize
%void = OpTypeVoid
%int = OpTypeInt 32 0
@ -3723,6 +3735,7 @@ OpCapability GroupNonUniform
OpMemoryModel Logical GLSL450
OpEntryPoint GLCompute %foo "foo"
OpExecutionMode %foo LocalSize 1 1 1
OpDecorate %struct Block
OpMemberDecorate %struct 0 BuiltIn SubgroupId
%void = OpTypeVoid
%int = OpTypeInt 32 0

7
test/val/val_decoration_test.cpp
View File

@ -226,6 +226,7 @@ TEST_F(ValidateDecorations, StructAllMembersHaveBuiltInDecorationsGood) {
OpCapability Shader
OpCapability Linkage
OpMemoryModel Logical GLSL450
OpDecorate %_struct_1 Block
OpMemberDecorate %_struct_1 0 BuiltIn Position
OpMemberDecorate %_struct_1 1 BuiltIn Position
OpMemberDecorate %_struct_1 2 BuiltIn Position
@ -243,6 +244,7 @@ TEST_F(ValidateDecorations, MixedBuiltInDecorationsBad) {
OpCapability Shader
OpCapability Linkage
OpMemoryModel Logical GLSL450
OpDecorate %_struct_1 Block
OpMemberDecorate %_struct_1 0 BuiltIn Position
OpMemberDecorate %_struct_1 1 BuiltIn Position
%float = OpTypeFloat 32
@ -265,6 +267,7 @@ TEST_F(ValidateDecorations, StructContainsBuiltInStructBad) {
OpCapability Shader
OpCapability Linkage
OpMemoryModel Logical GLSL450
OpDecorate %_struct_1 Block
OpMemberDecorate %_struct_1 0 BuiltIn Position
OpMemberDecorate %_struct_1 1 BuiltIn Position
OpMemberDecorate %_struct_1 2 BuiltIn Position
@ -305,6 +308,8 @@ TEST_F(ValidateDecorations, MultipleBuiltInObjectsConsumedByOpEntryPointBad) {
OpEntryPoint Geometry %main "main" %in_1 %in_2
OpExecutionMode %main InputPoints
OpExecutionMode %main OutputPoints
OpDecorate %struct_1 Block
OpDecorate %struct_2 Block
OpMemberDecorate %struct_1 0 BuiltIn InvocationId
OpMemberDecorate %struct_2 0 BuiltIn Position
%int = OpTypeInt 32 1
@ -339,6 +344,8 @@ TEST_F(ValidateDecorations,
OpEntryPoint Geometry %main "main" %in_1 %out_1
OpExecutionMode %main InputPoints
OpExecutionMode %main OutputPoints
OpDecorate %struct_1 Block
OpDecorate %struct_2 Block
OpMemberDecorate %struct_1 0 BuiltIn InvocationId
OpMemberDecorate %struct_2 0 BuiltIn Position
%int = OpTypeInt 32 1

36
test/val/val_interfaces_test.cpp
View File

@ -1500,6 +1500,42 @@ OpFunctionEnd
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_0));
}
TEST_F(ValidateInterfacesTest, StructWithBuiltinsMissingBlock_Bad) {
// See https://github.com/KhronosGroup/SPIRV-Registry/issues/134
//
// When a shader input or output is a struct that does not have Block,
// then it must have a Location.
// But BuiltIns must not have locations.
const std::string text = R"(
OpCapability Shader
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %main "main" %in
OpExecutionMode %main OriginUpperLeft
; %struct needs a Block decoration
OpMemberDecorate %struct 0 BuiltIn Position
%void = OpTypeVoid
%float = OpTypeFloat 32
%v4float = OpTypeVector %float 4
%struct = OpTypeStruct %v4float
%in_ptr = OpTypePointer Input %struct
%in = OpVariable %in_ptr Input
%void_fn = OpTypeFunction %void
%main = OpFunction %void None %void_fn
%entry = OpLabel
OpReturn
OpFunctionEnd
)";
CompileSuccessfully(text, SPV_ENV_VULKAN_1_0);
EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_VULKAN_1_0));
EXPECT_THAT(getDiagnosticString(),
AnyVUID("VUID-StandaloneSpirv-Location-04919"));
EXPECT_THAT(
getDiagnosticString(),
HasSubstr(
"Interface struct has no Block decoration but has BuiltIn members."));
}
} // namespace
} // namespace val
} // namespace spvtools