Skip to content
This repository was archived by the owner on Jul 19, 2018. It is now read-only.

WIP: layers: Use CB invalidation mechanism for VU135. #1789

Open
chrisforbes wants to merge 1 commit into
base: master
Choose a base branch
from
Open

WIP: layers: Use CB invalidation mechanism for VU135. #1789

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 14 additions & 15 deletions layers/core_validation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1736,7 +1736,6 @@ static void resetCB(layer_data *dev_data, const VkCommandBuffer cb) {
pCB->drawData.clear();
pCB->currentDrawData.buffers.clear();
pCB->vertex_buffer_used = false;
pCB->primaryCommandBuffer = VK_NULL_HANDLE;
// If secondary, invalidate any primary command buffer that may call us.
if (pCB->createInfo.level == VK_COMMAND_BUFFER_LEVEL_SECONDARY) {
invalidateCommandBuffers(dev_data,
Expand Down Expand Up @@ -2509,6 +2508,8 @@ static bool validatePrimaryCommandBufferState(layer_data *dev_data, GLOBAL_CB_NO
// Track in-use for resources off of primary and any secondary CBs
bool skip = false;

// TODO: inline this function into sole caller

// If USAGE_SIMULTANEOUS_USE_BIT not set then CB cannot already be executing
// on device
skip |= validateCommandBufferSimultaneousUse(dev_data, pCB, current_submit_count);
Expand All @@ -2517,16 +2518,6 @@ static bool validatePrimaryCommandBufferState(layer_data *dev_data, GLOBAL_CB_NO

for (auto pSubCB : pCB->linkedCommandBuffers) {
skip |= validateResources(dev_data, pSubCB);
// TODO: replace with invalidateCommandBuffers() at recording.
if ((pSubCB->primaryCommandBuffer != pCB->commandBuffer) &&
!(pSubCB->beginInfo.flags & VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT)) {
log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT, 0,
__LINE__, VALIDATION_ERROR_31a00092, "DS",
"Commandbuffer 0x%p was submitted with secondary buffer 0x%p but that buffer has subsequently been bound to "
"primary cmd buffer 0x%p and it does not have VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT set. %s",
pCB->commandBuffer, pSubCB->commandBuffer, pSubCB->primaryCommandBuffer,
validation_error_map[VALIDATION_ERROR_31a00092]);
}
}

skip |= validateCommandBufferState(dev_data, pCB, "vkQueueSubmit()", current_submit_count, VALIDATION_ERROR_31a00090);
Expand Down Expand Up @@ -4109,7 +4100,7 @@ VKAPI_ATTR VkResult VKAPI_CALL ResetFences(VkDevice device, uint32_t fenceCount,
}

// For given cb_nodes, invalidate them and track object causing invalidation
void invalidateCommandBuffers(const layer_data *dev_data, std::unordered_set<GLOBAL_CB_NODE *> const &cb_nodes, VK_OBJECT obj) {
void invalidateCommandBuffers(const layer_data *dev_data, std::unordered_set<GLOBAL_CB_NODE *> const &cb_nodes, VK_OBJECT obj, bool track_invalidator) {
for (auto cb_node : cb_nodes) {
if (cb_node->state == CB_RECORDING) {
log_msg(dev_data->report_data, VK_DEBUG_REPORT_WARNING_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT,
Expand All @@ -4120,11 +4111,13 @@ void invalidateCommandBuffers(const layer_data *dev_data, std::unordered_set<GLO
else {
cb_node->state = CB_INVALID_COMPLETE;
}
cb_node->broken_bindings.push_back(obj);
if (track_invalidator) {
cb_node->broken_bindings.push_back(obj);
}

// if secondary, then propagate the invalidation to the primaries that will call us.
if (cb_node->createInfo.level == VK_COMMAND_BUFFER_LEVEL_SECONDARY) {
invalidateCommandBuffers(dev_data, cb_node->linkedCommandBuffers, obj);
invalidateCommandBuffers(dev_data, cb_node->linkedCommandBuffers, obj, track_invalidator);
}
}
}
Expand Down Expand Up @@ -8320,6 +8313,13 @@ VKAPI_ATTR void VKAPI_CALL CmdExecuteCommands(VkCommandBuffer commandBuffer, uin
pCommandBuffers[i], pCB->commandBuffer);
pCB->beginInfo.flags &= ~VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT;
}
if (pSubCB->linkedCommandBuffers.size()) {
// TODO: adjust message support so we can report an accurate error message for this.
Copy link
Contributor

@tobine tobine May 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, change looks good and fulfills the old TODO but currently killing VAL_ERR_00135 which is in Command Buffer Submission portion of spec. Need to preserve that check. It's similar to 133, which is handled in validateCommandBufferSimultaneousUse() just before the killed 135 check. Not immediately obvious to me the best way to preserve 135.

Copy link
Collaborator Author

@chrisforbes chrisforbes May 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From private conversation with @tobine: may be better to remove this VU from the spec as it's redundant with the 'causes the command buffer to become invalid' language.

invalidateCommandBuffers(dev_data, pSubCB->linkedCommandBuffers,
{HandleToUint64(pCommandBuffers[i]), kVulkanObjectTypeCommandBuffer},
false /* don't track invalidator. we've already complained. */);
// TODO: consistent strategy for removing links.
}
}
if (!pCB->activeQueries.empty() && !dev_data->enabled_features.inheritedQueries) {
skip |=
Expand All @@ -8336,7 +8336,6 @@ VKAPI_ATTR void VKAPI_CALL CmdExecuteCommands(VkCommandBuffer commandBuffer, uin
for (auto ilm_entry : pSubCB->imageLayoutMap) {
SetLayout(dev_data, pCB, ilm_entry.first, ilm_entry.second);
}
pSubCB->primaryCommandBuffer = pCB->commandBuffer;
pCB->linkedCommandBuffers.insert(pSubCB);
pSubCB->linkedCommandBuffers.insert(pCB);
for (auto &function : pSubCB->queryUpdates) {
Expand Down
4 changes: 1 addition & 3 deletions layers/core_validation_types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,6 @@ struct GLOBAL_CB_NODE : public BASE_NODE {
std::vector<DRAW_DATA> drawData;
DRAW_DATA currentDrawData;
bool vertex_buffer_used; // Track for perf warning to make sure any bound vtx buffer used
VkCommandBuffer primaryCommandBuffer;
// Track images and buffers that are updated by this CB at the point of a draw
std::unordered_set<VkImageView> updateImages;
std::unordered_set<VkBuffer> updateBuffers;
Expand Down Expand Up @@ -787,7 +786,6 @@ const PHYS_DEV_PROPERTIES_NODE *GetPhysDevProperties(const layer_data *device_da
const VkPhysicalDeviceFeatures *GetEnabledFeatures(const layer_data *device_data);
const DeviceExtensions *GetEnabledExtensions(const layer_data *device_data);

void invalidateCommandBuffers(const layer_data *, std::unordered_set<GLOBAL_CB_NODE *> const &, VK_OBJECT);
bool ValidateMemoryIsBoundToBuffer(const layer_data *, const BUFFER_STATE *, const char *, UNIQUE_VALIDATION_ERROR_CODE);
bool ValidateMemoryIsBoundToImage(const layer_data *, const IMAGE_STATE *, const char *, UNIQUE_VALIDATION_ERROR_CODE);
void AddCommandBufferBindingSampler(GLOBAL_CB_NODE *, SAMPLER_STATE *);
Expand All @@ -796,7 +794,7 @@ void AddCommandBufferBindingImageView(const layer_data *, GLOBAL_CB_NODE *, IMAG
void AddCommandBufferBindingBuffer(const layer_data *, GLOBAL_CB_NODE *, BUFFER_STATE *);
void AddCommandBufferBindingBufferView(const layer_data *, GLOBAL_CB_NODE *, BUFFER_VIEW_STATE *);
bool ValidateObjectNotInUse(const layer_data *dev_data, BASE_NODE *obj_node, VK_OBJECT obj_struct, UNIQUE_VALIDATION_ERROR_CODE error_code);
void invalidateCommandBuffers(const layer_data *dev_data, std::unordered_set<GLOBAL_CB_NODE *> const &cb_nodes, VK_OBJECT obj);
void invalidateCommandBuffers(const layer_data *dev_data, std::unordered_set<GLOBAL_CB_NODE *> const &cb_nodes, VK_OBJECT obj, bool track_invalidator=true);
void RemoveImageMemoryRange(uint64_t handle, DEVICE_MEM_INFO *mem_info);
void RemoveBufferMemoryRange(uint64_t handle, DEVICE_MEM_INFO *mem_info);
bool ClearMemoryObjectBindings(layer_data *dev_data, uint64_t handle, VulkanObjectType type);
Expand Down