From bf6dae6c58f68b14e37580c06a35df04f2cf3933 Mon Sep 17 00:00:00 2001 From: Chris Forbes Date: Mon, 7 Mar 2016 16:09:00 +1300 Subject: layers: Add descriptor type matching to DrawState (Squashed from 6 commits of WIP series) Signed-off-by: Chris Forbes --- layers/draw_state.cpp | 125 ++++++++++++++++++++++++++++++++-- layers/draw_state.h | 27 ++++---- layers/vk_validation_layer_details.md | 2 + 3 files changed, 134 insertions(+), 20 deletions(-) diff --git a/layers/draw_state.cpp b/layers/draw_state.cpp index c6d0fcda..ef31ca72 100644 --- a/layers/draw_state.cpp +++ b/layers/draw_state.cpp @@ -491,6 +491,12 @@ static char *describe_type(char *dst, shader_module const *src, unsigned type) { } case spv::OpTypeSampler: return dst + sprintf(dst, "sampler"); + case spv::OpTypeSampledImage: + dst += sprintf(dst, "sampler+"); + return describe_type(dst, src, insn.word(2)); + case spv::OpTypeImage: + dst += sprintf(dst, "image(dim=%u, sampled=%u)", insn.word(3), insn.word(7)); + return dst; default: return dst + sprintf(dst, "oddtype"); } @@ -1251,8 +1257,8 @@ static bool validate_push_constant_block_against_pipeline(layer_data *my_data, V if (log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT, /* dev */ 0, __LINE__, SHADER_CHECKER_PUSH_CONSTANT_NOT_ACCESSIBLE_FROM_STAGE, "SC", "Push constant range covering variable starting at " - "offset %u not accessible from %s stage", - offset, shader_stage_attribs[get_shader_stage_id(stage)].name)) { + "offset %u not accessible from stage %s", + offset, string_VkShaderStageFlagBits(stage))) { pass = false; } } @@ -1296,16 +1302,27 @@ static bool validate_push_constant_usage(layer_data *my_data, VkDevice dev, // For given pipelineLayout verify that the setLayout at slot.first // has the requested binding at slot.second -static bool has_descriptor_binding(layer_data *my_data, vector *pipelineLayout, descriptor_slot_t slot) { +static bool has_descriptor_binding(layer_data *my_data, vector *pipelineLayout, descriptor_slot_t slot, + VkDescriptorType &type, VkShaderStageFlags &stage_flags) { + type = VkDescriptorType(0); + stage_flags = VkShaderStageFlags(0); + if (!pipelineLayout) return false; if (slot.first >= pipelineLayout->size()) return false; - const auto &bindingMap = my_data->descriptorSetLayoutMap[(*pipelineLayout)[slot.first]]->bindingToIndexMap; + auto const layout_node = my_data->descriptorSetLayoutMap[(*pipelineLayout)[slot.first]]; + + auto bindingIt = layout_node->bindingToIndexMap.find(slot.second); + if (bindingIt == layout_node->bindingToIndexMap.end()) + return false; + + type = layout_node->descriptorTypes[slot.second]; + stage_flags = layout_node->stageFlags[slot.second]; - return (bindingMap.find(slot.second) != bindingMap.end()); + return true; } // Block of code at start here for managing/tracking Pipeline state that this layer cares about @@ -1575,6 +1592,75 @@ static VkBool32 validate_specialization_offsets(layer_data *my_data, VkPipelineS return pass; } +static bool descriptor_type_match(layer_data *my_data, shader_module const *module, uint32_t type_id, + VkDescriptorType descriptor_type) { + auto type = module->get_def(type_id); + + /* Strip off any array or ptrs */ + /* TODO: if we see an array type here, we should make use of it in order to + * validate the number of descriptors actually required to be set in the + * API. + */ + while (type.opcode() == spv::OpTypeArray || type.opcode() == spv::OpTypePointer) { + type = module->get_def(type.word(type.opcode() == spv::OpTypeArray ? 2 : 3)); + } + + switch (type.opcode()) { + case spv::OpTypeStruct: { + for (auto insn : *module) { + if (insn.opcode() == spv::OpDecorate && insn.word(1) == type.word(1)) { + if (insn.word(2) == spv::DecorationBlock) { + return descriptor_type == VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER || + descriptor_type == VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC; + } else if (insn.word(2) == spv::DecorationBufferBlock) { + return descriptor_type == VK_DESCRIPTOR_TYPE_STORAGE_BUFFER || + descriptor_type == VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC; + } + } + } + + /* Invalid */ + return false; + } + + case spv::OpTypeSampler: + return descriptor_type == VK_DESCRIPTOR_TYPE_SAMPLER; + + case spv::OpTypeSampledImage: + return descriptor_type == VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER; + + case spv::OpTypeImage: { + /* Many descriptor types backing image types-- depends on dimension + * and whether the image will be used with a sampler. SPIRV for + * Vulkan requires that sampled be 1 or 2 -- leaving the decision to + * runtime is unacceptable. + */ + auto dim = type.word(3); + auto sampled = type.word(7); + + if (dim == spv::DimSubpassData) { + return descriptor_type == VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT; + } else if (dim == spv::DimBuffer) { + if (sampled == 1) { + return descriptor_type == VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER; + } else { + return descriptor_type == VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER; + } + } else if (sampled == 1) { + return descriptor_type == VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE; + } else { + return descriptor_type == VK_DESCRIPTOR_TYPE_STORAGE_IMAGE; + } + } + + /* We shouldn't really see any other junk types -- but if we do, they're + * a mismatch. + */ + default: + return false; /* Mismatch */ + } +} + // Validate that the shaders used by the given pipeline // As a side effect this function also records the sets that are actually used by the pipeline static VkBool32 validate_pipeline_shaders(layer_data *my_data, VkDevice dev, PIPELINE_NODE *pPipeline) { @@ -1614,7 +1700,8 @@ static VkBool32 validate_pipeline_shaders(layer_data *my_data, VkDevice dev, PIP if (entrypoints[stage_id] == module->end()) { if (log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT, /*dev*/ 0, __LINE__, SHADER_CHECKER_MISSING_ENTRYPOINT, "SC", - "No entrypoint found named `%s` for stages %u", pStage->pName, pStage->stage)) { + "No entrypoint found named `%s` for stage %s", pStage->pName, + string_VkShaderStageFlagBits(pStage->stage))) { pass = VK_FALSE; } } @@ -1636,7 +1723,9 @@ static VkBool32 validate_pipeline_shaders(layer_data *my_data, VkDevice dev, PIP pPipeline->active_sets.insert(it->first.first); /* find the matching binding */ - auto found = has_descriptor_binding(my_data, layouts, it->first); + VkDescriptorType descriptor_type; + VkShaderStageFlags descriptor_stage_flags; + auto found = has_descriptor_binding(my_data, layouts, it->first, descriptor_type, descriptor_stage_flags); if (!found) { char type_name[1024]; @@ -1647,6 +1736,28 @@ static VkBool32 validate_pipeline_shaders(layer_data *my_data, VkDevice dev, PIP it->first.first, it->first.second, type_name)) { pass = VK_FALSE; } + } else if (~descriptor_stage_flags & pStage->stage) { + char type_name[1024]; + describe_type(type_name, module, it->second.type_id); + if (log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT, + /*dev*/ 0, __LINE__, SHADER_CHECKER_DESCRIPTOR_NOT_ACCESSIBLE_FROM_STAGE, "SC", + "Shader uses descriptor slot %u.%u (used " + "as type `%s`) but descriptor not " + "accessible from stage %s", + it->first.first, it->first.second, type_name, string_VkShaderStageFlagBits(pStage->stage))) { + pass = VK_FALSE; + } + } else if (!descriptor_type_match(my_data, module, it->second.type_id, descriptor_type)) { + char type_name[1024]; + describe_type(type_name, module, it->second.type_id); + if (log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT, + /*dev*/ 0, __LINE__, SHADER_CHECKER_DESCRIPTOR_TYPE_MISMATCH, "SC", + "Type mismatch on descriptor slot " + "%u.%u (used as type `%s`) but " + "descriptor of type %s", + it->first.first, it->first.second, type_name, string_VkDescriptorType(descriptor_type))) { + pass = VK_FALSE; + } } } diff --git a/layers/draw_state.h b/layers/draw_state.h index ec4e9056..aa76a23a 100644 --- a/layers/draw_state.h +++ b/layers/draw_state.h @@ -205,19 +205,20 @@ typedef enum _DRAW_STATE_ERROR { typedef enum _SHADER_CHECKER_ERROR { SHADER_CHECKER_NONE, - SHADER_CHECKER_FS_MIXED_BROADCAST, /* FS writes broadcast output AND custom outputs -- DEFUNCT */ - SHADER_CHECKER_INTERFACE_TYPE_MISMATCH, /* Type mismatch between shader stages or shader and pipeline */ - SHADER_CHECKER_OUTPUT_NOT_CONSUMED, /* Entry appears in output interface, but missing in input */ - SHADER_CHECKER_INPUT_NOT_PRODUCED, /* Entry appears in input interface, but missing in output */ - SHADER_CHECKER_NON_SPIRV_SHADER, /* Shader image is not SPIR-V */ - SHADER_CHECKER_INCONSISTENT_SPIRV, /* General inconsistency within a SPIR-V module */ - SHADER_CHECKER_UNKNOWN_STAGE, /* Stage is not supported by analysis */ - SHADER_CHECKER_INCONSISTENT_VI, /* VI state contains conflicting binding or attrib descriptions */ - SHADER_CHECKER_MISSING_DESCRIPTOR, /* Shader attempts to use a descriptor binding not declared in the layout */ - SHADER_CHECKER_BAD_SPECIALIZATION, /* Specialization map entry points outside specialization data block */ - SHADER_CHECKER_MISSING_ENTRYPOINT, /* Shader module does not contain the requested entrypoint */ - SHADER_CHECKER_PUSH_CONSTANT_OUT_OF_RANGE, /* Push constant variable is not in a push constant range */ - SHADER_CHECKER_PUSH_CONSTANT_NOT_ACCESSIBLE_FROM_STAGE, /* Push constant range exists, but not accessible from stage */ + SHADER_CHECKER_INTERFACE_TYPE_MISMATCH, // Type mismatch between shader stages or shader and pipeline + SHADER_CHECKER_OUTPUT_NOT_CONSUMED, // Entry appears in output interface, but missing in input + SHADER_CHECKER_INPUT_NOT_PRODUCED, // Entry appears in input interface, but missing in output + SHADER_CHECKER_NON_SPIRV_SHADER, // Shader image is not SPIR-V + SHADER_CHECKER_INCONSISTENT_SPIRV, // General inconsistency within a SPIR-V module + SHADER_CHECKER_UNKNOWN_STAGE, // Stage is not supported by analysis + SHADER_CHECKER_INCONSISTENT_VI, // VI state contains conflicting binding or attrib descriptions + SHADER_CHECKER_MISSING_DESCRIPTOR, // Shader attempts to use a descriptor binding not declared in the layout + SHADER_CHECKER_BAD_SPECIALIZATION, // Specialization map entry points outside specialization data block + SHADER_CHECKER_MISSING_ENTRYPOINT, // Shader module does not contain the requested entrypoint + SHADER_CHECKER_PUSH_CONSTANT_OUT_OF_RANGE, // Push constant variable is not in a push constant range + SHADER_CHECKER_PUSH_CONSTANT_NOT_ACCESSIBLE_FROM_STAGE, // Push constant range exists, but not accessible from stage + SHADER_CHECKER_DESCRIPTOR_TYPE_MISMATCH, // Descriptor type does not match shader resource type + SHADER_CHECKER_DESCRIPTOR_NOT_ACCESSIBLE_FROM_STAGE, // Descriptor used by shader, but not accessible from stage } SHADER_CHECKER_ERROR; typedef enum _DRAW_TYPE { diff --git a/layers/vk_validation_layer_details.md b/layers/vk_validation_layer_details.md index 62b467bd..75828ba0 100644 --- a/layers/vk_validation_layer_details.md +++ b/layers/vk_validation_layer_details.md @@ -142,6 +142,8 @@ depends on the pair of pipeline stages involved. | Missing Entrypoint | Flags error if specified entrypoint is not present in the shader module | MISSING_ENTRYPOINT | vkCreateGraphicsPipelines | TBD | NA | | Push constant out of range | Flags error if a member of a push constant block is not contained within a push constant range specified in the pipeline layout | PUSH_CONSTANT_OUT_OF_RANGE | vkCreateGraphicsPipelines | CreatePipelinePushContantsNotInLayout | NA | | Push constant not accessible from stage | Flags error if the push constant range containing a push constant block member is not accessible from the current shader stage. | PUSH_CONSTANT_NOT_ACCESSIBLE_FROM_STAGE | vkCreateGraphicsPipelines | TBD | NA | +| Descriptor not accessible from stage | Flags error if a descriptor used by a shader stage does not include that stage in its stageFlags | DESCRIPTOR_NOT_ACCESSIBLE_FROM_STAGE | vkCreateGraphicsPipelines | TBD | NA | +| Descriptor type mismatch | Flags error if a descriptor type does not match the shader resource type. | DESCRIPTOR_TYPE_MISMATCH | vkCreateGraphicsPipelines | TBD | NA | | NA | Enum used for informational messages | NONE | | NA | None | ### VK_LAYER_LUNARG_ShaderChecker Pending Work -- cgit v1.2.3