layers: Add descriptor sets to CB_INVALID tracking

This is start of a series intended to enable CB_INVALID tracking for
descriptor sets. Much of the tracking is already in place in the
special-purpose uniqueBoundSets data struct, so I'll be migrating
the tracking from that struct to the more general-purpose
object_bindings set.

diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp
index e941f18..16b97b6 100644 (file)
--- a/layers/core_validation.cpp
+++ b/layers/core_validation.cpp
@@ -75,16 +75,6 @@
 
 using namespace std;
 
-// TODO : CB really needs it's own class and files so this is just temp code until that happens
-GLOBAL_CB_NODE::~GLOBAL_CB_NODE() {
-    for (uint32_t i=0; i<VK_PIPELINE_BIND_POINT_RANGE_SIZE; ++i) {
-        // Make sure that no sets hold onto deleted CB binding
-        for (auto set : lastBound[i].uniqueBoundSets) {
-            set->RemoveBoundCommandBuffer(this);
-        }
-    }
-}
-
 namespace core_validation {
 
 using std::unordered_map;
@@ -3941,6 +3931,12 @@ static void removeCommandBufferBinding(layer_data *dev_data, VK_OBJECT const *ob
             pipe_node->cb_bindings.erase(cb_node);
         break;
     }
+    case VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT: {
+        auto set_node = getSetNode(dev_data, reinterpret_cast<const VkDescriptorSet &>(object->handle));
+        if (set_node)
+            set_node->RemoveBoundCommandBuffer(cb_node);
+        break;
+    }
     default:
         assert(0); // unhandled object type
     }
