From daec145d41014198f84861bd99032f68e646a6f0 Mon Sep 17 00:00:00 2001 From: spencer-lunarg Date: Tue, 7 Nov 2023 15:46:06 -0700 Subject: [PATCH] layers: Have single way to get SpecConstant values --- layers/core_checks/cc_spirv.cpp | 149 +++++++++++-------------- layers/state_tracker/shader_module.cpp | 1 - layers/state_tracker/shader_module.h | 5 +- layers/utils/shader_utils.cpp | 25 +++++ layers/utils/shader_utils.h | 2 + tests/unit/shader_spirv.cpp | 88 ++++++++++++++- tests/unit/shader_spirv_positive.cpp | 108 ++++++++++++++++++ tests/unit/subpass.cpp | 32 +++++- tests/unit/subpass_positive.cpp | 29 +++++ 9 files changed, 346 insertions(+), 93 deletions(-) diff --git a/layers/core_checks/cc_spirv.cpp b/layers/core_checks/cc_spirv.cpp index 1718415e00f..7ade0e76695 100644 --- a/layers/core_checks/cc_spirv.cpp +++ b/layers/core_checks/cc_spirv.cpp @@ -423,34 +423,6 @@ bool CoreChecks::ValidateShaderStorageImageFormatsVariables(const SPIRV_MODULE_S return skip; } -// copy the specialization constant value into buf, if it is present -void GetSpecConstantValue(const safe_VkSpecializationInfo *spec, uint32_t spec_id, void *buf) { - if (spec && spec_id < spec->mapEntryCount) { - memcpy(buf, (uint8_t *)spec->pData + spec->pMapEntries[spec_id].offset, spec->pMapEntries[spec_id].size); - } -} - -// Fill in value with the constant or specialization constant value, if available. -// Returns true if the value has been accurately filled out. -static bool GetIntConstantValue(const Instruction *insn, const SPIRV_MODULE_STATE &module_state, - const safe_VkSpecializationInfo *spec, uint32_t *value) { - const Instruction *type_id = module_state.FindDef(insn->Word(1)); - if (type_id->Opcode() != spv::OpTypeInt || type_id->Word(2) != 32) { - return false; - } - switch (insn->Opcode()) { - case spv::OpSpecConstant: - *value = insn->Word(3); - GetSpecConstantValue(spec, module_state.static_data_.id_to_spec_id.at(insn->Word(2)), value); - return true; - case spv::OpConstant: - *value = insn->Word(3); - return true; - default: - return false; - } -} - // Map SPIR-V type to VK_COMPONENT_TYPE enum VkComponentTypeKHR GetComponentType(const Instruction *insn) { switch (insn->Opcode()) { @@ -490,8 +462,6 @@ bool CoreChecks::ValidateCooperativeMatrix(const SPIRV_MODULE_STATE &module_stat const Location &loc) const { bool skip = false; - const safe_VkSpecializationInfo *spec = stage_state.GetSpecializationInfo(); - struct CoopMatType { VkScopeKHR scope; uint32_t rows; @@ -500,7 +470,7 @@ bool CoreChecks::ValidateCooperativeMatrix(const SPIRV_MODULE_STATE &module_stat bool all_constant; bool is_signed_int; - CoopMatType(uint32_t id, const SPIRV_MODULE_STATE &module_state, const safe_VkSpecializationInfo *spec) { + CoopMatType(uint32_t id, const SPIRV_MODULE_STATE &module_state, const PipelineStageState &stage_state) { const Instruction *insn = module_state.FindDef(id); const Instruction *component_type_insn = module_state.FindDef(insn->Word(2)); const Instruction *scope_insn = module_state.FindDef(insn->Word(3)); @@ -509,14 +479,14 @@ bool CoreChecks::ValidateCooperativeMatrix(const SPIRV_MODULE_STATE &module_stat all_constant = true; uint32_t tmp_scope = 0; // TODO - Remove GetIntConstantValue - if (!GetIntConstantValue(scope_insn, module_state, spec, &tmp_scope)) { + if (!stage_state.GetInt32ConstantValue(*scope_insn, &tmp_scope)) { all_constant = false; } scope = VkScopeKHR(tmp_scope); - if (!GetIntConstantValue(rows_insn, module_state, spec, &rows)) { + if (!stage_state.GetInt32ConstantValue(*rows_insn, &rows)) { all_constant = false; } - if (!GetIntConstantValue(cols_insn, module_state, spec, &cols)) { + if (!stage_state.GetInt32ConstantValue(*cols_insn, &cols)) { all_constant = false; } component_type = GetComponentType(component_type_insn); @@ -569,7 +539,7 @@ bool CoreChecks::ValidateCooperativeMatrix(const SPIRV_MODULE_STATE &module_stat const Instruction &insn = *cooperative_matrix_inst; switch (insn.Opcode()) { case spv::OpTypeCooperativeMatrixKHR: { - CoopMatType m(insn.Word(1), module_state, spec); + CoopMatType m(insn.Word(1), module_state, stage_state); if (m.all_constant) { // Validate that the type parameters are all supported for one of the @@ -615,10 +585,10 @@ bool CoreChecks::ValidateCooperativeMatrix(const SPIRV_MODULE_STATE &module_stat break; } case spv::OpCooperativeMatrixMulAddKHR: { - CoopMatType r(id_to_type_id[insn.Word(2)], module_state, spec); - CoopMatType a(id_to_type_id[insn.Word(3)], module_state, spec); - CoopMatType b(id_to_type_id[insn.Word(4)], module_state, spec); - CoopMatType c(id_to_type_id[insn.Word(5)], module_state, spec); + CoopMatType r(id_to_type_id[insn.Word(2)], module_state, stage_state); + CoopMatType a(id_to_type_id[insn.Word(3)], module_state, stage_state); + CoopMatType b(id_to_type_id[insn.Word(4)], module_state, stage_state); + CoopMatType c(id_to_type_id[insn.Word(5)], module_state, stage_state); const uint32_t flags = insn.Length() > 6 ? insn.Word(6) : 0u; if (a.is_signed_int && ((flags & spv::CooperativeMatrixOperandsMatrixASignedComponentsKHRMask) == 0)) { skip |= LogError( @@ -734,7 +704,7 @@ bool CoreChecks::ValidateCooperativeMatrix(const SPIRV_MODULE_STATE &module_stat break; } case spv::OpTypeCooperativeMatrixNV: { - CoopMatType m(insn.Word(1), module_state, spec); + CoopMatType m(insn.Word(1), module_state, stage_state); if (m.all_constant) { // Validate that the type parameters are all supported for one of the @@ -776,10 +746,10 @@ bool CoreChecks::ValidateCooperativeMatrix(const SPIRV_MODULE_STATE &module_stat break; } case spv::OpCooperativeMatrixMulAddNV: { - CoopMatType d(id_to_type_id[insn.Word(2)], module_state, spec); - CoopMatType a(id_to_type_id[insn.Word(3)], module_state, spec); - CoopMatType b(id_to_type_id[insn.Word(4)], module_state, spec); - CoopMatType c(id_to_type_id[insn.Word(5)], module_state, spec); + CoopMatType d(id_to_type_id[insn.Word(2)], module_state, stage_state); + CoopMatType a(id_to_type_id[insn.Word(3)], module_state, stage_state); + CoopMatType b(id_to_type_id[insn.Word(4)], module_state, stage_state); + CoopMatType c(id_to_type_id[insn.Word(5)], module_state, stage_state); if (a.all_constant && b.all_constant && c.all_constant && d.all_constant) { // Validate that the type parameters are all supported for the same @@ -2126,46 +2096,57 @@ bool CoreChecks::ValidatePipelineShaderStage(const StageCreateInfo &stage_create auto const &specialization_data = reinterpret_cast(specialization_info->pData); std::unordered_map> id_value_map; // note: this must be std:: to work with spvtools id_value_map.reserve(specialization_info->mapEntryCount); - for (uint32_t i = 0; i < specialization_info->mapEntryCount; ++i) { - auto const &map_entry = specialization_info->pMapEntries[i]; - const auto itr = module_state.static_data_.spec_const_map.find(map_entry.constantID); + + // spirv-val makes sure every OpSpecConstant has a OpDecoration. + for (const auto &itr : module_state.static_data_.id_to_spec_id) { + const uint32_t spec_id = itr.second; + VkSpecializationMapEntry map_entry = {kInvalidSpirvValue, 0, 0}; + for (uint32_t i = 0; i < specialization_info->mapEntryCount; i++) { + if (specialization_info->pMapEntries[i].constantID == spec_id) { + map_entry = specialization_info->pMapEntries[i]; + break; + } + } + // "If a constantID value is not a specialization constant ID used in the shader, that map entry does not affect the // behavior of the pipeline." - if (itr != module_state.static_data_.spec_const_map.cend()) { - // Make sure map_entry.size matches the spec constant's size - uint32_t spec_const_size = kInvalidSpirvValue; - const Instruction *def_insn = module_state.FindDef(itr->second); - const Instruction *type_insn = module_state.FindDef(def_insn->Word(1)); - // Specialization constants can only be of type bool, scalar integer, or scalar floating point - switch (type_insn->Opcode()) { - case spv::OpTypeBool: - // "If the specialization constant is of type boolean, size must be the byte size of VkBool32" - spec_const_size = sizeof(VkBool32); - break; - case spv::OpTypeInt: - case spv::OpTypeFloat: - spec_const_size = type_insn->Word(2) / 8; - break; - default: - // spirv-val should catch if SpecId is not used on a - // OpSpecConstantTrue/OpSpecConstantFalse/OpSpecConstant and OpSpecConstant is validated to be a - // OpTypeInt or OpTypeFloat - break; - } + if (map_entry.constantID == kInvalidSpirvValue) { + continue; + } - if (map_entry.size != spec_const_size) { - std::stringstream name; - if (module_state.handle()) { - name << "shader module " << module_state.handle(); - } else { - name << "shader object"; - } - skip |= LogError("VUID-VkSpecializationMapEntry-constantID-00776", device, loc, - "specialization constant (ID = %" PRIu32 ", entry = %" PRIu32 - ") has invalid size %zu in %s. Expected size is %" PRIu32 " from shader definition.", - map_entry.constantID, i, map_entry.size, FormatHandle(module_state.handle()).c_str(), - spec_const_size); + uint32_t spec_const_size = kInvalidSpirvValue; + const Instruction *def_insn = module_state.FindDef(itr.first); + const Instruction *type_insn = module_state.FindDef(def_insn->Word(1)); + + // Specialization constants can only be of type bool, scalar integer, or scalar floating point + switch (type_insn->Opcode()) { + case spv::OpTypeBool: + // "If the specialization constant is of type boolean, size must be the byte size of VkBool32" + spec_const_size = sizeof(VkBool32); + break; + case spv::OpTypeInt: + case spv::OpTypeFloat: + spec_const_size = type_insn->Word(2) / 8; + break; + default: + // spirv-val should catch if SpecId is not used on a + // OpSpecConstantTrue/OpSpecConstantFalse/OpSpecConstant and OpSpecConstant is validated to be a + // OpTypeInt or OpTypeFloat + break; + } + + if (map_entry.size != spec_const_size) { + std::stringstream name; + if (module_state.handle()) { + name << "shader module " << module_state.handle(); + } else { + name << "shader object"; } + skip |= LogError("VUID-VkSpecializationMapEntry-constantID-00776", device, loc, + "specialization constant (ID = %" PRIu32 ", entry = %" PRIu32 + ") has invalid size %zu in %s. Expected size is %" PRIu32 " from shader definition.", + map_entry.constantID, spec_id, map_entry.size, FormatHandle(module_state.handle()).c_str(), + spec_const_size); } if ((map_entry.offset + map_entry.size) <= specialization_info->dataSize) { @@ -2737,14 +2718,12 @@ bool CoreChecks::ValidateEmitMeshTasksSize(const SPIRV_MODULE_STATE &module_stat const PipelineStageState &stage_state, const Location &loc) const { bool skip = false; - const safe_VkSpecializationInfo *spec = stage_state.GetSpecializationInfo(); - for (const Instruction &insn : module_state.static_data_.instructions) { if (insn.Opcode() == spv::OpEmitMeshTasksEXT) { uint32_t x, y, z; - bool found_x = GetIntConstantValue(module_state.FindDef(insn.Word(1)), module_state, spec, &x); - bool found_y = GetIntConstantValue(module_state.FindDef(insn.Word(2)), module_state, spec, &y); - bool found_z = GetIntConstantValue(module_state.FindDef(insn.Word(3)), module_state, spec, &z); + bool found_x = stage_state.GetInt32ConstantValue(*module_state.FindDef(insn.Word(1)), &x); + bool found_y = stage_state.GetInt32ConstantValue(*module_state.FindDef(insn.Word(2)), &y); + bool found_z = stage_state.GetInt32ConstantValue(*module_state.FindDef(insn.Word(3)), &z); if (found_x && x > phys_dev_ext_props.mesh_shader_props_ext.maxMeshWorkGroupCount[0]) { skip |= LogError("VUID-RuntimeSpirv-TaskEXT-07299", module_state.handle(), loc, "SPIR-V (%s) is emitting %" PRIu32 diff --git a/layers/state_tracker/shader_module.cpp b/layers/state_tracker/shader_module.cpp index 4d84af6b062..63b9c98ac6a 100644 --- a/layers/state_tracker/shader_module.cpp +++ b/layers/state_tracker/shader_module.cpp @@ -803,7 +803,6 @@ SPIRV_MODULE_STATE::StaticData::StaticData(const SPIRV_MODULE_STATE& module_stat if (insn.Word(2) == spv::DecorationBuiltIn) { builtin_decoration_inst.push_back(&insn); } else if (insn.Word(2) == spv::DecorationSpecId) { - spec_const_map[insn.Word(3)] = target_id; id_to_spec_id[target_id] = insn.Word(3); } } break; diff --git a/layers/state_tracker/shader_module.h b/layers/state_tracker/shader_module.h index d725027a320..0a428ef20df 100644 --- a/layers/state_tracker/shader_module.h +++ b/layers/state_tracker/shader_module.h @@ -472,10 +472,7 @@ struct SPIRV_MODULE_STATE { vvl::unordered_map execution_modes; ExecutionModeSet empty_execution_mode; // all zero values, allows use to return a reference and not a copy each time - // target ID> mapping - vvl::unordered_map spec_const_map; - // Specialization constant ID> mapping - // TODO - Remove having a second copy for the map in reverse + // [OpSpecConstant Result ID -> OpDecorate SpecID value] mapping vvl::unordered_map id_to_spec_id; // Find all decoration instructions to prevent relooping module later - many checks need this info std::vector decoration_inst; diff --git a/layers/utils/shader_utils.cpp b/layers/utils/shader_utils.cpp index 6d8918401bc..e0b782fa510 100644 --- a/layers/utils/shader_utils.cpp +++ b/layers/utils/shader_utils.cpp @@ -23,6 +23,7 @@ #include "generated/state_tracker_helper.h" #include "generated/vk_extension_helper.h" #include "state_tracker/shader_module.h" +#include "state_tracker/shader_instruction.h" spv_target_env PickSpirvEnv(const APIVersion &api_version, bool spirv_1_4) { if (api_version >= VK_API_VERSION_1_3) { @@ -125,6 +126,30 @@ const void *PipelineStageState::GetPNext() const { return (pipeline_create_info) ? pipeline_create_info->pNext : shader_object_create_info->pNext; } +bool PipelineStageState::GetInt32ConstantValue(const Instruction &insn, uint32_t *value) const { + const Instruction *type_id = spirv_state->FindDef(insn.Word(1)); + if (type_id->Opcode() != spv::OpTypeInt || type_id->Word(2) != 32) { + return false; + } + + if (insn.Opcode() == spv::OpConstant) { + *value = insn.Word(3); + return true; + } else if (insn.Opcode() == spv::OpSpecConstant) { + *value = insn.Word(3); // default value + const auto *spec_info = GetSpecializationInfo(); + const uint32_t spec_id = spirv_state->static_data_.id_to_spec_id.at(insn.Word(2)); + if (spec_info && spec_id < spec_info->mapEntryCount) { + memcpy(value, (uint8_t *)spec_info->pData + spec_info->pMapEntries[spec_id].offset, + spec_info->pMapEntries[spec_id].size); + } + return true; + } + + // This means the value is not known until runtime and will need to be checked in GPU-AV + return false; +} + PipelineStageState::PipelineStageState(const safe_VkPipelineShaderStageCreateInfo *pipeline_create_info, const safe_VkShaderCreateInfoEXT *shader_object_create_info, std::shared_ptr module_state, diff --git a/layers/utils/shader_utils.h b/layers/utils/shader_utils.h index 264c228f8ff..2317115d646 100644 --- a/layers/utils/shader_utils.h +++ b/layers/utils/shader_utils.h @@ -99,6 +99,7 @@ struct SPIRV_MODULE_STATE; struct safe_VkPipelineShaderStageCreateInfo; struct safe_VkShaderCreateInfoEXT; struct safe_VkSpecializationInfo; +class Instruction; struct PipelineStageState { // We use this over a SPIRV_MODULE_STATE because there are times we need to create empty objects @@ -118,6 +119,7 @@ struct PipelineStageState { VkShaderStageFlagBits GetStage() const; safe_VkSpecializationInfo *GetSpecializationInfo() const; const void *GetPNext() const; + bool GetInt32ConstantValue(const Instruction &insn, uint32_t *value) const; }; using StageStateVec = std::vector; diff --git a/tests/unit/shader_spirv.cpp b/tests/unit/shader_spirv.cpp index 65f13b8a3f2..949c7c28881 100644 --- a/tests/unit/shader_spirv.cpp +++ b/tests/unit/shader_spirv.cpp @@ -2461,4 +2461,90 @@ TEST_F(NegativeShaderSpirv, DISABLED_ImageFormatTypeMismatchWithZeroExtend) { pipe.InitState(); pipe.CreateComputePipeline(); m_errorMonitor->VerifyFound(); -} \ No newline at end of file +} + +// TODO - https://github.com/KhronosGroup/SPIRV-Tools/issues/5468 +TEST_F(NegativeShaderSpirv, DISABLED_SpecConstantTextureIndex) { + TEST_DESCRIPTION("Apply spec constant to lower array size and detect array being oob"); + RETURN_IF_SKIP(Init()); + InitRenderTarget(); + + const char *fragment_source = R"glsl( + #version 400 + #extension GL_ARB_separate_shader_objects : enable + #extension GL_ARB_shading_language_420pack : enable + + layout (location = 0) out vec4 out_color; + + layout (constant_id = 0) const int num_textures = 3; + layout (binding = 0) uniform sampler2D textures[num_textures]; + + void main() { + out_color = texture(textures[2], vec2(0.0)); + } + )glsl"; + + uint32_t data = 2; // will make textures[2] OOB + VkSpecializationMapEntry entry = {0, 0, sizeof(uint32_t)}; + VkSpecializationInfo specialization_info = {1, &entry, sizeof(uint32_t), &data}; + const VkShaderObj fs(this, fragment_source, VK_SHADER_STAGE_FRAGMENT_BIT, SPV_ENV_VULKAN_1_0, SPV_SOURCE_GLSL, + &specialization_info); + + m_errorMonitor->SetDesiredFailureMsg(kErrorBit, "VUID-VkPipelineShaderStageCreateInfo-pSpecializationInfo-06849"); + CreatePipelineHelper pipe(*this); + pipe.dsl_bindings_ = {{0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 2, VK_SHADER_STAGE_ALL_GRAPHICS, nullptr}}; + pipe.InitState(); + pipe.shader_stages_ = {pipe.vs_->GetStageCreateInfo(), fs.GetStageCreateInfo()}; + pipe.CreateGraphicsPipeline(); + m_errorMonitor->VerifyFound(); +} + +TEST_F(NegativeShaderSpirv, DescriptorCountConstant) { + RETURN_IF_SKIP(Init()) + InitRenderTarget(); + + char const *fsSource = R"glsl( + #version 450 + layout (set = 0, binding = 0) uniform sampler2D tex[3]; + layout (location = 0) out vec4 out_color; + void main() { + out_color = textureLodOffset(tex[1], vec2(0), 0, ivec2(0)); + } + )glsl"; + + const VkShaderObj fs(this, fsSource, VK_SHADER_STAGE_FRAGMENT_BIT); + + const auto set_info = [&](CreatePipelineHelper &helper) { + helper.shader_stages_ = {helper.vs_->GetStageCreateInfo(), fs.GetStageCreateInfo()}; + helper.dsl_bindings_ = {{0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 2, VK_SHADER_STAGE_FRAGMENT_BIT, nullptr}}; + }; + CreatePipelineHelper::OneshotTest(*this, set_info, kErrorBit, "VUID-VkGraphicsPipelineCreateInfo-layout-07991"); +} + +// This is not working because of a bug in the Spec Constant logic +// https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/5911 +TEST_F(NegativeShaderSpirv, DISABLED_DescriptorCountSpecConstant) { + RETURN_IF_SKIP(Init()) + InitRenderTarget(); + + char const *fsSource = R"glsl( + #version 450 + layout (constant_id = 0) const int index = 2; + layout (set = 0, binding = 0) uniform sampler2D tex[index]; + layout (location = 0) out vec4 out_color; + void main() { + out_color = textureLodOffset(tex[1], vec2(0), 0, ivec2(0)); + } + )glsl"; + + uint32_t data = 4; // over VkDescriptorSetLayoutBinding::descriptorCount + VkSpecializationMapEntry entry = {0, 0, sizeof(uint32_t)}; + VkSpecializationInfo specialization_info = {1, &entry, sizeof(uint32_t), &data}; + const VkShaderObj fs(this, fsSource, VK_SHADER_STAGE_FRAGMENT_BIT, SPV_ENV_VULKAN_1_0, SPV_SOURCE_GLSL, &specialization_info); + + const auto set_info = [&](CreatePipelineHelper &helper) { + helper.shader_stages_ = {helper.vs_->GetStageCreateInfo(), fs.GetStageCreateInfo()}; + helper.dsl_bindings_ = {{0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 2, VK_SHADER_STAGE_FRAGMENT_BIT, nullptr}}; + }; + CreatePipelineHelper::OneshotTest(*this, set_info, kErrorBit, "VUID-VkGraphicsPipelineCreateInfo-layout-07991"); +} diff --git a/tests/unit/shader_spirv_positive.cpp b/tests/unit/shader_spirv_positive.cpp index 0d2a0bdf434..34bd2e0c513 100644 --- a/tests/unit/shader_spirv_positive.cpp +++ b/tests/unit/shader_spirv_positive.cpp @@ -1829,3 +1829,111 @@ void main() { )glsl"; const VkShaderObj vs(this, source, VK_SHADER_STAGE_VERTEX_BIT); } + +TEST_F(PositiveShaderSpirv, SpecConstantTextureIndexDefault) { + TEST_DESCRIPTION("https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/6293"); + RETURN_IF_SKIP(Init()); + InitRenderTarget(); + + const char *fragment_source = R"glsl( + #version 450 + layout (location = 0) out vec4 out_color; + + layout (constant_id = 0) const int num_textures = 2; + layout (binding = 0) uniform sampler2D textures[num_textures]; + + void main() { + out_color = texture(textures[0], vec2(0.0)); + } + )glsl"; + + const VkShaderObj fs(this, fragment_source, VK_SHADER_STAGE_FRAGMENT_BIT); + + CreatePipelineHelper pipe(*this); + pipe.dsl_bindings_ = {{0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 2, VK_SHADER_STAGE_ALL_GRAPHICS, nullptr}}; + pipe.InitState(); + pipe.shader_stages_ = {pipe.vs_->GetStageCreateInfo(), fs.GetStageCreateInfo()}; + pipe.CreateGraphicsPipeline(); +} + +TEST_F(PositiveShaderSpirv, SpecConstantTextureIndexValue) { + RETURN_IF_SKIP(Init()); + InitRenderTarget(); + + const char *fragment_source = R"( + OpCapability Shader + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %main "main" %out_color + OpExecutionMode %main OriginUpperLeft + OpDecorate %out_color Location 0 + OpDecorate %num_textures SpecId 0 + OpDecorate %textures DescriptorSet 0 + OpDecorate %textures Binding 0 + %void = OpTypeVoid + %3 = OpTypeFunction %void + %float = OpTypeFloat 32 + %v4float = OpTypeVector %float 4 +%_ptr_Output_v4float = OpTypePointer Output %v4float + %out_color = OpVariable %_ptr_Output_v4float Output + %10 = OpTypeImage %float 2D 0 0 0 1 Unknown + %11 = OpTypeSampledImage %10 + %int = OpTypeInt 32 1 + ;; index is invalid +%num_textures = OpSpecConstant %int 2 +%_arr_11_num_textures = OpTypeArray %11 %num_textures +%_ptr_UniformConstant__arr_11_num_textures = OpTypePointer UniformConstant %_arr_11_num_textures + %textures = OpVariable %_ptr_UniformConstant__arr_11_num_textures UniformConstant + %int_2 = OpConstant %int 2 +%_ptr_UniformConstant_11 = OpTypePointer UniformConstant %11 + %v2float = OpTypeVector %float 2 + %float_0 = OpConstant %float 0 + %23 = OpConstantComposite %v2float %float_0 %float_0 + %main = OpFunction %void None %3 + %5 = OpLabel + %19 = OpAccessChain %_ptr_UniformConstant_11 %textures %int_2 + %20 = OpLoad %11 %19 + %24 = OpImageSampleImplicitLod %v4float %20 %23 + OpStore %out_color %24 + OpReturn + OpFunctionEnd + )"; + + uint32_t data = 3; + VkSpecializationMapEntry entry = {0, 0, sizeof(uint32_t)}; + VkSpecializationInfo specialization_info = {1, &entry, sizeof(uint32_t), &data}; + const VkShaderObj fs(this, fragment_source, VK_SHADER_STAGE_FRAGMENT_BIT, SPV_ENV_VULKAN_1_0, SPV_SOURCE_ASM, + &specialization_info); + + CreatePipelineHelper pipe(*this); + pipe.dsl_bindings_ = {{0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 2, VK_SHADER_STAGE_ALL_GRAPHICS, nullptr}}; + pipe.InitState(); + pipe.shader_stages_ = {pipe.vs_->GetStageCreateInfo(), fs.GetStageCreateInfo()}; + pipe.CreateGraphicsPipeline(); +} + +TEST_F(PositiveShaderSpirv, DescriptorCountSpecConstant) { + RETURN_IF_SKIP(Init()) + InitRenderTarget(); + + char const *fsSource = R"glsl( + #version 450 + // over VkDescriptorSetLayoutBinding::descriptorCount + layout (constant_id = 0) const int index = 4; + layout (set = 0, binding = 0) uniform sampler2D tex[index]; + layout (location = 0) out vec4 out_color; + void main() { + out_color = textureLodOffset(tex[1], vec2(0), 0, ivec2(0)); + } + )glsl"; + + uint32_t data = 2; + VkSpecializationMapEntry entry = {0, 0, sizeof(uint32_t)}; + VkSpecializationInfo specialization_info = {1, &entry, sizeof(uint32_t), &data}; + const VkShaderObj fs(this, fsSource, VK_SHADER_STAGE_FRAGMENT_BIT, SPV_ENV_VULKAN_1_0, SPV_SOURCE_GLSL, &specialization_info); + + const auto set_info = [&](CreatePipelineHelper &helper) { + helper.shader_stages_ = {helper.vs_->GetStageCreateInfo(), fs.GetStageCreateInfo()}; + helper.dsl_bindings_ = {{0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 2, VK_SHADER_STAGE_FRAGMENT_BIT, nullptr}}; + }; + CreatePipelineHelper::OneshotTest(*this, set_info, kErrorBit); +} diff --git a/tests/unit/subpass.cpp b/tests/unit/subpass.cpp index f9574192cc1..f5780254a84 100644 --- a/tests/unit/subpass.cpp +++ b/tests/unit/subpass.cpp @@ -1086,6 +1086,34 @@ TEST_F(NegativeSubpass, InputAttachmentMissingArray) { CreatePipelineHelper::OneshotTest(*this, set_info, kErrorBit, "VUID-VkGraphicsPipelineCreateInfo-renderPass-06038"); } +// This is not working because of a bug in the Spec Constant logic +// https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/5911 +TEST_F(NegativeSubpass, DISABLED_InputAttachmentMissingSpecConstant) { + RETURN_IF_SKIP(Init()) + InitRenderTarget(); + + char const *fsSource = R"glsl( + #version 450 + layout (constant_id = 0) const int index = 2; + layout(input_attachment_index=0, set=0, binding=0) uniform subpassInput xs[index]; + layout(location=0) out vec4 color; + void main() { + color = subpassLoad(xs[0]); + } + )glsl"; + + uint32_t data = 4; // over VkDescriptorSetLayoutBinding::descriptorCount + VkSpecializationMapEntry entry = {0, 0, sizeof(uint32_t)}; + VkSpecializationInfo specialization_info = {1, &entry, sizeof(uint32_t), &data}; + const VkShaderObj fs(this, fsSource, VK_SHADER_STAGE_FRAGMENT_BIT, SPV_ENV_VULKAN_1_0, SPV_SOURCE_GLSL, &specialization_info); + + const auto set_info = [&](CreatePipelineHelper &helper) { + helper.shader_stages_ = {helper.vs_->GetStageCreateInfo(), fs.GetStageCreateInfo()}; + helper.dsl_bindings_ = {{0, VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT, 2, VK_SHADER_STAGE_FRAGMENT_BIT, nullptr}}; + }; + CreatePipelineHelper::OneshotTest(*this, set_info, kErrorBit, "VUID-VkGraphicsPipelineCreateInfo-layout-07991"); +} + TEST_F(NegativeSubpass, InputAttachmentSharingVariable) { TEST_DESCRIPTION("Make sure if 2 loads use same variable, both are tracked"); @@ -1127,7 +1155,7 @@ TEST_F(NegativeSubpass, InputAttachmentSharingVariable) { // There are 2 OpLoad/OpAccessChain that point the same OpVariable // Make sure we are not just taking the first load and checking all loads on a variable - const char *fs_source = R"( + const char *fs_source = R"glsl( #version 460 layout(input_attachment_index=0, set=0, binding=0) uniform subpassInput xs[2]; layout(location=0) out vec4 color; @@ -1135,7 +1163,7 @@ TEST_F(NegativeSubpass, InputAttachmentSharingVariable) { color = subpassLoad(xs[1]); // valid color = subpassLoad(xs[0]); // invalid } - )"; + )glsl"; VkShaderObj fs(this, fs_source, VK_SHADER_STAGE_FRAGMENT_BIT, SPV_ENV_VULKAN_1_0, SPV_SOURCE_GLSL); const auto set_info = [&](CreatePipelineHelper &helper) { diff --git a/tests/unit/subpass_positive.cpp b/tests/unit/subpass_positive.cpp index f4b05a6dee8..d3a0f38447c 100644 --- a/tests/unit/subpass_positive.cpp +++ b/tests/unit/subpass_positive.cpp @@ -16,6 +16,7 @@ */ #include "../framework/layer_validation_tests.h" +#include "../framework/pipeline_helper.h" TEST_F(PositiveSubpass, SubpassImageBarrier) { TEST_DESCRIPTION("Subpass with image barrier (self-dependency)"); @@ -214,3 +215,31 @@ TEST_F(PositiveSubpass, SubpassWithEventWait) { m_commandBuffer->end(); } } + +// This is not working because of a bug in the Spec Constant logic +// https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/5911 +TEST_F(PositiveSubpass, DISABLED_InputAttachmentMissingSpecConstant) { + RETURN_IF_SKIP(Init()) + InitRenderTarget(); + + char const *fsSource = R"glsl( + #version 450 + layout (constant_id = 0) const int index = 4; // over VkDescriptorSetLayoutBinding::descriptorCount + layout(input_attachment_index=0, set=0, binding=0) uniform subpassInput xs[index]; + layout(location=0) out vec4 color; + void main() { + color = subpassLoad(xs[0]); + } + )glsl"; + + uint32_t data = 2; + VkSpecializationMapEntry entry = {0, 0, sizeof(uint32_t)}; + VkSpecializationInfo specialization_info = {1, &entry, sizeof(uint32_t), &data}; + const VkShaderObj fs(this, fsSource, VK_SHADER_STAGE_FRAGMENT_BIT, SPV_ENV_VULKAN_1_0, SPV_SOURCE_GLSL, &specialization_info); + + const auto set_info = [&](CreatePipelineHelper &helper) { + helper.shader_stages_ = {helper.vs_->GetStageCreateInfo(), fs.GetStageCreateInfo()}; + helper.dsl_bindings_ = {{0, VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT, 2, VK_SHADER_STAGE_FRAGMENT_BIT, nullptr}}; + }; + CreatePipelineHelper::OneshotTest(*this, set_info, kErrorBit); +}