From 05045e0ad8588d7c679ec93d12b9921104f7bac8 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Thu, 28 Jan 2021 14:44:15 +0000 Subject: [PATCH] dawn_native: Do CommandBufferStateTracker validation at encoding time This is the last piece of validation that was done in a separate validation pass so the validation pass is removed. Bug: dawn:635 Change-Id: I91ce5d5a512ac188f3dd56c90db9e69aee518844 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/38845 Commit-Queue: Corentin Wallez Reviewed-by: Austin Eng --- src/dawn_native/CommandEncoder.cpp | 67 ------ src/dawn_native/CommandValidation.cpp | 229 -------------------- src/dawn_native/CommandValidation.h | 4 - src/dawn_native/ComputePassEncoder.cpp | 7 + src/dawn_native/ProgrammablePassEncoder.cpp | 2 + src/dawn_native/ProgrammablePassEncoder.h | 2 + src/dawn_native/RenderBundleEncoder.cpp | 1 - src/dawn_native/RenderEncoderBase.cpp | 12 + src/dawn_native/RenderPassEncoder.cpp | 2 + 9 files changed, 25 insertions(+), 301 deletions(-) diff --git a/src/dawn_native/CommandEncoder.cpp b/src/dawn_native/CommandEncoder.cpp index d6d2de1008..e8c81df054 100644 --- a/src/dawn_native/CommandEncoder.cpp +++ b/src/dawn_native/CommandEncoder.cpp @@ -904,73 +904,6 @@ namespace dawn_native { return DAWN_VALIDATION_ERROR("Each Push must be balanced by a corresponding Pop."); } - commands->Reset(); - Command type; - while (commands->NextCommandId(&type)) { - switch (type) { - case Command::BeginComputePass: { - commands->NextCommand(); - DAWN_TRY(ValidateComputePass(commands)); - break; - } - - case Command::BeginRenderPass: { - const BeginRenderPassCmd* cmd = commands->NextCommand(); - DAWN_TRY(ValidateRenderPass(commands, cmd)); - break; - } - - case Command::CopyBufferToBuffer: { - commands->NextCommand(); - break; - } - - case Command::CopyBufferToTexture: { - commands->NextCommand(); - break; - } - - case Command::CopyTextureToBuffer: { - commands->NextCommand(); - break; - } - - case Command::CopyTextureToTexture: { - commands->NextCommand(); - break; - } - - case Command::InsertDebugMarker: { - const InsertDebugMarkerCmd* cmd = commands->NextCommand(); - commands->NextData(cmd->length + 1); - break; - } - - case Command::PopDebugGroup: { - commands->NextCommand(); - break; - } - - case Command::PushDebugGroup: { - const PushDebugGroupCmd* cmd = commands->NextCommand(); - commands->NextData(cmd->length + 1); - break; - } - - case Command::ResolveQuerySet: { - commands->NextCommand(); - break; - } - - case Command::WriteTimestamp: { - commands->NextCommand(); - break; - } - default: - return DAWN_VALIDATION_ERROR("Command disallowed outside of a pass"); - } - } - return {}; } diff --git a/src/dawn_native/CommandValidation.cpp b/src/dawn_native/CommandValidation.cpp index ed0f184cc4..04dfb22f46 100644 --- a/src/dawn_native/CommandValidation.cpp +++ b/src/dawn_native/CommandValidation.cpp @@ -27,235 +27,6 @@ namespace dawn_native { - namespace { - - inline MaybeError ValidateRenderBundleCommand(CommandIterator* commands, - Command type, - CommandBufferStateTracker* commandBufferState, - const char* disallowedMessage) { - switch (type) { - case Command::Draw: { - commands->NextCommand(); - DAWN_TRY(commandBufferState->ValidateCanDraw()); - break; - } - - case Command::DrawIndexed: { - commands->NextCommand(); - DAWN_TRY(commandBufferState->ValidateCanDrawIndexed()); - break; - } - - case Command::DrawIndirect: { - commands->NextCommand(); - DAWN_TRY(commandBufferState->ValidateCanDraw()); - break; - } - - case Command::DrawIndexedIndirect: { - commands->NextCommand(); - DAWN_TRY(commandBufferState->ValidateCanDrawIndexed()); - break; - } - - case Command::InsertDebugMarker: { - InsertDebugMarkerCmd* cmd = commands->NextCommand(); - commands->NextData(cmd->length + 1); - break; - } - - case Command::PopDebugGroup: { - commands->NextCommand(); - break; - } - - case Command::PushDebugGroup: { - PushDebugGroupCmd* cmd = commands->NextCommand(); - commands->NextData(cmd->length + 1); - break; - } - - case Command::SetRenderPipeline: { - SetRenderPipelineCmd* cmd = commands->NextCommand(); - RenderPipelineBase* pipeline = cmd->pipeline.Get(); - commandBufferState->SetRenderPipeline(pipeline); - break; - } - - case Command::SetBindGroup: { - SetBindGroupCmd* cmd = commands->NextCommand(); - if (cmd->dynamicOffsetCount > 0) { - commands->NextData(cmd->dynamicOffsetCount); - } - - commandBufferState->SetBindGroup(cmd->index, cmd->group.Get()); - break; - } - - case Command::SetIndexBuffer: { - SetIndexBufferCmd* cmd = commands->NextCommand(); - commandBufferState->SetIndexBuffer(cmd->format); - break; - } - - case Command::SetVertexBuffer: { - SetVertexBufferCmd* cmd = commands->NextCommand(); - commandBufferState->SetVertexBuffer(cmd->slot); - break; - } - - default: - return DAWN_VALIDATION_ERROR(disallowedMessage); - } - - return {}; - } - - } // namespace - - MaybeError ValidateRenderBundle(CommandIterator* commands) { - CommandBufferStateTracker commandBufferState; - Command type; - while (commands->NextCommandId(&type)) { - DAWN_TRY(ValidateRenderBundleCommand(commands, type, &commandBufferState, - "Command disallowed inside a render bundle")); - } - - return {}; - } - - MaybeError ValidateRenderPass(CommandIterator* commands, const BeginRenderPassCmd* renderPass) { - CommandBufferStateTracker commandBufferState; - Command type; - while (commands->NextCommandId(&type)) { - switch (type) { - case Command::BeginOcclusionQuery: { - commands->NextCommand(); - break; - } - - case Command::EndOcclusionQuery: { - commands->NextCommand(); - break; - } - - case Command::EndRenderPass: { - commands->NextCommand(); - return {}; - } - - case Command::ExecuteBundles: { - ExecuteBundlesCmd* cmd = commands->NextCommand(); - commands->NextData>(cmd->count); - commandBufferState = CommandBufferStateTracker{}; - break; - } - - case Command::SetStencilReference: { - commands->NextCommand(); - break; - } - - case Command::SetBlendColor: { - commands->NextCommand(); - break; - } - - case Command::SetViewport: { - commands->NextCommand(); - break; - } - - case Command::SetScissorRect: { - commands->NextCommand(); - break; - } - - case Command::WriteTimestamp: { - commands->NextCommand(); - break; - } - - default: - DAWN_TRY( - ValidateRenderBundleCommand(commands, type, &commandBufferState, - "Command disallowed inside a render pass")); - } - } - - UNREACHABLE(); - return DAWN_VALIDATION_ERROR("Unfinished render pass"); - } - - MaybeError ValidateComputePass(CommandIterator* commands) { - CommandBufferStateTracker commandBufferState; - Command type; - while (commands->NextCommandId(&type)) { - switch (type) { - case Command::EndComputePass: { - commands->NextCommand(); - return {}; - } - - case Command::Dispatch: { - commands->NextCommand(); - DAWN_TRY(commandBufferState.ValidateCanDispatch()); - break; - } - - case Command::DispatchIndirect: { - commands->NextCommand(); - DAWN_TRY(commandBufferState.ValidateCanDispatch()); - break; - } - - case Command::InsertDebugMarker: { - InsertDebugMarkerCmd* cmd = commands->NextCommand(); - commands->NextData(cmd->length + 1); - break; - } - - case Command::PopDebugGroup: { - commands->NextCommand(); - break; - } - - case Command::PushDebugGroup: { - PushDebugGroupCmd* cmd = commands->NextCommand(); - commands->NextData(cmd->length + 1); - break; - } - - case Command::SetComputePipeline: { - SetComputePipelineCmd* cmd = commands->NextCommand(); - ComputePipelineBase* pipeline = cmd->pipeline.Get(); - commandBufferState.SetComputePipeline(pipeline); - break; - } - - case Command::SetBindGroup: { - SetBindGroupCmd* cmd = commands->NextCommand(); - if (cmd->dynamicOffsetCount > 0) { - commands->NextData(cmd->dynamicOffsetCount); - } - commandBufferState.SetBindGroup(cmd->index, cmd->group.Get()); - break; - } - - case Command::WriteTimestamp: { - commands->NextCommand(); - break; - } - - default: - return DAWN_VALIDATION_ERROR("Command disallowed inside a compute pass"); - } - } - - UNREACHABLE(); - return DAWN_VALIDATION_ERROR("Unfinished compute pass"); - } - // Performs the per-pass usage validation checks // This will eventually need to differentiate between render and compute passes. // It will be valid to use a buffer both as uniform and storage in the same compute pass. diff --git a/src/dawn_native/CommandValidation.h b/src/dawn_native/CommandValidation.h index 26db8b42b7..390920341f 100644 --- a/src/dawn_native/CommandValidation.h +++ b/src/dawn_native/CommandValidation.h @@ -29,10 +29,6 @@ namespace dawn_native { struct PassResourceUsage; struct TexelBlockInfo; - MaybeError ValidateRenderBundle(CommandIterator* commands); - MaybeError ValidateRenderPass(CommandIterator* commands, const BeginRenderPassCmd* renderPass); - MaybeError ValidateComputePass(CommandIterator* commands); - MaybeError ValidatePassResourceUsage(const PassResourceUsage& usage); MaybeError ValidateTimestampQuery(QuerySetBase* querySet, uint32_t queryIndex); diff --git a/src/dawn_native/ComputePassEncoder.cpp b/src/dawn_native/ComputePassEncoder.cpp index ac8eb09013..3d5224985c 100644 --- a/src/dawn_native/ComputePassEncoder.cpp +++ b/src/dawn_native/ComputePassEncoder.cpp @@ -61,6 +61,10 @@ namespace dawn_native { void ComputePassEncoder::Dispatch(uint32_t x, uint32_t y, uint32_t z) { mEncodingContext->TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError { + if (IsValidationEnabled()) { + DAWN_TRY(mCommandBufferState.ValidateCanDispatch()); + } + DispatchCmd* dispatch = allocator->Allocate(Command::Dispatch); dispatch->x = x; dispatch->y = y; @@ -75,6 +79,7 @@ namespace dawn_native { if (IsValidationEnabled()) { DAWN_TRY(GetDevice()->ValidateObject(indirectBuffer)); DAWN_TRY(ValidateCanUseAs(indirectBuffer, wgpu::BufferUsage::Indirect)); + DAWN_TRY(mCommandBufferState.ValidateCanDispatch()); // Indexed dispatches need a compute-shader based validation to check that the // dispatch sizes aren't too big. Disallow them as unsafe until the validation is @@ -113,6 +118,8 @@ namespace dawn_native { DAWN_TRY(GetDevice()->ValidateObject(pipeline)); } + mCommandBufferState.SetComputePipeline(pipeline); + SetComputePipelineCmd* cmd = allocator->Allocate(Command::SetComputePipeline); cmd->pipeline = pipeline; diff --git a/src/dawn_native/ProgrammablePassEncoder.cpp b/src/dawn_native/ProgrammablePassEncoder.cpp index 81d7868351..362362dc63 100644 --- a/src/dawn_native/ProgrammablePassEncoder.cpp +++ b/src/dawn_native/ProgrammablePassEncoder.cpp @@ -211,6 +211,8 @@ namespace dawn_native { } } + mCommandBufferState.SetBindGroup(groupIndex, group); + SetBindGroupCmd* cmd = allocator->Allocate(Command::SetBindGroup); cmd->index = groupIndex; cmd->group = group; diff --git a/src/dawn_native/ProgrammablePassEncoder.h b/src/dawn_native/ProgrammablePassEncoder.h index 6d6f067d72..fcb74748b5 100644 --- a/src/dawn_native/ProgrammablePassEncoder.h +++ b/src/dawn_native/ProgrammablePassEncoder.h @@ -15,6 +15,7 @@ #ifndef DAWNNATIVE_PROGRAMMABLEPASSENCODER_H_ #define DAWNNATIVE_PROGRAMMABLEPASSENCODER_H_ +#include "dawn_native/CommandBufferStateTracker.h" #include "dawn_native/CommandEncoder.h" #include "dawn_native/Error.h" #include "dawn_native/ObjectBase.h" @@ -57,6 +58,7 @@ namespace dawn_native { PassResourceUsageTracker mUsageTracker; uint64_t mDebugGroupStackSize = 0; + CommandBufferStateTracker mCommandBufferState; private: const bool mValidationEnabled; diff --git a/src/dawn_native/RenderBundleEncoder.cpp b/src/dawn_native/RenderBundleEncoder.cpp index fbc03d37d6..b8f3c82355 100644 --- a/src/dawn_native/RenderBundleEncoder.cpp +++ b/src/dawn_native/RenderBundleEncoder.cpp @@ -131,7 +131,6 @@ namespace dawn_native { TRACE_EVENT0(GetDevice()->GetPlatform(), Validation, "RenderBundleEncoder::ValidateFinish"); DAWN_TRY(GetDevice()->ValidateObject(this)); DAWN_TRY(ValidatePassResourceUsage(usages)); - DAWN_TRY(ValidateRenderBundle(commands)); return {}; } diff --git a/src/dawn_native/RenderEncoderBase.cpp b/src/dawn_native/RenderEncoderBase.cpp index 18d587cb59..f309bb2664 100644 --- a/src/dawn_native/RenderEncoderBase.cpp +++ b/src/dawn_native/RenderEncoderBase.cpp @@ -61,6 +61,8 @@ namespace dawn_native { uint32_t firstInstance) { mEncodingContext->TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError { if (IsValidationEnabled()) { + DAWN_TRY(mCommandBufferState.ValidateCanDraw()); + if (mDisableBaseInstance && firstInstance != 0) { return DAWN_VALIDATION_ERROR("Non-zero first instance not supported"); } @@ -83,6 +85,8 @@ namespace dawn_native { uint32_t firstInstance) { mEncodingContext->TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError { if (IsValidationEnabled()) { + DAWN_TRY(mCommandBufferState.ValidateCanDrawIndexed()); + if (mDisableBaseInstance && firstInstance != 0) { return DAWN_VALIDATION_ERROR("Non-zero first instance not supported"); } @@ -107,6 +111,7 @@ namespace dawn_native { if (IsValidationEnabled()) { DAWN_TRY(GetDevice()->ValidateObject(indirectBuffer)); DAWN_TRY(ValidateCanUseAs(indirectBuffer, wgpu::BufferUsage::Indirect)); + DAWN_TRY(mCommandBufferState.ValidateCanDraw()); if (indirectOffset % 4 != 0) { return DAWN_VALIDATION_ERROR("Indirect offset must be a multiple of 4"); @@ -134,6 +139,7 @@ namespace dawn_native { if (IsValidationEnabled()) { DAWN_TRY(GetDevice()->ValidateObject(indirectBuffer)); DAWN_TRY(ValidateCanUseAs(indirectBuffer, wgpu::BufferUsage::Indirect)); + DAWN_TRY(mCommandBufferState.ValidateCanDrawIndexed()); // Indexed indirect draws need a compute-shader based validation check that the // range of indices is contained inside the index buffer on Metal. Disallow them as @@ -178,6 +184,8 @@ namespace dawn_native { } } + mCommandBufferState.SetRenderPipeline(pipeline); + SetRenderPipelineCmd* cmd = allocator->Allocate(Command::SetRenderPipeline); cmd->pipeline = pipeline; @@ -227,6 +235,8 @@ namespace dawn_native { } } + mCommandBufferState.SetIndexBuffer(format); + SetIndexBufferCmd* cmd = allocator->Allocate(Command::SetIndexBuffer); cmd->buffer = buffer; @@ -272,6 +282,8 @@ namespace dawn_native { } } + mCommandBufferState.SetVertexBuffer(VertexBufferSlot(uint8_t(slot))); + SetVertexBufferCmd* cmd = allocator->Allocate(Command::SetVertexBuffer); cmd->slot = VertexBufferSlot(static_cast(slot)); diff --git a/src/dawn_native/RenderPassEncoder.cpp b/src/dawn_native/RenderPassEncoder.cpp index dbc0463b1c..c5c31a301d 100644 --- a/src/dawn_native/RenderPassEncoder.cpp +++ b/src/dawn_native/RenderPassEncoder.cpp @@ -208,6 +208,8 @@ namespace dawn_native { } } + mCommandBufferState = CommandBufferStateTracker{}; + ExecuteBundlesCmd* cmd = allocator->Allocate(Command::ExecuteBundles); cmd->count = count;