Skip to content

Commit

Permalink
spirv-val: Handle Built-ins in/out of block
Browse files Browse the repository at this point in the history
  • Loading branch information
@spencer-lunarg
spencer-lunarg committed May 27, 2024
1 parent 4fecbf7 commit a5c8b0c
Show file tree
Hide file tree
Showing 2 changed files with 144 additions and 65 deletions.
116 changes: 55 additions & 61 deletions source/val/validate_decorations.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,17 +71,6 @@ uint32_t GetArrayStride(uint32_t array_id, ValidationState_t& vstate) {
return 0;
}

// Returns true if the given structure type has any members with BuiltIn
// decoration.
bool isBuiltInStruct(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 spv::Decoration::BuiltIn == d.dec_type() &&
Decoration::kInvalidMember != d.struct_member_index();
});
}

// 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);
Expand Down Expand Up @@ -822,67 +811,72 @@ spv_result_t CheckDecorationsOfEntryPoints(ValidationState_t& vstate) {
// to.
const uint32_t type_id = ptr_instr->word(3);
Instruction* type_instr = vstate.FindDef(type_id);
if (type_instr && spv::Op::OpTypeStruct == type_instr->opcode() &&
isBuiltInStruct(type_id, vstate)) {
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.";
const bool is_struct =
type_instr && spv::Op::OpTypeStruct == type_instr->opcode();

// Search all Built-in (on the variable or the struct)
bool has_built_in = false;
for (auto& dec :
vstate.id_decorations(is_struct ? type_id : interface)) {
if (dec.dec_type() != spv::Decoration::BuiltIn) continue;
has_built_in = true;

if (!spvIsVulkanEnv(vstate.context()->target_env)) continue;

const spv::BuiltIn builtin = dec.builtin();
if (storage_class == spv::StorageClass::Input) {
if (!input_var_builtin.insert(builtin).second) {
return vstate.diag(SPV_ERROR_INVALID_ID, var_instr)
<< vstate.VkErrorID(9658)
<< "OpEntryPoint contains duplicate input variables "
"with "
<< vstate.grammar().lookupOperandName(
SPV_OPERAND_TYPE_BUILT_IN, (uint32_t)builtin)
<< " builtin";
}
}
if (storage_class == spv::StorageClass::Input)
++num_builtin_block_inputs;
if (storage_class == spv::StorageClass::Output)
++num_builtin_block_outputs;
if (num_builtin_block_inputs > 1 || num_builtin_block_outputs > 1)
break;
if (storage_class == spv::StorageClass::Output) {
if (!output_var_builtin.insert(builtin).second) {
return vstate.diag(SPV_ERROR_INVALID_ID, var_instr)
<< vstate.VkErrorID(9659)
<< "OpEntryPoint contains duplicate output variables "
"with "
<< vstate.grammar().lookupOperandName(
SPV_OPERAND_TYPE_BUILT_IN, (uint32_t)builtin)
<< " builtin";
}
}
}

if (has_built_in) {
if (auto error = CheckBuiltInVariable(interface, vstate))
return error;
} else {
// If there is not builtin block, check for builtin on the variable
for (auto& dec : vstate.id_decorations(interface)) {
if (dec.dec_type() != spv::Decoration::BuiltIn) continue;

if (is_struct) {
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 == spv::StorageClass::Input)
++num_builtin_block_inputs;
if (storage_class == spv::StorageClass::Output)
++num_builtin_block_outputs;
if (num_builtin_block_inputs > 1 || num_builtin_block_outputs > 1)
break;
if (auto error = CheckBuiltInVariable(interface, vstate))
return error;

if (!spvIsVulkanEnv(vstate.context()->target_env)) continue;

const spv::BuiltIn builtin = dec.builtin();
if (storage_class == spv::StorageClass::Input) {
// RayTmaxKHR can be duplicated
// (https://gitlab.khronos.org/vulkan/vulkan/-/issues/3885)
if (!input_var_builtin.insert(builtin).second &&
builtin != spv::BuiltIn::RayTmaxKHR) {
return vstate.diag(SPV_ERROR_INVALID_ID, var_instr)
<< vstate.VkErrorID(9658)
<< "OpEntryPoint contains duplicate input variables "
"with "
<< vstate.grammar().lookupOperandName(
SPV_OPERAND_TYPE_BUILT_IN, (uint32_t)builtin)
<< " builtin";
}
}
if (storage_class == spv::StorageClass::Output) {
if (!output_var_builtin.insert(builtin).second) {
return vstate.diag(SPV_ERROR_INVALID_ID, var_instr)
<< vstate.VkErrorID(9659)
<< "OpEntryPoint contains duplicate output variables "
"with "
<< vstate.grammar().lookupOperandName(
SPV_OPERAND_TYPE_BUILT_IN, (uint32_t)builtin)
<< " builtin";
}
}
}
}

if (storage_class == spv::StorageClass::Workgroup) {
++num_workgroup_variables;
if (type_instr && spv::Op::OpTypeStruct == type_instr->opcode()) {
if (is_struct) {
if (hasDecoration(type_id, spv::Decoration::Block, vstate))
++num_workgroup_variables_with_block;
if (hasDecoration(var_instr->id(), spv::Decoration::Aliased,
Expand Down
93 changes: 89 additions & 4 deletions test/val/val_decoration_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9841,9 +9841,6 @@ TEST_F(ValidateDecorations, MultipleBuiltinsOutputFragment) {
AnyVUID("VUID-StandaloneSpirv-OpEntryPoint-09659"));
}

// Exception being discussed in
// https://gitlab.khronos.org/vulkan/vulkan/-/issues/3885 simplified version of
// Glsl/CompileToSpirv14Test.FromFile/spv_ext_AnyHitShader_rahit
TEST_F(ValidateDecorations, MultipleBuiltinsRayTmaxKHR) {
const std::string body = R"(
OpCapability RayTracingKHR
Expand Down Expand Up @@ -9879,7 +9876,95 @@ TEST_F(ValidateDecorations, MultipleBuiltinsRayTmaxKHR) {
)";

CompileSuccessfully(body.c_str(), SPV_ENV_VULKAN_1_2);
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_2));
ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_2));
EXPECT_THAT(
getDiagnosticString(),
HasSubstr(
"OpEntryPoint contains duplicate input variables with RayTmax"));
EXPECT_THAT(getDiagnosticString(),
AnyVUID("VUID-StandaloneSpirv-OpEntryPoint-09658"));
}

TEST_F(ValidateDecorations, MultipleBuiltinsBlock) {
const std::string body = R"(
OpCapability Shader
OpMemoryModel Logical GLSL450
OpEntryPoint Vertex %main "main" %var
OpMemberDecorate %gl_PerVertex 0 BuiltIn Position
OpMemberDecorate %gl_PerVertex 1 BuiltIn Position
OpDecorate %gl_PerVertex Block
%void = OpTypeVoid
%3 = OpTypeFunction %void
%float = OpTypeFloat 32
%v4float = OpTypeVector %float 4
%gl_PerVertex = OpTypeStruct %v4float %v4float
%_ptr_gl_PerVertex = OpTypePointer Output %gl_PerVertex
%var = OpVariable %_ptr_gl_PerVertex Output
%int = OpTypeInt 32 1
%int_0 = OpConstant %int 0
%int_1 = OpConstant %int 1
%float_0 = OpConstant %float 0
%17 = OpConstantComposite %v4float %float_0 %float_0 %float_0 %float_0
%ptr_vec4 = OpTypePointer Output %v4float
%main = OpFunction %void None %3
%5 = OpLabel
%19 = OpAccessChain %ptr_vec4 %var %int_0
OpStore %19 %17
%22 = OpAccessChain %ptr_vec4 %var %int_1
OpStore %22 %17
OpReturn
OpFunctionEnd
)";

CompileSuccessfully(body.c_str(), SPV_ENV_VULKAN_1_0);
ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_0));
EXPECT_THAT(
getDiagnosticString(),
HasSubstr(
"OpEntryPoint contains duplicate output variables with Position"));
EXPECT_THAT(getDiagnosticString(),
AnyVUID("VUID-StandaloneSpirv-OpEntryPoint-09659"));
}

