diff --git a/source/val/validate_annotation.cpp b/source/val/validate_annotation.cpp index b8ac6cb87a..703bf48423 100644 --- a/source/val/validate_annotation.cpp +++ b/source/val/validate_annotation.cpp @@ -53,9 +53,8 @@ bool IsMemberDecorationOnly(spv::Decoration dec) { case spv::Decoration::RowMajor: case spv::Decoration::ColMajor: case spv::Decoration::MatrixStride: - // SPIR-V spec bug? Offset is generated on variables when dealing with - // transform feedback. - // case spv::Decoration::Offset: + case spv::Decoration::Offset: + case spv::Decoration::OffsetIdEXT: return true; default: break; @@ -328,7 +327,13 @@ spv_result_t ValidateDecorate(ValidationState_t& _, const Instruction* inst) { } if (target->opcode() != spv::Op::OpDecorationGroup) { - if (IsMemberDecorationOnly(decoration)) { + // SPIR-V spec bug? (...example lost in time) + // Offset is generated on variables when dealing with transform feedback. + const bool tf_exception = + decoration == spv::Decoration::Offset && + _.HasCapability(spv::Capability::TransformFeedback); + + if (IsMemberDecorationOnly(decoration) && !tf_exception) { return _.diag(SPV_ERROR_INVALID_ID, inst) << _.SpvDecorationString(decoration) << " can only be applied to structure members"; @@ -409,11 +414,12 @@ spv_result_t ValidateDecorateId(ValidationState_t& _, const Instruction* inst) { } } - // No member decorations take id parameters, so we don't bother checking if - // we are using a member only decoration here. + if (IsMemberDecorationOnly(decoration)) { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << _.SpvDecorationString(decoration) + << " can only be applied to structure members"; + } - // TODO: Add validations for these decorations. - // UniformId is covered elsewhere. return SPV_SUCCESS; } diff --git a/test/val/val_annotation_test.cpp b/test/val/val_annotation_test.cpp index e4a947499c..0bc8186286 100644 --- a/test/val/val_annotation_test.cpp +++ b/test/val/val_annotation_test.cpp @@ -1157,6 +1157,84 @@ INSTANTIATE_TEST_SUITE_P(ValidateVulkanInterpolationStorageClass, VulkanInterpolationStorageClass, Values("Flat", "NoPerspective", "Centroid", "Sample")); +TEST_F(DecorationTest, OffsetOnStruct) { + const std::string text = R"( + OpCapability Shader + OpMemoryModel Logical GLSL450 + OpEntryPoint GLCompute %main "main" %x + OpExecutionMode %main LocalSize 1 1 1 + OpDecorate %ssbo Block + OpDecorate %ssbo Offset 0 + OpMemberDecorate %ssbo 0 Offset 0 + OpMemberDecorate %ssbo 1 Offset 4 + OpMemberDecorate %ssbo 2 Offset 8 + OpDecorate %x Binding 0 + OpDecorate %x DescriptorSet 0 + %void = OpTypeVoid + %3 = OpTypeFunction %void + %uint = OpTypeInt 32 0 + %ssbo = OpTypeStruct %uint %uint %uint + %uint_2 = OpConstant %uint 2 +%_arr_ssbo_uint_2 = OpTypeArray %ssbo %uint_2 +%_ptr_StorageBuffer__arr_ssbo_uint_2 = OpTypePointer StorageBuffer %_arr_ssbo_uint_2 + %x = OpVariable %_ptr_StorageBuffer__arr_ssbo_uint_2 StorageBuffer + %int = OpTypeInt 32 1 + %int_1 = OpConstant %int 1 + %uint_0 = OpConstant %uint 0 +%_ptr_StorageBuffer_uint = OpTypePointer StorageBuffer %uint + %main = OpFunction %void None %3 + %5 = OpLabel + %16 = OpAccessChain %_ptr_StorageBuffer_uint %x %int_1 %int_1 + OpStore %16 %uint_0 + OpReturn + OpFunctionEnd +)"; + + CompileSuccessfully(text, SPV_ENV_VULKAN_1_3); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_3)); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("Offset can only be applied to structure members")); +} + +TEST_F(DecorationTest, OffsetOnArray) { + const std::string text = R"( + OpCapability Shader + OpMemoryModel Logical GLSL450 + OpEntryPoint GLCompute %main "main" %x + OpExecutionMode %main LocalSize 1 1 1 + OpDecorate %ssbo Block + OpMemberDecorate %ssbo 0 Offset 0 + OpMemberDecorate %ssbo 1 Offset 4 + OpMemberDecorate %ssbo 2 Offset 8 + OpDecorate %x Binding 0 + OpDecorate %x DescriptorSet 0 + OpDecorate %_arr_ssbo_uint_2 Offset 0 + %void = OpTypeVoid + %3 = OpTypeFunction %void + %uint = OpTypeInt 32 0 + %ssbo = OpTypeStruct %uint %uint %uint + %uint_2 = OpConstant %uint 2 +%_arr_ssbo_uint_2 = OpTypeArray %ssbo %uint_2 +%_ptr_StorageBuffer__arr_ssbo_uint_2 = OpTypePointer StorageBuffer %_arr_ssbo_uint_2 + %x = OpVariable %_ptr_StorageBuffer__arr_ssbo_uint_2 StorageBuffer + %int = OpTypeInt 32 1 + %int_1 = OpConstant %int 1 + %uint_0 = OpConstant %uint 0 +%_ptr_StorageBuffer_uint = OpTypePointer StorageBuffer %uint + %main = OpFunction %void None %3 + %5 = OpLabel + %16 = OpAccessChain %_ptr_StorageBuffer_uint %x %int_1 %int_1 + OpStore %16 %uint_0 + OpReturn + OpFunctionEnd +)"; + + CompileSuccessfully(text, SPV_ENV_VULKAN_1_3); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_3)); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("Offset can only be applied to structure members")); +} + } // namespace } // namespace val } // namespace spvtools diff --git a/test/val/val_extension_spv_ext_descriptor_heap.cpp b/test/val/val_extension_spv_ext_descriptor_heap.cpp index e387546fe0..45ddc16480 100644 --- a/test/val/val_extension_spv_ext_descriptor_heap.cpp +++ b/test/val/val_extension_spv_ext_descriptor_heap.cpp @@ -1144,7 +1144,10 @@ TEST_F(ValidateSpvEXTDescriptorHeap, BufferPointerEXTStorageClass) { "type with a Storage Class of Uniform or StorageBuffer.")); } -TEST_F(ValidateSpvEXTDescriptorHeap, BufferPointerEXTLayout) { +// TODO - Seems like VUID-StandaloneSpirv-Result-11346 is wrong because +// AllowsLayout() shows Uniform and StorageBuffer (the only two allowed) +// are always explicit layout +TEST_F(ValidateSpvEXTDescriptorHeap, DISABLED_BufferPointerEXTLayout) { const std::string str = R"( OpCapability Shader OpCapability UntypedPointersKHR @@ -1167,12 +1170,13 @@ TEST_F(ValidateSpvEXTDescriptorHeap, BufferPointerEXTLayout) { OpDecorate %U Block OpMemberDecorate %U 0 Offset 0 OpDecorateId %_runtimearr_17 ArrayStrideIdEXT %18 - OpDecorate %_ptr_UniformConstant Offset 0 + ; OpDecorate %_ptr_Uniform Offset 0 %void = OpTypeVoid %3 = OpTypeFunction %void %uint = OpTypeInt 32 0 %_ptr_Output_uint = OpTypePointer Output %uint %o = OpVariable %_ptr_Output_uint Output +%_ptr_Uniform = OpTypeUntypedPointerKHR Uniform %_ptr_UniformConstant = OpTypeUntypedPointerKHR UniformConstant %resource_heap = OpUntypedVariableKHR %_ptr_UniformConstant UniformConstant %int = OpTypeInt 32 1 @@ -1185,8 +1189,8 @@ TEST_F(ValidateSpvEXTDescriptorHeap, BufferPointerEXTLayout) { %main = OpFunction %void None %3 %5 = OpLabel %16 = OpUntypedAccessChainKHR %_ptr_UniformConstant %_runtimearr_17 %resource_heap %int_9 - %20 = OpBufferPointerEXT %_ptr_UniformConstant %16 - %21 = OpUntypedAccessChainKHR %_ptr_UniformConstant %U %20 %int_0 + %20 = OpBufferPointerEXT %_ptr_Uniform %16 + %21 = OpUntypedAccessChainKHR %_ptr_Uniform %U %20 %int_0 %22 = OpLoad %uint %21 OpStore %o %22 OpReturn @@ -2417,6 +2421,47 @@ TEST_F(ValidateSpvEXTDescriptorHeap, ArrayStrideSpecConstantZero) { EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_4)); } +// https://github.com/KhronosGroup/SPIRV-Tools/issues/6696 +TEST_F(ValidateSpvEXTDescriptorHeap, OffsetIdOnArray) { + const std::string str = R"( + OpCapability Shader + OpCapability UntypedPointersKHR + OpCapability DescriptorHeapEXT + OpCapability Sampled1D + OpExtension "SPV_KHR_untyped_pointers" + OpExtension "SPV_EXT_descriptor_heap" + OpMemoryModel Logical GLSL450 + OpEntryPoint GLCompute %1 "main" %2 + OpExecutionMode %1 LocalSize 1 1 1 + OpDecorate %2 BuiltIn ResourceHeapEXT + OpDecorateId %7 OffsetIdEXT %4 + %11 = OpTypeVoid + %12 = OpTypeFunction %11 + %13 = OpTypeInt 32 0 + %16 = OpConstant %13 0 + %4 = OpConstant %13 0 + %20 = OpTypeImage %13 1D 0 0 0 1 Unknown + %21 = OpTypeBufferEXT StorageBuffer + %22 = OpTypeSampler + %23 = OpTypeSampledImage %20 + %7 = OpTypeRuntimeArray %20 + %24 = OpTypeUntypedPointerKHR UniformConstant + %25 = OpTypeUntypedPointerKHR StorageBuffer + %2 = OpUntypedVariableKHR %24 UniformConstant + %1 = OpFunction %11 None %12 + %26 = OpLabel + %27 = OpUntypedAccessChainKHR %24 %7 %2 %16 + %28 = OpLoad %20 %27 + OpReturn + OpFunctionEnd + )"; + CompileSuccessfully(str.c_str(), SPV_ENV_VULKAN_1_4); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_4)); + EXPECT_THAT( + getDiagnosticString(), + HasSubstr("OffsetIdEXT can only be applied to structure members")); +} + } // namespace } // namespace val } // namespace spvtools