Skip to content

Commit 5a1eea1

Browse files
spencer-lunargspencer-lunarg
authored
spirv-val: Add SubpassData and TileImageEXT checks (KhronosGroup#6591)
Adds `VUID-StandaloneSpirv-SubpassData-04660` and `VUID-StandaloneSpirv-None-08720`
1 parent 7eda548 commit 5a1eea1

4 files changed

Lines changed: 192 additions & 8 deletions

File tree

‎source/val/validate_image.cpp‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1111,6 +1111,34 @@ spv_result_t ValidateImageCoordinate(ValidationState_t& _,
11111111
<< " components, but given only " << actual_coord_size;
11121112
}
11131113

1114+
if (info.dim == spv::Dim::SubpassData) {
1115+
const Instruction* coord_inst =
1116+
_.FindDef(inst->GetOperandAs<uint32_t>(word_index));
1117+
1118+
bool is_zero_vector = false;
1119+
if (coord_inst->opcode() == spv::Op::OpConstantNull) {
1120+
is_zero_vector = true;
1121+
} else if (coord_inst->opcode() == spv::Op::OpConstantComposite) {
1122+
// There is zero reason we should be allowing a OpSpecConstantComposite
1123+
if (coord_inst->words().size() == 5) {
1124+
uint64_t val_0 = 0;
1125+
uint64_t val_1 = 0;
1126+
if (_.EvalConstantValUint64(coord_inst->word(3), &val_0) &&
1127+
_.EvalConstantValUint64(coord_inst->word(4), &val_1) &&
1128+
val_0 == 0 && val_1 == 0) {
1129+
is_zero_vector = true;
1130+
}
1131+
}
1132+
}
1133+
1134+
if (!is_zero_vector) {
1135+
return _.diag(SPV_ERROR_INVALID_DATA, inst)
1136+
<< _.VkErrorID(4660)
1137+
<< "Expected Coordinate for a SubpassData image to be a "
1138+
"OpConstantComposite of (0,0) or OpConstantNull";
1139+
}
1140+
}
1141+
11141142
return SPV_SUCCESS;
11151143
}
11161144

‎source/val/validation_state.cpp‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -708,13 +708,32 @@ void ValidationState_t::RegisterStorageClassConsumer(
708708
*message =
709709
errorVUID +
710710
"in Vulkan environment, Workgroup Storage Class is limited "
711-
"to MeshNV, TaskNV, and GLCompute execution model";
711+
"to MeshEXT, TaskEXT, MeshNV, TaskNV, and GLCompute "
712+
"execution model";
712713
}
713714
return false;
714715
}
715716
return true;
716717
});
717718
}
719+
720+
if (storage_class == spv::StorageClass::TileImageEXT) {
721+
std::string errorVUID = VkErrorID(8720);
722+
function(consumer->function()->id())
723+
->RegisterExecutionModelLimitation(
724+
[errorVUID](spv::ExecutionModel model, std::string* message) {
725+
if (model != spv::ExecutionModel::Fragment) {
726+
if (message) {
727+
*message = errorVUID +
728+
"in Vulkan environment, TileImageEXT Storage "
729+
"Class is limited "
730+
"to Fragment execution model";
731+
}
732+
return false;
733+
}
734+
return true;
735+
});
736+
}
718737
}
719738