TEST_F(ValidateDecorations, MultipleBuiltinsBlockMixed) {
const std::string body = R"(
OpCapability Shader
OpMemoryModel Logical GLSL450
OpEntryPoint Vertex %main "main" %var %position
OpMemberDecorate %gl_PerVertex 0 BuiltIn Position
OpDecorate %gl_PerVertex Block
OpDecorate %position BuiltIn Position
%void = OpTypeVoid
%3 = OpTypeFunction %void
%float = OpTypeFloat 32
%v4float = OpTypeVector %float 4
%gl_PerVertex = OpTypeStruct %v4float
%_ptr_gl_PerVertex = OpTypePointer Output %gl_PerVertex
%var = OpVariable %_ptr_gl_PerVertex Output
%int = OpTypeInt 32 1
%int_0 = OpConstant %int 0
%int_1 = OpConstant %int 1
%float_0 = OpConstant %float 0
%17 = OpConstantComposite %v4float %float_0 %float_0 %float_0 %float_0
%ptr_vec4 = OpTypePointer Output %v4float
%position = OpVariable %ptr_vec4 Output
%main = OpFunction %void None %3
%5 = OpLabel
%19 = OpAccessChain %ptr_vec4 %var %int_0
OpStore %19 %17
OpStore %position %17
OpReturn
OpFunctionEnd
)";

CompileSuccessfully(body.c_str(), SPV_ENV_VULKAN_1_0);
ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_0));
EXPECT_THAT(
getDiagnosticString(),
HasSubstr(
"OpEntryPoint contains duplicate output variables with Position"));
EXPECT_THAT(getDiagnosticString(),
AnyVUID("VUID-StandaloneSpirv-OpEntryPoint-09659"));
}

} // namespace
Expand Down

0 comments on commit a5c8b0c

Please sign in to comment.