sync: Add VertexInputCommand
diff --git a/layers/sync/sync_command.cpp b/layers/sync/sync_command.cpp index 441acfd..90b922b 100644 --- a/layers/sync/sync_command.cpp +++ b/layers/sync/sync_command.cpp
@@ -187,6 +187,10 @@ replay_common(command_data.dispatch_indirect_commands[index], access_context, replay_tag); continue; } + case CommandType::kDraw: { + replay_draw(command_data.draw_commands[index], access_context, replay_tag); + continue; + } case CommandType::kDrawIndirect: { replay_draw(command_data.draw_indirect_commands[index], access_context, replay_tag); continue; @@ -214,6 +218,7 @@ begin_render_pass_commands.clear(); shader_access_commands.clear(); dispatch_indirect_commands.clear(); + draw_commands.clear(); draw_indirect_commands.clear(); draw_indirect_count_commands.clear(); draw_mesh_tasks_commands.clear(); @@ -226,12 +231,15 @@ image_views.clear(); render_passes.clear(); pipelines.clear(); + pipeline_lookup.clear(); + last_pipeline = nullptr; buffer_copy_regions.clear(); image_copy_regions.clear(); barrier_sets.clear(); rendering_attachments.clear(); descriptor_buffer_accesses.clear(); descriptor_image_accesses.clear(); + vertex_input_accesses.clear(); descriptor_sets.clear(); descriptor_set_lookup.clear(); @@ -267,7 +275,14 @@ } void CommandData::AddPipeline(const vvl::Pipeline& pipeline) { - pipelines.emplace_back(std::static_pointer_cast<const vvl::Pipeline>(pipeline.shared_from_this())); + if (last_pipeline == &pipeline) { + return; + } + const auto [it, inserted] = pipeline_lookup.emplace(&pipeline); + if (inserted) { + pipelines.emplace_back(std::static_pointer_cast<const vvl::Pipeline>(pipeline.shared_from_this())); + } + last_pipeline = &pipeline; } void CommandData::AddDescriptorSet(const vvl::DescriptorSet& descriptor_set) { @@ -853,6 +868,64 @@ } } +VertexInputCommand VertexInputCommand::Storage::MakeCommand(const CommandData& command_data) const { + vvl::span<const Access> accesses; + if (access_count != 0) { + accesses = vvl::make_span(&command_data.vertex_input_accesses[first_access], access_count); + } + return {pipeline, accesses, access_index}; +} + +VertexInputCommand::Storage VertexInputCommand::MakeStorage(CommandData& command_data) const { + if (pipeline) { + command_data.AddPipeline(*pipeline); + } + for (const Access& access : accesses) { + command_data.AddBuffer(*access.buffer); + } + const uint32_t first_access = uint32_t(command_data.vertex_input_accesses.size()); + vvl::Append(command_data.vertex_input_accesses, accesses); + return {pipeline, first_access, uint32_t(accesses.size()), access_index}; +} + +bool VertexInputCommand::Validate(const CommandBufferContext& cb_context, const Location& loc) const { + return Validate(cb_context.GetSyncEnvironment(), cb_context.GetCurrentAccessContext(), cb_context, kInvalidTag, loc); +} + +bool VertexInputCommand::Validate(const SyncEnvironment& env, const AccessContext& access_context, + const CommandBufferContext& cb_context, ResourceUsageTag replay_tag, const Location& loc) const { + bool skip = false; + for (const Access& access : accesses) { + const HazardResult hazard = access_context.DetectHazard(*access.buffer, access_index, access.range); + if (hazard.IsHazard()) { + const SyncValidator& validator = env.validator; + LogObjectList objlist = BaseObjectList(env, cb_context, access.buffer->Handle()); + if (pipeline) { + objlist.add(pipeline->Handle()); + } + const char* buffer_name = access_index == SYNC_INDEX_INPUT_INDEX_READ ? "index " : "vertex "; + const std::string resource_description = buffer_name + validator.FormatHandle(*access.buffer); + const std::string error = + validator.error_messages_.BufferError(env, hazard, cb_context, replay_tag, loc, resource_description, access.range); + skip |= validator.SyncError(hazard.Hazard(), objlist, loc, error); + } + } + return skip; +} + +void VertexInputCommand::Apply(SyncEnvironment& env, ResourceUsageTag tag, AccessContext& access_context) const { + for (const Access& access : accesses) { + access_context.UpdateAccessState(*access.buffer, access_index, access.range, ResourceUsageTagEx{tag, access.handle_index}, + 0, env.queue_id); + } +} + +void VertexInputAccesses::RegisterResources(CommandBufferContext& cb_context, ResourceUsageTag tag) { + for (auto& access : accesses) { + access.handle_index = cb_context.AddCommandHandle(tag, access.buffer->Handle()).handle_index; + } +} + DispatchIndirectCommand DispatchIndirectCommand::Storage::MakeCommand(const CommandData& command_data) const { return {shader_access_storage.MakeCommand(command_data), indirect_access_storage.MakeCommand(command_data)}; } @@ -917,6 +990,36 @@ } } +DrawCommand DrawCommand::Storage::MakeCommand(const CommandData& command_data, RenderPassAccessContext* render_pass_context, + const RenderingInstance* rendering_instance) const { + return {shader_access_storage.MakeCommand(command_data), vertex_access_storage.MakeCommand(command_data), + attachment_access_storage.MakeCommand(render_pass_context, rendering_instance)}; +} + +DrawCommand::Storage DrawCommand::MakeStorage(CommandData& command_data) const { + return {shader_accesses.MakeStorage(command_data), vertex_accesses.MakeStorage(command_data), + attachment_accesses.MakeStorage(command_data)}; +} + +bool DrawCommand::Validate(const CommandBufferContext& cb_context, const Location& loc) const { + return Validate(cb_context.GetSyncEnvironment(), cb_context.GetCurrentAccessContext(), cb_context, kInvalidTag, loc); +} + +bool DrawCommand::Validate(const SyncEnvironment& env, const AccessContext& access_context, const CommandBufferContext& cb_context, + ResourceUsageTag replay_tag, const Location& loc) const { + bool skip = false; + skip |= shader_accesses.Validate(env, access_context, cb_context, replay_tag, loc); + skip |= vertex_accesses.Validate(env, access_context, cb_context, replay_tag, loc); + skip |= attachment_accesses.Validate(env, access_context, cb_context, replay_tag, loc); + return skip; +} + +void DrawCommand::Apply(SyncEnvironment& env, ResourceUsageTag tag, AccessContext& access_context) const { + shader_accesses.Apply(env, tag, access_context); + vertex_accesses.Apply(env, tag, access_context); + attachment_accesses.Apply(env, tag, access_context); +} + DrawIndirectCommand DrawIndirectCommand::Storage::MakeCommand(const CommandData& command_data, RenderPassAccessContext* render_pass_context, const RenderingInstance* rendering_instance) const {
diff --git a/layers/sync/sync_command.h b/layers/sync/sync_command.h index 76ee075..fe847ab 100644 --- a/layers/sync/sync_command.h +++ b/layers/sync/sync_command.h
@@ -20,6 +20,7 @@ #include "sync_barrier.h" #include "sync_dynamic_rendering.h" #include "containers/custom_containers.h" +#include "containers/small_vector.h" #include "containers/span.h" #include "generated/vk_object_types.h" @@ -63,6 +64,7 @@ kEndRenderPass, kShaderAccess, kDispatchIndirect, + kDraw, kDrawIndirect, kDrawIndirectCount, kDrawMeshTasks, @@ -393,6 +395,60 @@ void Apply(SyncEnvironment& env, ResourceUsageTag tag, AccessContext& access_context) const; }; +struct VertexInputCommand { + struct Access { + const vvl::Buffer* buffer; + AccessRange range; + uint32_t handle_index = vvl::kNoIndex32; + }; + const vvl::Pipeline* pipeline; + vvl::span<const Access> accesses; + SyncAccessIndex access_index; + + struct Storage { + const vvl::Pipeline* pipeline; + uint32_t first_access; + uint32_t access_count; + SyncAccessIndex access_index; + VertexInputCommand MakeCommand(const CommandData& command_data) const; + }; + Storage MakeStorage(CommandData& command_data) const; + bool Validate(const CommandBufferContext& cb_context, const Location& loc) const; + bool Validate(const SyncEnvironment& env, const AccessContext& access_context, const CommandBufferContext& cb_context, + ResourceUsageTag replay_tag, const Location& loc) const; + void Apply(SyncEnvironment& env, ResourceUsageTag tag, AccessContext& access_context) const; +}; + +// Returned by CommandBufferContext::CollectVertexAccesses/CollectIndexAccesses. +// Owns the access array used to construct a VertexInputCommand during recording. +struct VertexInputAccesses { + const vvl::Pipeline* pipeline = nullptr; + small_vector<VertexInputCommand::Access, 2> accesses; + SyncAccessIndex access_index = SYNC_ACCESS_INDEX_NONE; + + void RegisterResources(CommandBufferContext& cb_context, ResourceUsageTag tag); + VertexInputCommand MakeCommand() const { return {pipeline, accesses, access_index}; } +}; + +struct DrawCommand { + ShaderAccessCommand shader_accesses; + VertexInputCommand vertex_accesses; + DrawAttachmentCommand attachment_accesses; + + struct Storage { + ShaderAccessCommand::Storage shader_access_storage; + VertexInputCommand::Storage vertex_access_storage; + DrawAttachmentCommand::Storage attachment_access_storage; + DrawCommand MakeCommand(const CommandData& command_data, RenderPassAccessContext* render_pass_context, + const RenderingInstance* rendering_instance) const; + }; + Storage MakeStorage(CommandData& command_data) const; + bool Validate(const CommandBufferContext& cb_context, const Location& loc) const; + bool Validate(const SyncEnvironment& env, const AccessContext& access_context, const CommandBufferContext& cb_context, + ResourceUsageTag replay_tag, const Location& loc) const; + void Apply(SyncEnvironment& env, ResourceUsageTag tag, AccessContext& access_context) const; +}; + struct DrawIndirectCommand { ShaderAccessCommand shader_accesses; DrawAttachmentCommand attachment_accesses; @@ -469,6 +525,7 @@ std::vector<BeginRenderPassCommand::Storage> begin_render_pass_commands; std::vector<ShaderAccessCommand::Storage> shader_access_commands; std::vector<DispatchIndirectCommand::Storage> dispatch_indirect_commands; + std::vector<DrawCommand::Storage> draw_commands; std::vector<DrawIndirectCommand::Storage> draw_indirect_commands; std::vector<DrawIndirectCountCommand::Storage> draw_indirect_count_commands; std::vector<DrawMeshTasksCommand::Storage> draw_mesh_tasks_commands; @@ -483,14 +540,19 @@ std::vector<std::shared_ptr<const vvl::Image>> images; std::vector<std::shared_ptr<const vvl::ImageView>> image_views; - std::vector<std::shared_ptr<const vvl::RenderPass>> render_passes; + std::vector<std::shared_ptr<const vvl::Pipeline>> pipelines; + vvl::unordered_set<const vvl::Pipeline*> pipeline_lookup; + const vvl::Pipeline* last_pipeline = nullptr; // cache last accessed pipeline + + std::vector<std::shared_ptr<const vvl::RenderPass>> render_passes; std::vector<BufferCopyRegion> buffer_copy_regions; std::vector<VkImageCopy> image_copy_regions; std::vector<BarrierSet> barrier_sets; std::vector<RenderingAttachment> rendering_attachments; std::vector<ShaderAccessCommand::BufferAccess> descriptor_buffer_accesses; std::vector<ShaderAccessCommand::ImageViewAccess> descriptor_image_accesses; + std::vector<VertexInputCommand::Access> vertex_input_accesses; std::vector<std::shared_ptr<const vvl::DescriptorSet>> descriptor_sets; vvl::unordered_set<const vvl::DescriptorSet*> descriptor_set_lookup; @@ -540,6 +602,7 @@ CommandRef Store(const DispatchIndirectCommand::Storage& storage) { return Store(CommandType::kDispatchIndirect, dispatch_indirect_commands, storage); } + CommandRef Store(const DrawCommand::Storage& storage) { return Store(CommandType::kDraw, draw_commands, storage); } CommandRef Store(const DrawIndirectCommand::Storage& storage) { return Store(CommandType::kDrawIndirect, draw_indirect_commands, storage); }
diff --git a/layers/sync/sync_command_buffer.cpp b/layers/sync/sync_command_buffer.cpp index d0af5b9..8bd4e99 100644 --- a/layers/sync/sync_command_buffer.cpp +++ b/layers/sync/sync_command_buffer.cpp
@@ -824,17 +824,19 @@ } } -bool CommandBufferContext::ValidateDrawVertex(uint32_t vertexCount, uint32_t firstVertex, const Location& loc) const { - bool skip = false; - const auto* pipe = cb_state_->GetLastBoundGraphics().pipeline_state; - if (!pipe) { - return skip; +VertexInputAccesses CommandBufferContext::CollectVertexAccesses(uint32_t first_vertex, uint32_t vertex_count) const { + const vvl::Pipeline* pipeline = cb_state_->GetLastBoundGraphics().pipeline_state; + if (!pipeline) { + return {}; } + VertexInputAccesses result; + result.pipeline = pipeline; + result.access_index = SYNC_VERTEX_ATTRIBUTE_INPUT_VERTEX_ATTRIBUTE_READ; const auto& binding_buffers = cb_state_->current_vertex_buffer_binding_info; - const auto& vertex_bindings = pipe->IsDynamic(CB_DYNAMIC_STATE_VERTEX_INPUT_EXT) + const auto& vertex_bindings = pipeline->IsDynamic(CB_DYNAMIC_STATE_VERTEX_INPUT_EXT) ? cb_state_->dynamic_state_value.vertex_bindings - : pipe->vertex_input_state->bindings; + : pipeline->vertex_input_state->bindings; for (const auto& [_, binding_state] : vertex_bindings) { const auto& binding_desc = binding_state.desc; @@ -844,102 +846,59 @@ } if (const vvl::VertexBufferBinding* vertex_buffer = vvl::Find(binding_buffers, binding_desc.binding)) { // TODO - Handle https://gitlab.khronos.org/vulkan/Vulkan-ValidationLayers/-/issues/45 - const auto buf_state = sync_state_.Get<vvl::Buffer>(vertex_buffer->Buffer()); - if (!buf_state) continue; // also skips if using nullDescriptor - - const AccessRange range = - MakeRangeForVertexData(vertex_buffer->BufferOffset(), firstVertex, vertexCount, binding_state); - auto hazard = current_context_->DetectHazard(*buf_state, SYNC_VERTEX_ATTRIBUTE_INPUT_VERTEX_ATTRIBUTE_READ, range); - if (hazard.IsHazard()) { - LogObjectList objlist(cb_state_->Handle(), buf_state->Handle(), pipe->Handle()); - const std::string resource_description = "vertex " + sync_state_.FormatHandle(*buf_state); - const auto error = error_messages_.BufferError(hazard, *this, loc.function, resource_description, range); - skip |= sync_state_.SyncError(hazard.Hazard(), objlist, loc, error); + const auto buffer = sync_state_.Get<vvl::Buffer>(vertex_buffer->Buffer()); + if (!buffer) { + continue; // also skips if using nullDescriptor } + VkDeviceSize offset = vertex_buffer->BufferOffset(); + const AccessRange range = MakeRangeForVertexData(offset, first_vertex, vertex_count, binding_state); + result.accesses.emplace_back(VertexInputCommand::Access{buffer.get(), range}); } } - return skip; + return result; } -void CommandBufferContext::RecordDrawVertex(uint32_t vertexCount, uint32_t firstVertex, const ResourceUsageTag tag) { - const auto* pipe = cb_state_->GetLastBoundGraphics().pipeline_state; - if (!pipe) { - return; - } - const auto& binding_buffers = cb_state_->current_vertex_buffer_binding_info; - const auto& vertex_bindings = pipe->IsDynamic(CB_DYNAMIC_STATE_VERTEX_INPUT_EXT) - ? cb_state_->dynamic_state_value.vertex_bindings - : pipe->vertex_input_state->bindings; - - for (const auto& [_, binding_state] : vertex_bindings) { - const auto& binding_desc = binding_state.desc; - if (binding_desc.inputRate != VK_VERTEX_INPUT_RATE_VERTEX) { - // TODO: add support to determine range of instance level attributes - continue; - } - if (const auto* vertex_buffer = vvl::Find(binding_buffers, binding_desc.binding)) { - // TODO - Handle https://gitlab.khronos.org/vulkan/Vulkan-ValidationLayers/-/issues/45 - const auto buf_state = sync_state_.Get<vvl::Buffer>(vertex_buffer->Buffer()); - if (!buf_state) continue; // also skips if using nullDescriptor - - const AccessRange range = - MakeRangeForVertexData(vertex_buffer->BufferOffset(), firstVertex, vertexCount, binding_state); - const ResourceUsageTagEx tag_ex = AddCommandHandle(tag, buf_state->Handle()); - current_context_->UpdateAccessState(*buf_state, SYNC_VERTEX_ATTRIBUTE_INPUT_VERTEX_ATTRIBUTE_READ, range, tag_ex); - } - } -} - -bool CommandBufferContext::ValidateDrawVertexIndex(uint32_t index_count, uint32_t firstIndex, const Location& loc) const { - bool skip = false; +VertexInputAccesses CommandBufferContext::CollectIndexAccesses(uint32_t first_index, uint32_t index_count) const { + const vvl::Pipeline* pipeline = cb_state_->GetLastBoundGraphics().pipeline_state; const auto& index_binding = cb_state_->index_buffer_binding; // TODO - Handle https://gitlab.khronos.org/vulkan/Vulkan-ValidationLayers/-/issues/45 - const auto index_buf_state = sync_state_.Get<vvl::Buffer>(index_binding.Buffer()); - if (!index_buf_state) return skip; - - const uint32_t index_size = IndexTypeByteSize(index_binding.index_type); - const AccessRange range = MakeRangeForIndexData(index_binding.BufferOffset(), firstIndex, index_count, index_size); - - auto hazard = current_context_->DetectHazard(*index_buf_state, SYNC_INDEX_INPUT_INDEX_READ, range); - if (hazard.IsHazard()) { - LogObjectList objlist(cb_state_->Handle(), index_buf_state->Handle()); - if (const auto* pipe = cb_state_->GetLastBoundGraphics().pipeline_state) { - objlist.add(pipe->Handle()); - } - const std::string resource_description = "index " + sync_state_.FormatHandle(*index_buf_state); - const auto error = error_messages_.BufferError(hazard, *this, loc.function, resource_description, range); - skip |= sync_state_.SyncError(hazard.Hazard(), objlist, loc, error); + const auto index_buffer = sync_state_.Get<vvl::Buffer>(index_binding.Buffer()); + if (!index_buffer) { + return {}; } + const uint32_t index_size = IndexTypeByteSize(index_binding.index_type); + const VkDeviceSize offset = index_binding.BufferOffset(); + const AccessRange range = MakeRangeForIndexData(offset, first_index, index_count, index_size); - // TODO: Shader instrumentation support is needed to read index buffer content and determine the range of accessed - // versices (new syncval mode). Scanning index buffer for each draw call might be the simplest option to implement - // and the most reliable one, but potentially it can be heavy (still might be okay, testing is needed). - // Some other options: a) rescan index buffer when its modification is detected, b) scan index buffer only once and - // then assume it is immutable (common scenario). - // skip |= ValidateDrawVertex(?, ?, loc); - - return skip; + VertexInputAccesses result; + result.pipeline = pipeline; + result.access_index = SYNC_INDEX_INPUT_INDEX_READ; + result.accesses.emplace_back(VertexInputCommand::Access{index_buffer.get(), range}); + return result; } -void CommandBufferContext::RecordDrawVertexIndex(uint32_t indexCount, uint32_t firstIndex, const ResourceUsageTag tag) { - const auto& index_binding = cb_state_->index_buffer_binding; - // TODO - Handle https://gitlab.khronos.org/vulkan/Vulkan-ValidationLayers/-/issues/45 - const auto index_buf_state = sync_state_.Get<vvl::Buffer>(index_binding.Buffer()); - if (!index_buf_state) { - return; - } +bool CommandBufferContext::ValidateDrawVertex(uint32_t vertex_count, uint32_t first_vertex, const Location& loc) const { + const auto vertex_accesses = CollectVertexAccesses(first_vertex, vertex_count); + return vertex_accesses.MakeCommand().Validate(*this, loc); +} - const uint32_t index_size = IndexTypeByteSize(index_binding.index_type); - const AccessRange range = MakeRangeForIndexData(index_binding.BufferOffset(), firstIndex, indexCount, index_size); - const ResourceUsageTagEx tag_ex = AddCommandHandle(tag, index_buf_state->Handle()); - current_context_->UpdateAccessState(*index_buf_state, SYNC_INDEX_INPUT_INDEX_READ, range, tag_ex); +void CommandBufferContext::RecordDrawVertex(uint32_t vertex_count, uint32_t first_vertex, ResourceUsageTag tag) { + auto vertex_accesses = CollectVertexAccesses(first_vertex, vertex_count); + vertex_accesses.RegisterResources(*this, tag); + vertex_accesses.MakeCommand().Apply(environment_, tag, *current_context_); +} - // TODO: Shader instrumentation support is needed to read index buffer content and determine the range of accessed - // versices (new syncval mode). Scanning index buffer for each draw call might be the simplest option to implement - // and the most reliable one, but potentially it can be heavy (still might be okay, testing is needed). - // Some other options: a) rescan index buffer when its modification is detected, b) scan index buffer only once and - // then assume it is immutable (common scenario). - // RecordDrawVertex(?, ?, tag); +bool CommandBufferContext::ValidateDrawVertexIndex(uint32_t index_count, uint32_t first_index, const Location& loc) const { + const auto index_accesses = CollectIndexAccesses(first_index, index_count); + // TODO: Shader instrumentation support is needed to read index buffer content and determine + // the range of accessed versices. This is an expensive scan and likely has to be off by default. + return index_accesses.MakeCommand().Validate(*this, loc); +} + +void CommandBufferContext::RecordDrawVertexIndex(uint32_t index_count, uint32_t first_index, ResourceUsageTag tag) { + auto index_accesses = CollectIndexAccesses(first_index, index_count); + index_accesses.RegisterResources(*this, tag); + index_accesses.MakeCommand().Apply(environment_, tag, *current_context_); } static bool IsStencilWriteable(const LastBound& last_bound_state) { @@ -1380,6 +1339,10 @@ import_common(command_data.dispatch_indirect_commands[index], command_data, tag, entry.tag_count); continue; } + case CommandType::kDraw: { + import_draw(command_data.draw_commands[index], command_data, tag, entry.tag_count); + continue; + } case CommandType::kDrawIndirect: { import_draw(command_data.draw_indirect_commands[index], command_data, tag, entry.tag_count); continue;
diff --git a/layers/sync/sync_command_buffer.h b/layers/sync/sync_command_buffer.h index c62fb17..7342fea 100644 --- a/layers/sync/sync_command_buffer.h +++ b/layers/sync/sync_command_buffer.h
@@ -226,6 +226,8 @@ DrawAttachmentCommand GetDrawAttachmentCommand() const; + VertexInputAccesses CollectVertexAccesses(uint32_t first_vertex, uint32_t vertex_count) const; + VertexInputAccesses CollectIndexAccesses(uint32_t first_index, uint32_t index_count) const; bool ValidateDrawVertex(uint32_t vertexCount, uint32_t firstVertex, const Location& loc) const; void RecordDrawVertex(uint32_t vertexCount, uint32_t firstVertex, ResourceUsageTag tag); bool ValidateDrawVertexIndex(uint32_t indexCount, uint32_t firstIndex, const Location& loc) const;
diff --git a/layers/sync/sync_validation.cpp b/layers/sync/sync_validation.cpp index 2be7239..7e7e425 100644 --- a/layers/sync/sync_validation.cpp +++ b/layers/sync/sync_validation.cpp
@@ -1196,14 +1196,18 @@ bool SyncValidator::PreCallValidateCmdDraw(VkCommandBuffer commandBuffer, uint32_t vertexCount, uint32_t instanceCount, uint32_t firstVertex, uint32_t firstInstance, const ErrorObject& error_obj) const { - bool skip = false; + if (!syncval_settings.IsRecordTimeValidationEnabled()) { + return false; + } const auto cb_state = Get<vvl::CommandBuffer>(commandBuffer); const CommandBufferContext& cb_context = GetCommandBufferContext(*cb_state); - skip |= cb_context.ValidateDispatchDrawDescriptorSet(VK_PIPELINE_BIND_POINT_GRAPHICS, error_obj.location); - skip |= cb_context.ValidateDrawVertex(vertexCount, firstVertex, error_obj.location); - skip |= cb_context.ValidateDrawAttachment(error_obj.location); - return skip; + const DescriptorAccesses descriptor_accesses = cb_context.CollectDescriptorAccesses(VK_PIPELINE_BIND_POINT_GRAPHICS); + const VertexInputAccesses vertex_accesses = cb_context.CollectVertexAccesses(firstVertex, vertexCount); + + const DrawCommand command{descriptor_accesses.MakeCommand(), vertex_accesses.MakeCommand(), + cb_context.GetDrawAttachmentCommand()}; + return command.Validate(cb_context, error_obj.location); } void SyncValidator::PostCallRecordCmdDraw(VkCommandBuffer commandBuffer, uint32_t vertexCount, uint32_t instanceCount, @@ -1212,22 +1216,36 @@ CommandBufferContext& cb_context = GetCommandBufferContext(*cb_state); const ResourceUsageTag tag = cb_context.NextCommandTag(record_obj.location.function); - cb_context.RecordDispatchDrawDescriptorSet(VK_PIPELINE_BIND_POINT_GRAPHICS, tag); - cb_context.RecordDrawVertex(vertexCount, firstVertex, tag); - cb_context.RecordDrawAttachment(tag); + DescriptorAccesses descriptor_accesses = cb_context.CollectDescriptorAccesses(VK_PIPELINE_BIND_POINT_GRAPHICS); + descriptor_accesses.RegisterResources(cb_context, tag); + VertexInputAccesses vertex_accesses = cb_context.CollectVertexAccesses(firstVertex, vertexCount); + vertex_accesses.RegisterResources(cb_context, tag); + + const DrawCommand command{descriptor_accesses.MakeCommand(), vertex_accesses.MakeCommand(), + cb_context.GetDrawAttachmentCommand()}; + if (syncval_settings.IsRecordTimeValidationEnabled()) { + command.Apply(cb_context.GetSyncEnvironment(), tag, cb_context.GetCurrentAccessContext()); + } + if (syncval_settings.full_validation) { + cb_context.StoreCommand(tag, command); + } } bool SyncValidator::PreCallValidateCmdDrawIndexed(VkCommandBuffer commandBuffer, uint32_t indexCount, uint32_t instanceCount, uint32_t firstIndex, int32_t vertexOffset, uint32_t firstInstance, const ErrorObject& error_obj) const { - bool skip = false; + if (!syncval_settings.IsRecordTimeValidationEnabled()) { + return false; + } const auto cb_state = Get<vvl::CommandBuffer>(commandBuffer); const CommandBufferContext& cb_context = GetCommandBufferContext(*cb_state); - skip |= cb_context.ValidateDispatchDrawDescriptorSet(VK_PIPELINE_BIND_POINT_GRAPHICS, error_obj.location); - skip |= cb_context.ValidateDrawVertexIndex(indexCount, firstIndex, error_obj.location); - skip |= cb_context.ValidateDrawAttachment(error_obj.location); - return skip; + const DescriptorAccesses descriptor_accesses = cb_context.CollectDescriptorAccesses(VK_PIPELINE_BIND_POINT_GRAPHICS); + const VertexInputAccesses vertex_accesses = cb_context.CollectIndexAccesses(firstIndex, indexCount); + + const DrawCommand command{descriptor_accesses.MakeCommand(), vertex_accesses.MakeCommand(), + cb_context.GetDrawAttachmentCommand()}; + return command.Validate(cb_context, error_obj.location); } void SyncValidator::PostCallRecordCmdDrawIndexed(VkCommandBuffer commandBuffer, uint32_t indexCount, uint32_t instanceCount, @@ -1237,9 +1255,19 @@ CommandBufferContext& cb_context = GetCommandBufferContext(*cb_state); const ResourceUsageTag tag = cb_context.NextCommandTag(record_obj.location.function); - cb_context.RecordDispatchDrawDescriptorSet(VK_PIPELINE_BIND_POINT_GRAPHICS, tag); - cb_context.RecordDrawVertexIndex(indexCount, firstIndex, tag); - cb_context.RecordDrawAttachment(tag); + DescriptorAccesses descriptor_accesses = cb_context.CollectDescriptorAccesses(VK_PIPELINE_BIND_POINT_GRAPHICS); + descriptor_accesses.RegisterResources(cb_context, tag); + VertexInputAccesses vertex_accesses = cb_context.CollectIndexAccesses(firstIndex, indexCount); + vertex_accesses.RegisterResources(cb_context, tag); + + const DrawCommand command{descriptor_accesses.MakeCommand(), vertex_accesses.MakeCommand(), + cb_context.GetDrawAttachmentCommand()}; + if (syncval_settings.IsRecordTimeValidationEnabled()) { + command.Apply(cb_context.GetSyncEnvironment(), tag, cb_context.GetCurrentAccessContext()); + } + if (syncval_settings.full_validation) { + cb_context.StoreCommand(tag, command); + } } bool SyncValidator::PreCallValidateCmdDrawIndirect(VkCommandBuffer commandBuffer, VkBuffer buffer, VkDeviceSize offset,
diff --git a/tests/unit/sync_val.cpp b/tests/unit/sync_val.cpp index 4c91c91..44eb00f 100644 --- a/tests/unit/sync_val.cpp +++ b/tests/unit/sync_val.cpp
@@ -7236,3 +7236,108 @@ m_errorMonitor->VerifyFound(); m_default_queue->Wait(); } + +TEST_F(NegativeSyncVal, DrawVertexInputWAR) { + TEST_DESCRIPTION("Vertex buffer write after a draw"); + SetTargetApiVersion(VK_API_VERSION_1_3); + AddRequiredFeature(vkt::Feature::dynamicRendering); + RETURN_IF_SKIP(InitSyncVal()); + + VkPipelineRenderingCreateInfo pipeline_rendering = vku::InitStructHelper(); + CreatePipelineHelper pipe(*this, &pipeline_rendering); + + VkVertexInputBindingDescription binding{0, 16, VK_VERTEX_INPUT_RATE_VERTEX}; + VkVertexInputAttributeDescription attribute{0, 0, VK_FORMAT_R32G32B32A32_SFLOAT, 0}; + pipe.vi_ci_.vertexBindingDescriptionCount = 1; + pipe.vi_ci_.pVertexBindingDescriptions = &binding; + pipe.vi_ci_.vertexAttributeDescriptionCount = 1; + pipe.vi_ci_.pVertexAttributeDescriptions = &attribute; + pipe.cb_ci_.attachmentCount = 0; + pipe.CreateGraphicsPipeline(); + + VkRenderingInfo rendering_info = vku::InitStructHelper(); + rendering_info.renderArea.extent = {32, 32}; + rendering_info.layerCount = 1; + + const VkBufferUsageFlags usage = VK_BUFFER_USAGE_VERTEX_BUFFER_BIT | VK_BUFFER_USAGE_TRANSFER_DST_BIT; + vkt::Buffer buffer(*m_device, 128, usage); + vkt::Buffer other_buffer(*m_device, 128, usage); + const VkDeviceSize offset = 16; + + vkt::CommandBuffer draw_cb(*m_device, m_command_pool); + draw_cb.Begin(); + draw_cb.BeginRendering(rendering_info); + vk::CmdBindPipeline(draw_cb, VK_PIPELINE_BIND_POINT_GRAPHICS, pipe); + vk::CmdBindVertexBuffers(draw_cb, 0, 1, &buffer.handle(), &offset); + vk::CmdDraw(draw_cb, 3, 1, 2 /* start reading at byte 48 */, 0); + + // Change bindings to check that submit validation still uses the buffer from the initial binding + vk::CmdBindVertexBuffers(draw_cb, 0, 1, &other_buffer.handle(), &offset); + draw_cb.EndRendering(); + draw_cb.End(); + + vkt::CommandBuffer fill_cb(*m_device, m_command_pool); + fill_cb.Begin(); + vk::CmdFillBuffer(fill_cb, buffer, 48, 4, 0); + fill_cb.End(); + + m_default_queue->Submit(draw_cb); + m_errorMonitor->SetDesiredError("SYNC-HAZARD-WRITE-AFTER-READ"); + m_default_queue->Submit(fill_cb); + m_errorMonitor->VerifyFound(); + m_default_queue->Wait(); +} + +TEST_F(NegativeSyncVal, DrawIndexInputRAW) { + TEST_DESCRIPTION("Index buffer read after a write"); + SetTargetApiVersion(VK_API_VERSION_1_3); + AddRequiredFeature(vkt::Feature::dynamicRendering); + RETURN_IF_SKIP(InitSyncVal()); + + VkPipelineRenderingCreateInfo pipeline_rendering = vku::InitStructHelper(); + CreatePipelineHelper pipe(*this, &pipeline_rendering); + + VkVertexInputBindingDescription binding{0, 16, VK_VERTEX_INPUT_RATE_VERTEX}; + VkVertexInputAttributeDescription attribute{0, 0, VK_FORMAT_R32G32B32A32_SFLOAT, 0}; + pipe.vi_ci_.vertexBindingDescriptionCount = 1; + pipe.vi_ci_.pVertexBindingDescriptions = &binding; + pipe.vi_ci_.vertexAttributeDescriptionCount = 1; + pipe.vi_ci_.pVertexAttributeDescriptions = &attribute; + pipe.cb_ci_.attachmentCount = 0; + pipe.CreateGraphicsPipeline(); + + VkRenderingInfo rendering_info = vku::InitStructHelper(); + rendering_info.renderArea.extent = {32, 32}; + rendering_info.layerCount = 1; + + const VkBufferUsageFlags usage = + VK_BUFFER_USAGE_VERTEX_BUFFER_BIT | VK_BUFFER_USAGE_INDEX_BUFFER_BIT | VK_BUFFER_USAGE_TRANSFER_DST_BIT; + vkt::Buffer buffer(*m_device, 128, usage); + vkt::Buffer other_buffer(*m_device, 128, usage); + const VkDeviceSize offset = 16; + + vkt::CommandBuffer draw_cb(*m_device, m_command_pool); + draw_cb.Begin(); + draw_cb.BeginRendering(rendering_info); + vk::CmdBindPipeline(draw_cb, VK_PIPELINE_BIND_POINT_GRAPHICS, pipe); + vk::CmdBindVertexBuffers(draw_cb, 0, 1, &buffer.handle(), &offset); + vk::CmdBindIndexBuffer(draw_cb, buffer, offset, VK_INDEX_TYPE_UINT32); + vk::CmdDrawIndexed(draw_cb, 3, 1, 2 /* start reading indices at byte 24 */, 0, 0); + + // Change bindings to check that submit validation still uses the initial binding + vk::CmdBindIndexBuffer(draw_cb, other_buffer, 0, VK_INDEX_TYPE_UINT16); + vk::CmdBindVertexBuffers(draw_cb, 0, 1, &other_buffer.handle(), &offset); + draw_cb.EndRendering(); + draw_cb.End(); + + vkt::CommandBuffer fill_cb(*m_device, m_command_pool); + fill_cb.Begin(); + vk::CmdFillBuffer(fill_cb, buffer, 24, 4, 0); + fill_cb.End(); + + m_default_queue->Submit(fill_cb); + m_errorMonitor->SetDesiredError("SYNC-HAZARD-READ-AFTER-WRITE"); + m_default_queue->Submit(draw_cb); + m_errorMonitor->VerifyFound(); + m_default_queue->Wait(); +}