720739
if (storage_class == spv::StorageClass::CallableDataKHR) {
@@ -2656,6 +2675,8 @@ std::string ValidationState_t::VkErrorID(uint32_t id,
26562675
return VUID_WRAP(VUID-StandaloneSpirv-OpImageTexelPointer-04658);
26572676
case 4659:
26582677
return VUID_WRAP(VUID-StandaloneSpirv-OpImageQuerySizeLod-04659);
2678+
case 4660:
2679+
return VUID_WRAP(VUID-StandaloneSpirv-SubpassData-04660);
26592680
case 4664:
26602681
return VUID_WRAP(VUID-StandaloneSpirv-OpImageGather-04664);
26612682
case 4667:
@@ -2834,6 +2855,8 @@ std::string ValidationState_t::VkErrorID(uint32_t id,
28342855
return VUID_WRAP(VUID-StandaloneSpirv-Component-07703);
28352856
case 7951:
28362857
return VUID_WRAP(VUID-StandaloneSpirv-SubgroupVoteKHR-07951);
2858+
case 8720:
2859+
return VUID_WRAP(VUID-StandaloneSpirv-None-08720);
28372860
case 8721:
28382861
return VUID_WRAP(VUID-StandaloneSpirv-OpEntryPoint-08721);
28392862
case 8722:

‎test/val/val_atomics_test.cpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -282,8 +282,9 @@ TEST_F(ValidateAtomics, AtomicLoadVulkanWrongStorageClass) {
282282
AnyVUID("VUID-StandaloneSpirv-None-04645"));
283283
EXPECT_THAT(
284284
getDiagnosticString(),
285-
HasSubstr("in Vulkan environment, Workgroup Storage Class is limited to "
286-
"MeshNV, TaskNV, and GLCompute execution model"));
285+
HasSubstr(
286+
"in Vulkan environment, Workgroup Storage Class is limited to "
287+
"MeshEXT, TaskEXT, MeshNV, TaskNV, and GLCompute execution model"));
287288
}
288289

289290
TEST_F(ValidateAtomics, AtomicAddIntVulkanWrongType1) {
@@ -714,8 +715,9 @@ OpAtomicStore %f32_var %device %relaxed %f32_1
714715
AnyVUID("VUID-StandaloneSpirv-None-04645"));
715716
EXPECT_THAT(
716717
getDiagnosticString(),
717-
HasSubstr("in Vulkan environment, Workgroup Storage Class is limited to "
718-
"MeshNV, TaskNV, and GLCompute execution model"));
718+
HasSubstr(
719+
"in Vulkan environment, Workgroup Storage Class is limited to "
720+
"MeshEXT, TaskEXT, MeshNV, TaskNV, and GLCompute execution model"));
719721
}
720722

