layers: Store PushConstant ranges in dictionary
authorJohn Zulauf <jzulauf@lunarg.com>
Fri, 16 Feb 2018 20:07:24 +0000 (13:07 -0700)
committerjzulauf-lunarg <32470354+jzulauf-lunarg@users.noreply.github.com>
Wed, 7 Mar 2018 20:11:29 +0000 (13:11 -0700)
For rapid compatiblity checks, create unique ID's for PushConstant
ranges.

Change-Id: Ie33372e2428e453e610e292b2917801225c11020

layers/core_validation.cpp
layers/core_validation_types.h
layers/hash_vk_types.h
layers/shader_validation.cpp

index 8f51546..b2f11ee 100644 (file)
@@ -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<PushConstantRanges, PushConstantRangesId, std::hash<PushConstantRanges>> 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<PushConstantRanges>();
+        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<const VkPushConstantRange *, PushConstantRangeCompare> 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<PushConstantRanges>(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 <It, bool> pair returned by insert, and taking the value from the
+    // <key, value> 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)) {
index c7f0bea..67392d4 100644 (file)
@@ -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<ImageSubresourcePair> {
 };
 }  // namespace std
 
+// The id defines a unique ID based on the contents of a PushConstantRanges set;
+using PushConstantRangesId = std::shared_ptr<PushConstantRanges>;
+
 // Store layouts and pushconstants for PipelineLayout
 struct PIPELINE_LAYOUT_NODE {
     VkPipelineLayout layout;
     std::vector<std::shared_ptr<cvdescriptorset::DescriptorSetLayout const>> set_layouts;
-    std::vector<VkPushConstantRange> 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();
     }
 };
 
index 871396e..b6d6713 100644 (file)
@@ -24,6 +24,7 @@
 
 #include <vulkan/vulkan.h>
 #include "vk_safe_struct.h"
+#include <vector>
 
 // Hash and equality and/or compare functions for selected Vk types (and useful collections thereof)
 
@@ -60,4 +61,26 @@ struct hash<safe_VkDescriptorSetLayoutBinding> {
 };
 }  // 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<VkPushConstantRange> {
+    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<VkPushConstantRange>;
+
+namespace std {
+template <>
+struct hash<PushConstantRanges> : public hash_util::IsOrderedContainer<PushConstantRanges> {};
+}  // namespace std
+
 #endif  // HASH_VK_TYPES_H_
index 8cbc240..621a475 100644 (file)
@@ -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