Skip to content

Commit da48bb2

Browse files
spencer-lunargspencer-lunarg
authored
val: Fix CullPrimitiveEXT array of bool (#6155)
* val: Fix CullPrimitiveEXT array of bool * val: Add CullPrimitiveEXT Block case
1 parent 0102146 commit da48bb2

File tree

2 files changed

+176
-3
lines changed

2 files changed

+176
-3
lines changed

‎source/val/validate_builtins.cpp

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,9 @@ class BuiltInsValidator {
590590
spv_result_t ValidateBool(
591591
const Decoration& decoration, const Instruction& inst,
592592
const std::function<spv_result_t(const std::string& message)>& diag);
593+
spv_result_t ValidateBlockBoolOrArrayedBool(
594+
const Decoration& decoration, const Instruction& inst,
595+
const std::function<spv_result_t(const std::string& message)>& diag);
593596
spv_result_t ValidateI(
594597
const Decoration& decoration, const Instruction& inst,
595598
const std::function<spv_result_t(const std::string& message)>& diag);
@@ -820,6 +823,30 @@ spv_result_t BuiltInsValidator::ValidateBool(
820823
return SPV_SUCCESS;
821824
}
822825

826+
spv_result_t BuiltInsValidator::ValidateBlockBoolOrArrayedBool(
827+
const Decoration& decoration, const Instruction& inst,
828+
const std::function<spv_result_t(const std::string& message)>& diag) {
829+
uint32_t underlying_type = 0;
830+
if (spv_result_t error =
831+
GetUnderlyingType(_, decoration, inst, &underlying_type)) {
832+
return error;
833+
}
834+
// Strip the array, if present.
835+
if (_.GetIdOpcode(underlying_type) == spv::Op::OpTypeArray) {
836+
underlying_type = _.FindDef(underlying_type)->word(2u);
837+
} else if (!_.HasDecoration(inst.id(), spv::Decoration::Block)) {
838+
// If not in array, and bool is in a struct, must be in a Block struct
839+
return diag(GetDefinitionDesc(decoration, inst) +
840+
" Scalar boolean must be in a Block.");
841+
}
842+
843+
if (!_.IsBoolScalarType(underlying_type)) {
844+
return diag(GetDefinitionDesc(decoration, inst) + " is not a bool scalar.");
845+
}
846+
847+
return SPV_SUCCESS;
848+
}
849+
823850
spv_result_t BuiltInsValidator::ValidateI(
824851
const Decoration& decoration, const Instruction& inst,
825852
const std::function<spv_result_t(const std::string& message)>& diag) {
@@ -4339,7 +4366,7 @@ spv_result_t BuiltInsValidator::ValidateMeshShadingEXTBuiltinsAtDefinition(
43394366
}
43404367
break;
43414368
case spv::BuiltIn::CullPrimitiveEXT:
4342-
if (spv_result_t error = ValidateBool(
4369+
if (spv_result_t error = ValidateBlockBoolOrArrayedBool(
43434370
decoration, inst,
43444371
[this, &inst, &decoration,
43454372
&vuid](const std::string& message) -> spv_result_t {
@@ -4350,8 +4377,8 @@ spv_result_t BuiltInsValidator::ValidateMeshShadingEXTBuiltinsAtDefinition(
43504377
<< _.grammar().lookupOperandName(
43514378
SPV_OPERAND_TYPE_BUILT_IN,
43524379
(uint32_t)decoration.builtin())
4353-
<< " variable needs to be a boolean value "
4354-
"array."
4380+
<< " variable needs to be a either a boolean or an "
4381+
"array of booleans."
43554382
<< message;
43564383
})) {
43574384
return error;

‎test/val/val_builtins_test.cpp

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5255,6 +5255,50 @@ TEST_F(ValidateBuiltIns, VulkanBuiltinCullPrimitiveEXT) {
52555255
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_3));
52565256
}
52575257

5258+
TEST_F(ValidateBuiltIns, VulkanBuiltinCullPrimitiveEXTMissingBlock) {
5259+
const std::string text = R"(
5260+
OpCapability MeshShadingEXT
5261+
OpExtension "SPV_EXT_mesh_shader"
5262+
%1 = OpExtInstImport "GLSL.std.450"
5263+
OpMemoryModel Logical GLSL450
5264+
OpEntryPoint MeshEXT %main "main" %gl_MeshPrimitivesEXT
5265+
OpExecutionModeId %main LocalSizeId %uint_32 %uint_1 %uint_1
5266+
OpExecutionMode %main OutputVertices 81
5267+
OpExecutionMode %main OutputPrimitivesEXT 32
5268+
OpExecutionMode %main OutputTrianglesEXT
5269+
OpSource GLSL 450
5270+
OpSourceExtension "GL_EXT_mesh_shader"
5271+
OpName %main "main"
5272+
OpName %gl_MeshPerPrimitiveEXT "gl_MeshPerPrimitiveEXT"
5273+
OpMemberName %gl_MeshPerPrimitiveEXT 0 "gl_CullPrimitiveEXT"
5274+
OpName %gl_MeshPrimitivesEXT "gl_MeshPrimitivesEXT"
5275+
OpMemberDecorate %gl_MeshPerPrimitiveEXT 0 BuiltIn CullPrimitiveEXT
5276+
OpMemberDecorate %gl_MeshPerPrimitiveEXT 0 PerPrimitiveEXT
5277+
%void = OpTypeVoid
5278+
%3 = OpTypeFunction %void
5279+
%uint = OpTypeInt 32 0
5280+
%uint_32 = OpConstant %uint 32
5281+
%uint_1 = OpConstant %uint 1
5282+
%int = OpTypeInt 32 1
5283+
%bool = OpTypeBool
5284+
%gl_MeshPerPrimitiveEXT = OpTypeStruct %bool
5285+
%_arr_gl_MeshPerPrimitiveEXT_uint_32 = OpTypeArray %gl_MeshPerPrimitiveEXT %uint_32
5286+
%_ptr_Output__arr_gl_MeshPerPrimitiveEXT_uint_32 = OpTypePointer Output %_arr_gl_MeshPerPrimitiveEXT_uint_32
5287+
%gl_MeshPrimitivesEXT = OpVariable %_ptr_Output__arr_gl_MeshPerPrimitiveEXT_uint_32 Output
5288+
%main = OpFunction %void None %3
5289+
%5 = OpLabel
5290+
OpReturn
5291+
OpFunctionEnd
5292+
)";
5293+
5294+
CompileSuccessfully(text, SPV_ENV_VULKAN_1_3);
5295+
EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_VULKAN_1_3));
5296+
EXPECT_THAT(getDiagnosticString(),
5297+
AnyVUID("VUID-CullPrimitiveEXT-CullPrimitiveEXT-07036"));
5298+
EXPECT_THAT(getDiagnosticString(),
5299+
AnyVUID("Scalar boolean must be in a Block"));
5300+
}
5301+
52585302
TEST_F(ValidateBuiltIns, BadVulkanBuiltinCullPrimitiveEXTType) {
52595303
const std::string text = R"(
52605304
OpCapability MeshShadingEXT
@@ -5298,6 +5342,108 @@ TEST_F(ValidateBuiltIns, BadVulkanBuiltinCullPrimitiveEXTType) {
52985342
AnyVUID("VUID-CullPrimitiveEXT-CullPrimitiveEXT-07036"));
52995343
}
53005344

5345+
// from https://github.com/KhronosGroup/SPIRV-Tools/issues/5980
5346+
TEST_F(ValidateBuiltIns, VulkanBuiltinCullPrimitiveArrayOfBool) {
5347+
const std::string text = R"(
5348+
OpCapability MeshShadingEXT
5349+
OpExtension "SPV_EXT_mesh_shader"
5350+
OpMemoryModel Logical GLSL450
5351+
OpEntryPoint MeshEXT %main "main" %gl_LocalInvocationIndex %gl_Position %4 %5
5352+
OpExecutionMode %main LocalSize 2 1 1
5353+
OpExecutionMode %main OutputTrianglesEXT
5354+
OpExecutionMode %main OutputVertices 2
5355+
OpExecutionMode %main OutputPrimitivesEXT 2
5356+
OpDecorate %gl_LocalInvocationIndex BuiltIn LocalInvocationIndex
5357+
OpDecorate %gl_Position BuiltIn Position
5358+
OpDecorate %4 BuiltIn PrimitiveTriangleIndicesEXT
5359+
OpDecorate %5 BuiltIn CullPrimitiveEXT
5360+
OpDecorate %5 PerPrimitiveEXT
5361+
%uint = OpTypeInt 32 0
5362+
%uint_2 = OpConstant %uint 2
5363+
%bool = OpTypeBool
5364+
%false = OpConstantFalse %bool
5365+
%_ptr_Input_uint = OpTypePointer Input %uint
5366+
%float = OpTypeFloat 32
5367+
%v4float = OpTypeVector %float 4
5368+
%_arr_v4float_uint_2 = OpTypeArray %v4float %uint_2
5369+
%_ptr_Output__arr_v4float_uint_2 = OpTypePointer Output %_arr_v4float_uint_2
5370+
%v3uint = OpTypeVector %uint 3
5371+
%_arr_v3uint_uint_2 = OpTypeArray %v3uint %uint_2
5372+
%_ptr_Output__arr_v3uint_uint_2 = OpTypePointer Output %_arr_v3uint_uint_2
5373+
%_arr_bool_uint_2 = OpTypeArray %bool %uint_2
5374+
%_ptr_Output__arr_bool_uint_2 = OpTypePointer Output %_arr_bool_uint_2
5375+
%void = OpTypeVoid
5376+
%21 = OpTypeFunction %void
5377+
%_ptr_Output_bool = OpTypePointer Output %bool
5378+
%gl_LocalInvocationIndex = OpVariable %_ptr_Input_uint Input
5379+
%gl_Position = OpVariable %_ptr_Output__arr_v4float_uint_2 Output
5380+
%4 = OpVariable %_ptr_Output__arr_v3uint_uint_2 Output
5381+
%5 = OpVariable %_ptr_Output__arr_bool_uint_2 Output
5382+
%main = OpFunction %void None %21
5383+
%23 = OpLabel
5384+
%24 = OpLoad %uint %gl_LocalInvocationIndex
5385+
OpSetMeshOutputsEXT %uint_2 %uint_2
5386+
%25 = OpAccessChain %_ptr_Output_bool %5 %24
5387+
OpStore %25 %false
5388+
OpReturn
5389+
OpFunctionEnd
5390+
)";
5391+
5392+
CompileSuccessfully(text, SPV_ENV_VULKAN_1_3);
5393+
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_3));
5394+
}
5395+
5396+
TEST_F(ValidateBuiltIns, BadVulkanBuiltinCullPrimitiveArrayOfBool) {
5397+
const std::string text = R"(
5398+
OpCapability MeshShadingEXT
5399+
OpExtension "SPV_EXT_mesh_shader"
5400+
OpMemoryModel Logical GLSL450
5401+
OpEntryPoint MeshEXT %main "main" %gl_LocalInvocationIndex %gl_Position %4 %5
5402+
OpExecutionMode %main LocalSize 2 1 1
5403+
OpExecutionMode %main OutputTrianglesEXT
5404+
OpExecutionMode %main OutputVertices 2
5405+
OpExecutionMode %main OutputPrimitivesEXT 2
5406+
OpDecorate %gl_LocalInvocationIndex BuiltIn LocalInvocationIndex
5407+
OpDecorate %gl_Position BuiltIn Position
5408+
OpDecorate %4 BuiltIn PrimitiveTriangleIndicesEXT
5409+
OpDecorate %5 BuiltIn CullPrimitiveEXT
5410+
OpDecorate %5 PerPrimitiveEXT
5411+
%uint = OpTypeInt 32 0
5412+
%uint_2 = OpConstant %uint 2
5413+
%bool = OpTypeBool
5414+
%false = OpConstantFalse %bool
5415+
%_ptr_Input_uint = OpTypePointer Input %uint
5416+
%float = OpTypeFloat 32
5417+
%v4float = OpTypeVector %float 4
5418+
%_arr_v4float_uint_2 = OpTypeArray %v4float %uint_2
5419+
%_ptr_Output__arr_v4float_uint_2 = OpTypePointer Output %_arr_v4float_uint_2
5420+
%v3uint = OpTypeVector %uint 3
5421+
%_arr_v3uint_uint_2 = OpTypeArray %v3uint %uint_2
5422+
%_ptr_Output__arr_v3uint_uint_2 = OpTypePointer Output %_arr_v3uint_uint_2
5423+
%_arr_uint_uint_2 = OpTypeArray %uint %uint_2
5424+
%_ptr_Output__arr_uint_uint_2 = OpTypePointer Output %_arr_uint_uint_2
5425+
%void = OpTypeVoid
5426+
%21 = OpTypeFunction %void
5427+
%_ptr_Output_uint = OpTypePointer Output %uint
5428+
%gl_LocalInvocationIndex = OpVariable %_ptr_Input_uint Input
5429+
%gl_Position = OpVariable %_ptr_Output__arr_v4float_uint_2 Output
5430+
%4 = OpVariable %_ptr_Output__arr_v3uint_uint_2 Output
5431+
%5 = OpVariable %_ptr_Output__arr_uint_uint_2 Output
5432+
%main = OpFunction %void None %21
5433+
%23 = OpLabel
5434+
%24 = OpLoad %uint %gl_LocalInvocationIndex
5435+
OpSetMeshOutputsEXT %uint_2 %uint_2
5436+
%25 = OpAccessChain %_ptr_Output_uint %5 %24
5437+
OpReturn
5438+
OpFunctionEnd
5439+
)";
5440+
5441+
CompileSuccessfully(text, SPV_ENV_VULKAN_1_3);
5442+
EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_VULKAN_1_3));
5443+
EXPECT_THAT(getDiagnosticString(),
5444+
AnyVUID("VUID-CullPrimitiveEXT-CullPrimitiveEXT-07036"));
5445+
}
5446+
53015447
TEST_F(ValidateBuiltIns, BadVulkanBuiltinCullPrimitiveEXTStorageClass) {
53025448
const std::string text = R"(
53035449
OpCapability MeshShadingEXT

0 commit comments

Comments
 (0)