From c5bc8e1a5a98212f80673b95b1389c4a95442a17 Mon Sep 17 00:00:00 2001 From: Tobin Ehlis Date: Thu, 21 Jul 2016 14:40:22 -0600 Subject: 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. --- layers/core_validation.cpp | 87 ++++++++++++++++++++++++++---------------- layers/core_validation_types.h | 5 --- layers/descriptor_sets.cpp | 16 +++++--- layers/descriptor_sets.h | 2 +- 4 files changed, 66 insertions(+), 44 deletions(-) diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp index e941f18e..16b97b6d 100644 --- 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; iRemoveBoundCommandBuffer(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(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(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(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 8f54cc3e..ce786210 100644 --- 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 uniqueBoundSets; // Ordered bound set tracking where index is set# that given set is bound to std::vector 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 memObjs; std::vector> eventUpdates; std::vector> queryUpdates; - - ~GLOBAL_CB_NODE(); }; struct SEMAPHORE_WAIT { diff --git a/layers/descriptor_sets.cpp b/layers/descriptor_sets.cpp index 1d568f4d..d9eb2777 100644 --- 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; ilastBound[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(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 d8222a60..6656873a 100644 --- 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 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 { -- cgit v1.2.3