@@ -3965,10 +3961,6 @@ static void resetCB(layer_data *dev_data, const VkCommandBuffer cb) {
         pCB->scissorMask = 0;
 
         for (uint32_t i = 0; i < VK_PIPELINE_BIND_POINT_RANGE_SIZE; ++i) {
-            // Before clearing lastBoundState, remove any CB bindings from all uniqueBoundSets
-            for (auto set : pCB->lastBound[i].uniqueBoundSets) {
-                set->RemoveBoundCommandBuffer(pCB);
-            }
             pCB->lastBound[i].reset();
         }
 
@@ -4005,6 +3997,7 @@ static void resetCB(layer_data *dev_data, const VkCommandBuffer cb) {
         for (auto obj : pCB->object_bindings) {
             removeCommandBufferBinding(dev_data, &obj, pCB);
         }
+        pCB->object_bindings.clear();
         // Remove this cmdBuffer's reference from each FrameBuffer's CB ref list
         for (auto framebuffer : pCB->framebuffers) {
             auto fb_node = getFramebuffer(dev_data, framebuffer);
@@ -4461,6 +4454,32 @@ static bool ValidateCmdBufImageLayouts(layer_data *dev_data, GLOBAL_CB_NODE *pCB
     return skip_call;
 }
 
+// Loop through bound objects and increment their in_use counts
+//  For any unknown objects, flag an error
+static bool ValidateAndIncrementBoundObjects(layer_data const *dev_data, GLOBAL_CB_NODE const *cb_node) {
+    bool skip_call = false;
+    for (auto obj : cb_node->object_bindings) {
+        switch (obj.type) {
+        case VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT: {
+            auto set_node = getSetNode(dev_data, reinterpret_cast<VkDescriptorSet &>(obj.handle));
+            if (!set_node) {
+                skip_call |=
+                    log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT,
+                            obj.handle, __LINE__, DRAWSTATE_INVALID_DESCRIPTOR_SET, "DS",
+                            "Cannot submit cmd buffer using deleted descriptor set 0x%" PRIx64 ".", obj.handle);
+            } else {
+                set_node->in_use.fetch_add(1);
+            }
+            break;
+        }
+        default:
+            // TODO : Merge handling of other objects types into this code
+            break;
+        }
+    }
+    return skip_call;
+}
+
 // Track which resources are in-flight by atomically incrementing their "in_use" count
 static bool validateAndIncrementResources(layer_data *my_data, GLOBAL_CB_NODE *pCB) {
     bool skip_call = false;
@@ -4468,6 +4487,11 @@ static bool validateAndIncrementResources(layer_data *my_data, GLOBAL_CB_NODE *p
     pCB->in_use.fetch_add(1);
     my_data->globalInFlightCmdBuffers.insert(pCB->commandBuffer);
 
+    // First Increment for all "generic" objects bound to cmd buffer, followed by special-case objects below
+    skip_call |= ValidateAndIncrementBoundObjects(my_data, pCB);
+    // TODO : We should be able to remove the NULL look-up checks from the code below as long as
+    //  all the corresponding cases are verified to cause CB_INVALID state and the CB_INVALID state
+    //  should then be flagged prior to calling this function
     for (auto drawDataElement : pCB->drawData) {
         for (auto buffer : drawDataElement.buffers) {
             auto buffer_node = getBufferNode(my_data, buffer);
@@ -4480,18 +4504,6 @@ static bool validateAndIncrementResources(layer_data *my_data, GLOBAL_CB_NODE *p
             }
         }
     }
-    for (uint32_t i = 0; i < VK_PIPELINE_BIND_POINT_RANGE_SIZE; ++i) {
-        for (auto set : pCB->lastBound[i].uniqueBoundSets) {
-            if (!my_data->setMap.count(set->GetSet())) {
-                skip_call |=
-                    log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT,
-                            (uint64_t)(set), __LINE__, DRAWSTATE_INVALID_DESCRIPTOR_SET, "DS",
-                            "Cannot submit cmd buffer using deleted descriptor set 0x%" PRIx64 ".", (uint64_t)(set));
-            } else {
-                set->in_use.fetch_add(1);
-            }
-        }
-    }
     for (auto event : pCB->events) {
         auto event_node = getEventNode(my_data, event);
         if (!event_node) {
@@ -4544,6 +4556,21 @@ static inline void removeInFlightCmdBuffer(layer_data *dev_data, VkCommandBuffer
     }
 }
 
+// Decrement in-use count for objects bound to command buffer
+static void DecrementBoundResources(layer_data const *dev_data, GLOBAL_CB_NODE const *cb_node) {
+    for (auto obj : cb_node->object_bindings) {
+        switch (obj.type) {
+        case VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT: {
+            auto set_node = getSetNode(dev_data, reinterpret_cast<VkDescriptorSet &>(obj.handle));
+            set_node->in_use.fetch_sub(1);
+            break;
+        }
+        default:
+            // TODO : Merge handling of other objects types into this code
+            break;
+        }
+    }
+}
 
 static bool RetireWorkOnQueue(layer_data *dev_data, QUEUE_NODE *pQueue, uint64_t seq)
 {
@@ -4568,6 +4595,8 @@ static bool RetireWorkOnQueue(layer_data *dev_data, QUEUE_NODE *pQueue, uint64_t
 
         for (auto cb : submission.cbs) {
             auto pCB = getCBNode(dev_data, cb);
+            // First perform decrement on general case bound objects
+            DecrementBoundResources(dev_data, pCB);
             for (auto drawDataElement : pCB->drawData) {
                 for (auto buffer : drawDataElement.buffers) {
                     auto buffer_node = getBufferNode(dev_data, buffer);
@@ -4576,11 +4605,6 @@ static bool RetireWorkOnQueue(layer_data *dev_data, QUEUE_NODE *pQueue, uint64_t
                     }
                 }
             }
-            for (uint32_t i = 0; i < VK_PIPELINE_BIND_POINT_RANGE_SIZE; ++i) {
-                for (auto set : pCB->lastBound[i].uniqueBoundSets) {
-                    set->in_use.fetch_sub(1);
-                }
-            }
             for (auto event : pCB->events) {
                 auto eventNode = dev_data->eventMap.find(event);
                 if (eventNode != dev_data->eventMap.end()) {
@@ -6974,7 +6998,6 @@ CmdBindDescriptorSets(VkCommandBuffer commandBuffer, VkPipelineBindPoint pipelin
             for (uint32_t i = 0; i < setCount; i++) {
                 cvdescriptorset::DescriptorSet *pSet = getSetNode(dev_data, pDescriptorSets[i]);
                 if (pSet) {
-                    pCB->lastBound[pipelineBindPoint].uniqueBoundSets.insert(pSet);
                     pSet->BindCommandBuffer(pCB);
                     pCB->lastBound[pipelineBindPoint].pipeline_layout = *pipeline_layout;
                     pCB->lastBound[pipelineBindPoint].boundDescriptorSets[i + firstSet] = pSet;

diff --git a/layers/core_validation_types.h b/layers/core_validation_types.h
index 8f54cc3..ce78621 100644 (file)
--- a/layers/core_validation_types.h
+++ b/layers/core_validation_types.h
@@ -483,8 +483,6 @@ struct LAST_BOUND_STATE {
     VkPipeline pipeline;
     PIPELINE_LAYOUT_NODE pipeline_layout;
     // Track each set that has been bound
-    // TODO : can unique be global per CB? (do we care about Gfx vs. Compute?)
-    std::unordered_set<cvdescriptorset::DescriptorSet *> uniqueBoundSets;
     // Ordered bound set tracking where index is set# that given set is bound to
     std::vector<cvdescriptorset::DescriptorSet *> boundDescriptorSets;
     // one dynamic offset per dynamic descriptor bound to this CB
@@ -493,7 +491,6 @@ struct LAST_BOUND_STATE {
     void reset() {
         pipeline = VK_NULL_HANDLE;
         pipeline_layout.reset();
-        uniqueBoundSets.clear();
         boundDescriptorSets.clear();
         dynamicOffsets.clear();
     }
@@ -554,8 +551,6 @@ struct GLOBAL_CB_NODE : public BASE_NODE {
     std::unordered_set<VkDeviceMemory> memObjs;
     std::vector<std::function<bool(VkQueue)>> eventUpdates;
     std::vector<std::function<bool(VkQueue)>> queryUpdates;
-
-    ~GLOBAL_CB_NODE();
 };
 
 struct SEMAPHORE_WAIT {

diff --git a/layers/descriptor_sets.cpp b/layers/descriptor_sets.cpp
index 1d568f4..d9eb277 100644 (file)
--- a/layers/descriptor_sets.cpp
+++ b/layers/descriptor_sets.cpp
@@ -320,12 +320,6 @@ cvdescriptorset::DescriptorSet::DescriptorSet(const VkDescriptorSet set, const D
 
 cvdescriptorset::DescriptorSet::~DescriptorSet() {
     InvalidateBoundCmdBuffers();
-    // Remove link to any cmd buffers
-    for (auto cb : cb_bindings) {
-        for (uint32_t i=0; i<VK_PIPELINE_BIND_POINT_RANGE_SIZE; ++i) {
-            cb->lastBound[i].uniqueBoundSets.erase(this);
-        }
-    }
 }
 
 
@@ -630,6 +624,16 @@ void cvdescriptorset::DescriptorSet::PerformCopyUpdate(const VkCopyDescriptorSet
     InvalidateBoundCmdBuffers();
 }
 
+void cvdescriptorset::DescriptorSet::BindCommandBuffer(GLOBAL_CB_NODE *cb_node) {
+    // bind cb to this descriptor set
+    cb_bindings.insert(cb_node);
+    // Add bindings for descriptor set and individual objects in the set
+    cb_node->object_bindings.insert({reinterpret_cast<uint64_t &>(set_), VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT});
+    // TODO : Can bind individual objects from within each descriptor : buffers/images and their views, samplers & memory
+    //  The trick is we should only bind the objects actually "in use" by the cmd buffer, meaning that we need to
+    //  check active descriptor slots based on last bound state for this CB
+}
+
 cvdescriptorset::SamplerDescriptor::SamplerDescriptor() : sampler_(VK_NULL_HANDLE), immutable_(false) {
     updated = false;
     descriptor_class = PlainSampler;

diff --git a/layers/descriptor_sets.h b/layers/descriptor_sets.h
index d8222a6..6656873 100644 (file)
--- a/layers/descriptor_sets.h
+++ b/layers/descriptor_sets.h
@@ -334,7 +334,7 @@ class DescriptorSet : public BASE_NODE {
     // Return unordered_set of all command buffers that this set is bound to
     std::unordered_set<GLOBAL_CB_NODE *> GetBoundCmdBuffers() const { return cb_bindings; }
     // Bind given cmd_buffer to this descriptor set
-    void BindCommandBuffer(GLOBAL_CB_NODE *cb_node) { cb_bindings.insert(cb_node); }
+    void BindCommandBuffer(GLOBAL_CB_NODE *cb_node);
     // If given cmd_buffer is in the cb_bindings set, remove it
     void RemoveBoundCommandBuffer(GLOBAL_CB_NODE *cb_node) { cb_bindings.erase(cb_node); }
     VkSampler const *GetImmutableSamplerPtrFromBinding(const uint32_t index) const {