From 2dbdf3828836c15f514143a24aa0aea42bbb789f Mon Sep 17 00:00:00 2001 From: John Zulauf Date: Fri, 16 Feb 2018 13:07:24 -0700 Subject: layers: Store PushConstant ranges in dictionary For rapid compatiblity checks, create unique ID's for PushConstant ranges. Change-Id: Ie33372e2428e453e610e292b2917801225c11020 --- layers/core_validation.cpp | 53 ++++++++++++++++++++++++++++++++++++++---- layers/core_validation_types.h | 8 +++++-- layers/hash_vk_types.h | 23 ++++++++++++++++++ layers/shader_validation.cpp | 2 +- 4 files changed, 78 insertions(+), 8 deletions(-) diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp index 8f515460..b2f11eef 100644 --- a/layers/core_validation.cpp +++ b/layers/core_validation.cpp @@ -5379,6 +5379,52 @@ static bool PreCallValiateCreatePipelineLayout(const layer_data *dev_data, const return skip; } +// Note: std::equal_to is looking for operator == to be defined in the innermost namespace enclosing DescriptorSetLayoutDef +// (i.e. cvdescriptorset) or in the class, base, or containing classes, and does *not* look in :: for it. +static std::unordered_map> push_constant_ranges_dict; + +// For repeatable sorting, not very useful for "memory in range" search +struct PushConstantRangeCompare { + bool operator()(const VkPushConstantRange *lhs, const VkPushConstantRange *rhs) const { + if (lhs->offset == rhs->offset) { + if (lhs->size == rhs->size) { + // The comparison is arbitrary, but avoids false aliasing by comparing all fields. + return lhs->stageFlags < rhs->stageFlags; + } + // If the offsets are the same then sorting by the end of range is useful for validation + return lhs->size < rhs->size; + } + return lhs->offset < rhs->offset; + } +}; +PushConstantRangesId get_definition_id(const VkPipelineLayoutCreateInfo *info) { + if (!info->pPushConstantRanges) { + // Hand back the empty entry (creating as needed)... see below for syntax notes + PushConstantRangesId empty = std::make_shared(); + auto insert_pair = push_constant_ranges_dict.insert({*empty, empty}); + return insert_pair.first->second; + } + + // Sort the input ranges to ensure equivalent ranges map to the same id + std::set sorted; + for (uint32_t i = 0; i < info->pushConstantRangeCount; i++) { + sorted.insert(info->pPushConstantRanges + i); + } + PushConstantRanges ranges(sorted.size()); + for (const auto range : sorted) { + ranges.emplace_back(*range); + } + PushConstantRangesId from_input = std::make_shared(std::move(ranges)); + + // Insert takes care of the "unique" id part by rejecting the insert if a key matching ranges exists, but returning us + // the entry with the extant shared_pointer(id->def) instead. + auto insert_pair = push_constant_ranges_dict.insert({*from_input, from_input}); + + // Return the value by dereferencing the Iterator from the pair returned by insert, and taking the value from the + // pair of the Iterator + return insert_pair.first->second; +} + static void PostCallRecordCreatePipelineLayout(layer_data *dev_data, const VkPipelineLayoutCreateInfo *pCreateInfo, const VkPipelineLayout *pPipelineLayout) { unique_lock_t lock(global_lock); // Lock while accessing state @@ -5389,10 +5435,7 @@ static void PostCallRecordCreatePipelineLayout(layer_data *dev_data, const VkPip for (uint32_t i = 0; i < pCreateInfo->setLayoutCount; ++i) { plNode.set_layouts[i] = GetDescriptorSetLayout(dev_data, pCreateInfo->pSetLayouts[i]); } - plNode.push_constant_ranges.resize(pCreateInfo->pushConstantRangeCount); - for (uint32_t i = 0; i < pCreateInfo->pushConstantRangeCount; ++i) { - plNode.push_constant_ranges[i] = pCreateInfo->pPushConstantRanges[i]; - } + plNode.push_constant_ranges = get_definition_id(pCreateInfo); // Implicit unlock }; @@ -8081,7 +8124,7 @@ VKAPI_ATTR void VKAPI_CALL CmdPushConstants(VkCommandBuffer commandBuffer, VkPip // same offset and size, but different stageFlags. So we can't just check the // stageFlags in the first range with matching offset and size. if (!skip) { - const auto &ranges = getPipelineLayout(dev_data, layout)->push_constant_ranges; + const auto &ranges = *getPipelineLayout(dev_data, layout)->push_constant_ranges; bool found_matching_range = false; for (const auto &range : ranges) { if ((stageFlags == range.stageFlags) && (offset >= range.offset) && (offset + size <= range.offset + range.size)) { diff --git a/layers/core_validation_types.h b/layers/core_validation_types.h index c7f0bead..67392d41 100644 --- a/layers/core_validation_types.h +++ b/layers/core_validation_types.h @@ -23,6 +23,7 @@ #ifndef CORE_VALIDATION_TYPES_H_ #define CORE_VALIDATION_TYPES_H_ +#include "hash_vk_types.h" #include "vk_safe_struct.h" #include "vulkan/vulkan.h" #include "vk_validation_error_messages.h" @@ -542,18 +543,21 @@ struct hash { }; } // namespace std +// The id defines a unique ID based on the contents of a PushConstantRanges set; +using PushConstantRangesId = std::shared_ptr; + // Store layouts and pushconstants for PipelineLayout struct PIPELINE_LAYOUT_NODE { VkPipelineLayout layout; std::vector> set_layouts; - std::vector push_constant_ranges; + PushConstantRangesId push_constant_ranges; PIPELINE_LAYOUT_NODE() : layout(VK_NULL_HANDLE), set_layouts{}, push_constant_ranges{} {} void reset() { layout = VK_NULL_HANDLE; set_layouts.clear(); - push_constant_ranges.clear(); + push_constant_ranges.reset(); } }; diff --git a/layers/hash_vk_types.h b/layers/hash_vk_types.h index 871396e8..b6d67136 100644 --- a/layers/hash_vk_types.h +++ b/layers/hash_vk_types.h @@ -24,6 +24,7 @@ #include #include "vk_safe_struct.h" +#include // Hash and equality and/or compare functions for selected Vk types (and useful collections thereof) @@ -60,4 +61,26 @@ struct hash { }; } // namespace std +// VkPushConstantRange +static bool operator==(const VkPushConstantRange &lhs, const VkPushConstantRange &rhs) { + return (lhs.stageFlags == rhs.stageFlags) && (lhs.offset == rhs.offset) && (lhs.size == rhs.size); +} + +namespace std { +template <> +struct hash { + size_t operator()(const VkPushConstantRange &value) const { + hash_util::HashCombiner hc; + return (hc << value.stageFlags << value.offset << value.size).Value(); + } +}; +} // namespace std + +using PushConstantRanges = std::vector; + +namespace std { +template <> +struct hash : public hash_util::IsOrderedContainer {}; +} // namespace std + #endif // HASH_VK_TYPES_H_ diff --git a/layers/shader_validation.cpp b/layers/shader_validation.cpp index 8cbc2407..621a4759 100644 --- a/layers/shader_validation.cpp +++ b/layers/shader_validation.cpp @@ -1329,7 +1329,7 @@ static bool validate_pipeline_shader_stage(layer_data *dev_data, VkPipelineShade auto descriptor_uses = collect_interface_by_descriptor_slot(report_data, module, accessible_ids); skip |= validate_specialization_offsets(report_data, pStage); - skip |= validate_push_constant_usage(report_data, &pipeline->pipeline_layout.push_constant_ranges, module, accessible_ids, + skip |= validate_push_constant_usage(report_data, pipeline->pipeline_layout.push_constant_ranges.get(), module, accessible_ids, pStage->stage); // Validate descriptor use -- cgit v1.2.3