721723
TEST_F(ValidateAtomics, AtomicStoreFloatVulkan) {

‎test/val/val_image_test.cpp‎

Lines changed: 134 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,7 @@ OpDecorate %input_flat_u32 Location 0
210210
%struct_u32_f32vec4_u32 = OpTypeStruct %u32 %f32vec4 %u32
211211
%struct_u32_u32arr4 = OpTypeStruct %u32 %u32arr4
212212
213+
%u32vec2_00 = OpConstantComposite %u32vec2 %u32_0 %u32_0
213214
%u32vec2_01 = OpConstantComposite %u32vec2 %u32_0 %u32_1
214215
%u32vec2_12 = OpConstantComposite %u32vec2 %u32_1 %u32_2
215216
%u32vec3_012 = OpConstantComposite %u32vec3 %u32_0 %u32_1 %u32_2
@@ -1693,7 +1694,7 @@ TEST_F(ValidateImage, ImageTexelPointerImageNotResultTypePointer) {
16931694
CompileSuccessfully(GenerateShaderCode(body).c_str());
16941695
ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
16951696
EXPECT_THAT(getDiagnosticString(),
1696-
HasSubstr("Operand '156[%156]' cannot be a "
1697+
HasSubstr("Operand '157[%157]' cannot be a "
16971698
"type"));
16981699
}
16991700

@@ -4153,7 +4154,7 @@ TEST_F(ValidateImage, ReadSuccess3) {
41534154
TEST_F(ValidateImage, ReadSuccess4) {
41544155
const std::string body = R"(
41554156
%img = OpLoad %type_image_f32_spd_0002 %uniform_image_f32_spd_0002
4156-
%res1 = OpImageRead %f32vec4 %img %u32vec2_01
4157+
%res1 = OpImageRead %f32vec4 %img %u32vec2_00
41574158
)";
41584159

41594160
CompileSuccessfully(GenerateShaderCode(body).c_str());
@@ -5564,7 +5565,7 @@ OpExecutionMode %main LocalSize 8 8 1
55645565
TEST_F(ValidateImage, ReadSubpassDataWrongExecutionModel) {
55655566
const std::string body = R"(
55665567
%img = OpLoad %type_image_f32_spd_0002 %uniform_image_f32_spd_0002
5567-
%res1 = OpImageRead %f32vec4 %img %u32vec2_01
5568+
%res1 = OpImageRead %f32vec4 %img %u32vec2_00
55685569
)";
55695570

55705571
const std::string extra = "\nOpCapability StorageImageReadWithoutFormat\n";
@@ -11467,6 +11468,136 @@ TEST_F(ValidateImage, ImageTexelPointerNotAPointer) {
1146711468
HasSubstr("Expected Result Type to be a pointer"));
1146811469
}
1146911470

11471+
TEST_F(ValidateImage, TileImageNotFragment) {
11472+
const std::string body = R"(
11473+
OpCapability Shader
11474+
OpCapability TileImageColorReadAccessEXT
11475+
OpExtension "SPV_EXT_shader_tile_image"
11476+
OpMemoryModel Logical GLSL450
11477+
OpEntryPoint GLCompute %main "main"
11478+
OpExecutionMode %main LocalSize 1 1 1
11479+
%void = OpTypeVoid
11480+
%func = OpTypeFunction %void
11481+
%float = OpTypeFloat 32
11482+
%v4float = OpTypeVector %float 4
11483+
%ptr = OpTypePointer TileImageEXT %v4float
11484+
%var = OpVariable %ptr TileImageEXT
11485+
%main = OpFunction %void None %func
11486+
%label = OpLabel
11487+
%val = OpLoad %v4float %var
11488+
OpReturn
11489+
OpFunctionEnd
11490+
)";
11491+
11492+
CompileSuccessfully(body.c_str());
11493+
ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_0));
11494+
EXPECT_THAT(getDiagnosticString(),
11495+
AnyVUID("VUID-StandaloneSpirv-None-08720"));
11496+
EXPECT_THAT(
11497+
getDiagnosticString(),
11498+
HasSubstr(
11499+
"TileImageEXT Storage Class is limited to Fragment execution model"));
11500+
}
11501+
11502+
TEST_F(ValidateImage, SubpassDataNonZero) {
11503+
const std::string body = R"(
11504+
OpCapability Shader
11505+
OpCapability InputAttachment
11506+
OpMemoryModel Logical GLSL450
11507+
OpEntryPoint Fragment %main "main" %outColor %inputColor
11508+
OpExecutionMode %main OriginUpperLeft
11509+
OpDecorate %outColor Location 0
11510+
OpDecorate %inputColor Binding 0
11511+
OpDecorate %inputColor DescriptorSet 0
11512+
OpDecorate %inputColor InputAttachmentIndex 0
11513+
%void = OpTypeVoid
11514+
%4 = OpTypeFunction %void
11515+
%float = OpTypeFloat 32
11516+
%v4float = OpTypeVector %float 4
11517+
%_ptr_Output_v4float = OpTypePointer Output %v4float
11518+
%outColor = OpVariable %_ptr_Output_v4float Output
11519+
%11 = OpTypeImage %float SubpassData 0 0 0 2 Unknown
11520+
%_ptr_UniformConstant_11 = OpTypePointer UniformConstant %11
11521+
%inputColor = OpVariable %_ptr_UniformConstant_11 UniformConstant
11522+
%int = OpTypeInt 32 1
11523+
%int_0 = OpConstant %int 0
11524+
%int_1 = OpConstant %int 1
11525+
%v2int = OpTypeVector %int 2
11526+
%v2int_1 = OpConstantComposite %v2int %int_1 %int_1
11527+
%main = OpFunction %void None %4
11528+
%6 = OpLabel
11529+
%14 = OpLoad %11 %inputColor
11530+
%19 = OpImageRead %v4float %14 %v2int_1
11531+
OpStore %outColor %19
11532+
OpReturn
11533+
OpFunctionEnd
11534+
)";
11535+
11536+
CompileSuccessfully(body.c_str());
11537+
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_VULKAN_1_0));
11538+
EXPECT_THAT(getDiagnosticString(),
11539+
AnyVUID("VUID-StandaloneSpirv-SubpassData-04660"));
11540+
EXPECT_THAT(getDiagnosticString(),
11541+
HasSubstr("Expected Coordinate for a SubpassData image to be a "
11542+
"OpConstantComposite of (0,0) or OpConstantNull"));
11543+
}
11544+
11545+
TEST_F(ValidateImage, SubpassDataNonConstant) {
11546+
const std::string body = R"(
11547+
OpCapability Shader
11548+
OpCapability InputAttachment
11549+
%2 = OpExtInstImport "GLSL.std.450"
11550+
OpMemoryModel Logical GLSL450
11551+
OpEntryPoint Fragment %main "main" %_ %outColor %inputColor
11552+
OpExecutionMode %main OriginUpperLeft
11553+
OpDecorate %UBO Block
11554+
OpMemberDecorate %UBO 0 Offset 0
11555+
OpDecorate %_ Binding 0
11556+
OpDecorate %_ DescriptorSet 0
11557+
OpDecorate %outColor Location 0
11558+
OpDecorate %inputColor Binding 0
11559+
OpDecorate %inputColor DescriptorSet 0
11560+
OpDecorate %inputColor InputAttachmentIndex 0
11561+
%void = OpTypeVoid
11562+
%4 = OpTypeFunction %void
11563+
%uint = OpTypeInt 32 0
11564+
%v2uint = OpTypeVector %uint 2
11565+
%_ptr_Function_v2uint = OpTypePointer Function %v2uint
11566+
%UBO = OpTypeStruct %v2uint
11567+
%_ptr_Uniform_UBO = OpTypePointer Uniform %UBO
11568+
%_ = OpVariable %_ptr_Uniform_UBO Uniform
11569+
%int = OpTypeInt 32 1
11570+
%int_0 = OpConstant %int 0
11571+
%_ptr_Uniform_v2uint = OpTypePointer Uniform %v2uint
11572+
%float = OpTypeFloat 32
11573+
%v4float = OpTypeVector %float 4
11574+
%_ptr_Output_v4float = OpTypePointer Output %v4float
11575+
%outColor = OpVariable %_ptr_Output_v4float Output
11576+
%23 = OpTypeImage %float SubpassData 0 0 0 2 Unknown
11577+
%_ptr_UniformConstant_23 = OpTypePointer UniformConstant %23
11578+
%inputColor = OpVariable %_ptr_UniformConstant_23 UniformConstant
11579+
%v2int = OpTypeVector %int 2
11580+
%main = OpFunction %void None %4
11581+
%6 = OpLabel
11582+
%x = OpVariable %_ptr_Function_v2uint Function
11583+
%17 = OpAccessChain %_ptr_Uniform_v2uint %_ %int_0
11584+
%18 = OpLoad %v2uint %17
11585+
%26 = OpLoad %23 %inputColor
11586+
%29 = OpImageRead %v4float %26 %18
11587+
OpStore %outColor %29
11588+
OpReturn
11589+
OpFunctionEnd
11590+
)";
11591+
11592+
CompileSuccessfully(body.c_str());
11593+
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_VULKAN_1_0));
11594+
EXPECT_THAT(getDiagnosticString(),
11595+
AnyVUID("VUID-StandaloneSpirv-SubpassData-04660"));
11596+
EXPECT_THAT(getDiagnosticString(),
11597+
HasSubstr("Expected Coordinate for a SubpassData image to be a "
11598+
"OpConstantComposite of (0,0) or OpConstantNull"));
11599+
}
11600+
1147011601
} // namespace
1147111602
} // namespace val
1147211603
} // namespace spvtools

0 commit comments

Comments
 (0)