diff --git a/src/dawn_native/Buffer.cpp b/src/dawn_native/Buffer.cpp index a44e3c22bd..c57d23a129 100644 --- a/src/dawn_native/Buffer.cpp +++ b/src/dawn_native/Buffer.cpp @@ -116,7 +116,7 @@ namespace dawn_native { mUsage(descriptor->usage), mState(BufferState::Unmapped) { // Add readonly storage usage if the buffer has a storage usage. The validation rules in - // PassResourceUsageTracker::ValidateUsages will make sure we don't use both at the same + // ValidatePassResourceUsage will make sure we don't use both at the same // time. if (mUsage & wgpu::BufferUsage::Storage) { mUsage |= kReadOnlyStorage; diff --git a/src/dawn_native/CommandEncoder.cpp b/src/dawn_native/CommandEncoder.cpp index 5af8ba0ab4..e0de3a215a 100644 --- a/src/dawn_native/CommandEncoder.cpp +++ b/src/dawn_native/CommandEncoder.cpp @@ -24,7 +24,6 @@ #include "dawn_native/ComputePassEncoder.h" #include "dawn_native/Device.h" #include "dawn_native/ErrorData.h" -#include "dawn_native/PassResourceUsageTracker.h" #include "dawn_native/RenderPassEncoder.h" #include "dawn_native/RenderPipeline.h" #include "dawn_platform/DawnPlatform.h" @@ -243,7 +242,7 @@ namespace dawn_native { return {}; } - MaybeError ValidateCanUseAs(BufferBase* buffer, wgpu::BufferUsage usage) { + MaybeError ValidateCanUseAs(const BufferBase* buffer, wgpu::BufferUsage usage) { ASSERT(wgpu::HasZeroOrOneBits(usage)); if (!(buffer->GetUsage() & usage)) { return DAWN_VALIDATION_ERROR("buffer doesn't have the required usage."); @@ -252,7 +251,7 @@ namespace dawn_native { return {}; } - MaybeError ValidateCanUseAs(TextureBase* texture, wgpu::TextureUsage usage) { + MaybeError ValidateCanUseAs(const TextureBase* texture, wgpu::TextureUsage usage) { ASSERT(wgpu::HasZeroOrOneBits(usage)); if (!(texture->GetUsage() & usage)) { return DAWN_VALIDATION_ERROR("texture doesn't have the required usage."); @@ -467,9 +466,9 @@ namespace dawn_native { } CommandBufferResourceUsage CommandEncoder::AcquireResourceUsages() { - ASSERT(!mWereResourceUsagesAcquired); - mWereResourceUsagesAcquired = true; - return std::move(mResourceUsages); + return CommandBufferResourceUsage{mEncodingContext.AcquirePassUsages(), + std::move(mTopLevelBuffers), + std::move(mTopLevelTextures)}; } CommandIterator CommandEncoder::AcquireCommands() { @@ -503,6 +502,7 @@ namespace dawn_native { RenderPassEncoder* CommandEncoder::BeginRenderPass(const RenderPassDescriptor* descriptor) { DeviceBase* device = GetDevice(); + PassResourceUsageTracker usageTracker; bool success = mEncodingContext.TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError { uint32_t width = 0; @@ -520,18 +520,29 @@ namespace dawn_native { cmd->attachmentState = device->GetOrCreateAttachmentState(descriptor); for (uint32_t i : IterateBitSet(cmd->attachmentState->GetColorAttachmentsMask())) { - cmd->colorAttachments[i].view = descriptor->colorAttachments[i].attachment; - cmd->colorAttachments[i].resolveTarget = - descriptor->colorAttachments[i].resolveTarget; + TextureViewBase* view = descriptor->colorAttachments[i].attachment; + TextureViewBase* resolveTarget = descriptor->colorAttachments[i].resolveTarget; + + cmd->colorAttachments[i].view = view; + cmd->colorAttachments[i].resolveTarget = resolveTarget; cmd->colorAttachments[i].loadOp = descriptor->colorAttachments[i].loadOp; cmd->colorAttachments[i].storeOp = descriptor->colorAttachments[i].storeOp; cmd->colorAttachments[i].clearColor = descriptor->colorAttachments[i].clearColor; + + usageTracker.TextureUsedAs(view->GetTexture(), + wgpu::TextureUsage::OutputAttachment); + + if (resolveTarget != nullptr) { + usageTracker.TextureUsedAs(resolveTarget->GetTexture(), + wgpu::TextureUsage::OutputAttachment); + } } if (cmd->attachmentState->HasDepthStencilAttachment()) { - cmd->depthStencilAttachment.view = - descriptor->depthStencilAttachment->attachment; + TextureViewBase* view = descriptor->depthStencilAttachment->attachment; + + cmd->depthStencilAttachment.view = view; cmd->depthStencilAttachment.clearDepth = descriptor->depthStencilAttachment->clearDepth; cmd->depthStencilAttachment.clearStencil = @@ -544,6 +555,9 @@ namespace dawn_native { descriptor->depthStencilAttachment->stencilLoadOp; cmd->depthStencilAttachment.stencilStoreOp = descriptor->depthStencilAttachment->stencilStoreOp; + + usageTracker.TextureUsedAs(view->GetTexture(), + wgpu::TextureUsage::OutputAttachment); } cmd->width = width; @@ -553,7 +567,8 @@ namespace dawn_native { }); if (success) { - RenderPassEncoder* passEncoder = new RenderPassEncoder(device, this, &mEncodingContext); + RenderPassEncoder* passEncoder = + new RenderPassEncoder(device, this, &mEncodingContext, std::move(usageTracker)); mEncodingContext.EnterPass(passEncoder); return passEncoder; } @@ -578,6 +593,10 @@ namespace dawn_native { copy->destinationOffset = destinationOffset; copy->size = size; + if (GetDevice()->IsValidationEnabled()) { + mTopLevelBuffers.insert(source); + mTopLevelBuffers.insert(destination); + } return {}; }); } @@ -610,6 +629,10 @@ namespace dawn_native { copy->source.imageHeight = source->imageHeight; } + if (GetDevice()->IsValidationEnabled()) { + mTopLevelBuffers.insert(source->buffer); + mTopLevelTextures.insert(destination->texture); + } return {}; }); } @@ -642,6 +665,10 @@ namespace dawn_native { copy->destination.imageHeight = destination->imageHeight; } + if (GetDevice()->IsValidationEnabled()) { + mTopLevelTextures.insert(source->texture); + mTopLevelBuffers.insert(destination->buffer); + } return {}; }); } @@ -665,6 +692,10 @@ namespace dawn_native { copy->destination.arrayLayer = destination->arrayLayer; copy->copySize = *copySize; + if (GetDevice()->IsValidationEnabled()) { + mTopLevelTextures.insert(source->texture); + mTopLevelTextures.insert(destination->texture); + } return {}; }); } @@ -704,46 +735,49 @@ namespace dawn_native { } CommandBufferBase* CommandEncoder::Finish(const CommandBufferDescriptor* descriptor) { - if (GetDevice()->ConsumedError(ValidateFinish(descriptor))) { - // Even if finish validation fails, it is now invalid to call any encoding commands on - // this object, so we set its state to finished. - return CommandBufferBase::MakeError(GetDevice()); + DeviceBase* device = GetDevice(); + // Even if mEncodingContext.Finish() validation fails, calling it will mutate the internal + // state of the encoding context. The internal state is set to finished, and subsequent + // calls to encode commands will generate errors. + if (device->ConsumedError(mEncodingContext.Finish()) || + (device->IsValidationEnabled() && + device->ConsumedError(ValidateFinish(mEncodingContext.GetIterator(), + mEncodingContext.GetPassUsages())))) { + return CommandBufferBase::MakeError(device); } ASSERT(!IsError()); - - return GetDevice()->CreateCommandBuffer(this, descriptor); + return device->CreateCommandBuffer(this, descriptor); } // Implementation of the command buffer validation that can be precomputed before submit - - MaybeError CommandEncoder::ValidateFinish(const CommandBufferDescriptor*) { + MaybeError CommandEncoder::ValidateFinish(CommandIterator* commands, + const PerPassUsages& perPassUsages) const { TRACE_EVENT0(GetDevice()->GetPlatform(), Validation, "CommandEncoder::ValidateFinish"); DAWN_TRY(GetDevice()->ValidateObject(this)); - // Even if Finish() validation fails, calling it will mutate the internal state of the - // encoding context. Subsequent calls to encode commands will generate errors. - DAWN_TRY(mEncodingContext.Finish()); + for (const PassResourceUsage& passUsage : perPassUsages) { + DAWN_TRY(ValidatePassResourceUsage(passUsage)); + } uint64_t debugGroupStackSize = 0; - CommandIterator* commands = mEncodingContext.GetIterator(); commands->Reset(); - Command type; while (commands->NextCommandId(&type)) { switch (type) { case Command::BeginComputePass: { commands->NextCommand(); - DAWN_TRY(ValidateComputePass(commands, &mResourceUsages.perPass)); + DAWN_TRY(ValidateComputePass(commands)); } break; case Command::BeginRenderPass: { - BeginRenderPassCmd* cmd = commands->NextCommand(); - DAWN_TRY(ValidateRenderPass(commands, cmd, &mResourceUsages.perPass)); + const BeginRenderPassCmd* cmd = commands->NextCommand(); + DAWN_TRY(ValidateRenderPass(commands, cmd)); } break; case Command::CopyBufferToBuffer: { - CopyBufferToBufferCmd* copy = commands->NextCommand(); + const CopyBufferToBufferCmd* copy = + commands->NextCommand(); DAWN_TRY( ValidateCopySizeFitsInBuffer(copy->source, copy->sourceOffset, copy->size)); @@ -754,13 +788,11 @@ namespace dawn_native { DAWN_TRY(ValidateCanUseAs(copy->source.Get(), wgpu::BufferUsage::CopySrc)); DAWN_TRY(ValidateCanUseAs(copy->destination.Get(), wgpu::BufferUsage::CopyDst)); - - mResourceUsages.topLevelBuffers.insert(copy->source.Get()); - mResourceUsages.topLevelBuffers.insert(copy->destination.Get()); } break; case Command::CopyBufferToTexture: { - CopyBufferToTextureCmd* copy = commands->NextCommand(); + const CopyBufferToTextureCmd* copy = + commands->NextCommand(); DAWN_TRY( ValidateTextureSampleCountInCopyCommands(copy->destination.texture.Get())); @@ -789,13 +821,11 @@ namespace dawn_native { ValidateCanUseAs(copy->source.buffer.Get(), wgpu::BufferUsage::CopySrc)); DAWN_TRY(ValidateCanUseAs(copy->destination.texture.Get(), wgpu::TextureUsage::CopyDst)); - - mResourceUsages.topLevelBuffers.insert(copy->source.buffer.Get()); - mResourceUsages.topLevelTextures.insert(copy->destination.texture.Get()); } break; case Command::CopyTextureToBuffer: { - CopyTextureToBufferCmd* copy = commands->NextCommand(); + const CopyTextureToBufferCmd* copy = + commands->NextCommand(); DAWN_TRY(ValidateTextureSampleCountInCopyCommands(copy->source.texture.Get())); @@ -824,13 +854,10 @@ namespace dawn_native { ValidateCanUseAs(copy->source.texture.Get(), wgpu::TextureUsage::CopySrc)); DAWN_TRY(ValidateCanUseAs(copy->destination.buffer.Get(), wgpu::BufferUsage::CopyDst)); - - mResourceUsages.topLevelTextures.insert(copy->source.texture.Get()); - mResourceUsages.topLevelBuffers.insert(copy->destination.buffer.Get()); } break; case Command::CopyTextureToTexture: { - CopyTextureToTextureCmd* copy = + const CopyTextureToTextureCmd* copy = commands->NextCommand(); DAWN_TRY(ValidateTextureToTextureCopyRestrictions( @@ -852,13 +879,10 @@ namespace dawn_native { ValidateCanUseAs(copy->source.texture.Get(), wgpu::TextureUsage::CopySrc)); DAWN_TRY(ValidateCanUseAs(copy->destination.texture.Get(), wgpu::TextureUsage::CopyDst)); - - mResourceUsages.topLevelTextures.insert(copy->source.texture.Get()); - mResourceUsages.topLevelTextures.insert(copy->destination.texture.Get()); } break; case Command::InsertDebugMarker: { - InsertDebugMarkerCmd* cmd = commands->NextCommand(); + const InsertDebugMarkerCmd* cmd = commands->NextCommand(); commands->NextData(cmd->length + 1); } break; @@ -869,7 +893,7 @@ namespace dawn_native { } break; case Command::PushDebugGroup: { - PushDebugGroupCmd* cmd = commands->NextCommand(); + const PushDebugGroupCmd* cmd = commands->NextCommand(); commands->NextData(cmd->length + 1); debugGroupStackSize++; } break; diff --git a/src/dawn_native/CommandEncoder.h b/src/dawn_native/CommandEncoder.h index 406ff33869..2c89c4bd77 100644 --- a/src/dawn_native/CommandEncoder.h +++ b/src/dawn_native/CommandEncoder.h @@ -61,12 +61,12 @@ namespace dawn_native { CommandBufferBase* Finish(const CommandBufferDescriptor* descriptor); private: - MaybeError ValidateFinish(const CommandBufferDescriptor* descriptor); + MaybeError ValidateFinish(CommandIterator* commands, + const PerPassUsages& perPassUsages) const; EncodingContext mEncodingContext; - - bool mWereResourceUsagesAcquired = false; - CommandBufferResourceUsage mResourceUsages; + std::set mTopLevelBuffers; + std::set mTopLevelTextures; }; } // namespace dawn_native diff --git a/src/dawn_native/CommandValidation.cpp b/src/dawn_native/CommandValidation.cpp index cdcb470dc7..dc7a3ccec5 100644 --- a/src/dawn_native/CommandValidation.cpp +++ b/src/dawn_native/CommandValidation.cpp @@ -19,7 +19,7 @@ #include "dawn_native/Buffer.h" #include "dawn_native/CommandBufferStateTracker.h" #include "dawn_native/Commands.h" -#include "dawn_native/PassResourceUsageTracker.h" +#include "dawn_native/PassResourceUsage.h" #include "dawn_native/RenderBundle.h" #include "dawn_native/RenderPipeline.h" @@ -27,47 +27,8 @@ namespace dawn_native { namespace { - void TrackBindGroupResourceUsage(BindGroupBase* group, - PassResourceUsageTracker* usageTracker) { - const auto& layoutInfo = group->GetLayout()->GetBindingInfo(); - - for (uint32_t i : IterateBitSet(layoutInfo.mask)) { - wgpu::BindingType type = layoutInfo.types[i]; - - switch (type) { - case wgpu::BindingType::UniformBuffer: { - BufferBase* buffer = group->GetBindingAsBufferBinding(i).buffer; - usageTracker->BufferUsedAs(buffer, wgpu::BufferUsage::Uniform); - } break; - - case wgpu::BindingType::StorageBuffer: { - BufferBase* buffer = group->GetBindingAsBufferBinding(i).buffer; - usageTracker->BufferUsedAs(buffer, wgpu::BufferUsage::Storage); - } break; - - case wgpu::BindingType::SampledTexture: { - TextureBase* texture = group->GetBindingAsTextureView(i)->GetTexture(); - usageTracker->TextureUsedAs(texture, wgpu::TextureUsage::Sampled); - } break; - - case wgpu::BindingType::ReadonlyStorageBuffer: { - BufferBase* buffer = group->GetBindingAsBufferBinding(i).buffer; - usageTracker->BufferUsedAs(buffer, kReadOnlyStorage); - } break; - - case wgpu::BindingType::Sampler: - break; - - case wgpu::BindingType::StorageTexture: - UNREACHABLE(); - break; - } - } - } - inline MaybeError ValidateRenderBundleCommand(CommandIterator* commands, Command type, - PassResourceUsageTracker* usageTracker, CommandBufferStateTracker* commandBufferState, const AttachmentState* attachmentState, uint64_t* debugGroupStackSize, @@ -84,17 +45,13 @@ namespace dawn_native { } break; case Command::DrawIndirect: { - DrawIndirectCmd* cmd = commands->NextCommand(); + commands->NextCommand(); DAWN_TRY(commandBufferState->ValidateCanDraw()); - usageTracker->BufferUsedAs(cmd->indirectBuffer.Get(), - wgpu::BufferUsage::Indirect); } break; case Command::DrawIndexedIndirect: { - DrawIndexedIndirectCmd* cmd = commands->NextCommand(); + commands->NextCommand(); DAWN_TRY(commandBufferState->ValidateCanDrawIndexed()); - usageTracker->BufferUsedAs(cmd->indirectBuffer.Get(), - wgpu::BufferUsage::Indirect); } break; case Command::InsertDebugMarker: { @@ -130,21 +87,16 @@ namespace dawn_native { commands->NextData(cmd->dynamicOffsetCount); } - TrackBindGroupResourceUsage(cmd->group.Get(), usageTracker); commandBufferState->SetBindGroup(cmd->index, cmd->group.Get()); } break; case Command::SetIndexBuffer: { - SetIndexBufferCmd* cmd = commands->NextCommand(); - - usageTracker->BufferUsedAs(cmd->buffer.Get(), wgpu::BufferUsage::Index); + commands->NextCommand(); commandBufferState->SetIndexBuffer(); } break; case Command::SetVertexBuffer: { SetVertexBufferCmd* cmd = commands->NextCommand(); - - usageTracker->BufferUsedAs(cmd->buffer.Get(), wgpu::BufferUsage::Vertex); commandBufferState->SetVertexBuffer(cmd->slot); } break; @@ -172,63 +124,31 @@ namespace dawn_native { } MaybeError ValidateRenderBundle(CommandIterator* commands, - const AttachmentState* attachmentState, - PassResourceUsage* resourceUsage) { - PassResourceUsageTracker usageTracker; + const AttachmentState* attachmentState) { CommandBufferStateTracker commandBufferState; uint64_t debugGroupStackSize = 0; Command type; while (commands->NextCommandId(&type)) { - DAWN_TRY(ValidateRenderBundleCommand(commands, type, &usageTracker, &commandBufferState, + DAWN_TRY(ValidateRenderBundleCommand(commands, type, &commandBufferState, attachmentState, &debugGroupStackSize, "Command disallowed inside a render bundle")); } DAWN_TRY(ValidateFinalDebugGroupStackSize(debugGroupStackSize)); - DAWN_TRY(usageTracker.ValidateRenderPassUsages()); - ASSERT(resourceUsage != nullptr); - *resourceUsage = usageTracker.AcquireResourceUsage(); - return {}; } - MaybeError ValidateRenderPass(CommandIterator* commands, - BeginRenderPassCmd* renderPass, - std::vector* perPassResourceUsages) { - PassResourceUsageTracker usageTracker; + MaybeError ValidateRenderPass(CommandIterator* commands, const BeginRenderPassCmd* renderPass) { CommandBufferStateTracker commandBufferState; uint64_t debugGroupStackSize = 0; - // Track usage of the render pass attachments - for (uint32_t i : IterateBitSet(renderPass->attachmentState->GetColorAttachmentsMask())) { - RenderPassColorAttachmentInfo* colorAttachment = &renderPass->colorAttachments[i]; - TextureBase* texture = colorAttachment->view->GetTexture(); - usageTracker.TextureUsedAs(texture, wgpu::TextureUsage::OutputAttachment); - - TextureViewBase* resolveTarget = colorAttachment->resolveTarget.Get(); - if (resolveTarget != nullptr) { - usageTracker.TextureUsedAs(resolveTarget->GetTexture(), - wgpu::TextureUsage::OutputAttachment); - } - } - - if (renderPass->attachmentState->HasDepthStencilAttachment()) { - TextureBase* texture = renderPass->depthStencilAttachment.view->GetTexture(); - usageTracker.TextureUsedAs(texture, wgpu::TextureUsage::OutputAttachment); - } - Command type; while (commands->NextCommandId(&type)) { switch (type) { case Command::EndRenderPass: { commands->NextCommand(); - DAWN_TRY(ValidateFinalDebugGroupStackSize(debugGroupStackSize)); - DAWN_TRY(usageTracker.ValidateRenderPassUsages()); - ASSERT(perPassResourceUsages != nullptr); - perPassResourceUsages->push_back(usageTracker.AcquireResourceUsage()); - return {}; } break; @@ -241,15 +161,6 @@ namespace dawn_native { return DAWN_VALIDATION_ERROR( "Render bundle is not compatible with render pass"); } - - const PassResourceUsage& usages = bundles[i]->GetResourceUsage(); - for (uint32_t i = 0; i < usages.buffers.size(); ++i) { - usageTracker.BufferUsedAs(usages.buffers[i], usages.bufferUsages[i]); - } - - for (uint32_t i = 0; i < usages.textures.size(); ++i) { - usageTracker.TextureUsedAs(usages.textures[i], usages.textureUsages[i]); - } } if (cmd->count > 0) { @@ -277,9 +188,8 @@ namespace dawn_native { default: DAWN_TRY(ValidateRenderBundleCommand( - commands, type, &usageTracker, &commandBufferState, - renderPass->attachmentState.Get(), &debugGroupStackSize, - "Command disallowed inside a render pass")); + commands, type, &commandBufferState, renderPass->attachmentState.Get(), + &debugGroupStackSize, "Command disallowed inside a render pass")); } } @@ -287,9 +197,7 @@ namespace dawn_native { return DAWN_VALIDATION_ERROR("Unfinished render pass"); } - MaybeError ValidateComputePass(CommandIterator* commands, - std::vector* perPassResourceUsages) { - PassResourceUsageTracker usageTracker; + MaybeError ValidateComputePass(CommandIterator* commands) { CommandBufferStateTracker commandBufferState; uint64_t debugGroupStackSize = 0; @@ -298,11 +206,7 @@ namespace dawn_native { switch (type) { case Command::EndComputePass: { commands->NextCommand(); - DAWN_TRY(ValidateFinalDebugGroupStackSize(debugGroupStackSize)); - DAWN_TRY(usageTracker.ValidateComputePassUsages()); - ASSERT(perPassResourceUsages != nullptr); - perPassResourceUsages->push_back(usageTracker.AcquireResourceUsage()); return {}; } break; @@ -312,10 +216,8 @@ namespace dawn_native { } break; case Command::DispatchIndirect: { - DispatchIndirectCmd* cmd = commands->NextCommand(); + commands->NextCommand(); DAWN_TRY(commandBufferState.ValidateCanDispatch()); - usageTracker.BufferUsedAs(cmd->indirectBuffer.Get(), - wgpu::BufferUsage::Indirect); } break; case Command::InsertDebugMarker: { @@ -346,8 +248,6 @@ namespace dawn_native { if (cmd->dynamicOffsetCount > 0) { commands->NextData(cmd->dynamicOffsetCount); } - - TrackBindGroupResourceUsage(cmd->group.Get(), &usageTracker); commandBufferState.SetBindGroup(cmd->index, cmd->group.Get()); } break; @@ -360,4 +260,46 @@ namespace dawn_native { 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. + MaybeError ValidatePassResourceUsage(const PassResourceUsage& pass) { + // Buffers can only be used as single-write or multiple read. + for (size_t i = 0; i < pass.buffers.size(); ++i) { + const BufferBase* buffer = pass.buffers[i]; + wgpu::BufferUsage usage = pass.bufferUsages[i]; + + if (usage & ~buffer->GetUsage()) { + return DAWN_VALIDATION_ERROR("Buffer missing usage for the pass"); + } + + bool readOnly = (usage & kReadOnlyBufferUsages) == usage; + bool singleUse = wgpu::HasZeroOrOneBits(usage); + + if (!readOnly && !singleUse) { + return DAWN_VALIDATION_ERROR( + "Buffer used as writable usage and another usage in pass"); + } + } + + // Textures can only be used as single-write or multiple read. + // TODO(cwallez@chromium.org): implement per-subresource tracking + for (size_t i = 0; i < pass.textures.size(); ++i) { + const TextureBase* texture = pass.textures[i]; + wgpu::TextureUsage usage = pass.textureUsages[i]; + + if (usage & ~texture->GetUsage()) { + return DAWN_VALIDATION_ERROR("Texture missing usage for the pass"); + } + + // For textures the only read-only usage in a pass is Sampled, so checking the + // usage constraint simplifies to checking a single usage bit is set. + if (!wgpu::HasZeroOrOneBits(usage)) { + return DAWN_VALIDATION_ERROR("Texture used with more than one usage in pass"); + } + } + + return {}; + } + } // namespace dawn_native diff --git a/src/dawn_native/CommandValidation.h b/src/dawn_native/CommandValidation.h index b5a14934ab..d649ce32ee 100644 --- a/src/dawn_native/CommandValidation.h +++ b/src/dawn_native/CommandValidation.h @@ -30,13 +30,11 @@ namespace dawn_native { MaybeError ValidateFinalDebugGroupStackSize(uint64_t debugGroupStackSize); MaybeError ValidateRenderBundle(CommandIterator* commands, - const AttachmentState* attachmentState, - PassResourceUsage* resourceUsage); - MaybeError ValidateRenderPass(CommandIterator* commands, - BeginRenderPassCmd* renderPass, - std::vector* perPassResourceUsages); - MaybeError ValidateComputePass(CommandIterator* commands, - std::vector* perPassResourceUsages); + const AttachmentState* attachmentState); + MaybeError ValidateRenderPass(CommandIterator* commands, const BeginRenderPassCmd* renderPass); + MaybeError ValidateComputePass(CommandIterator* commands); + + MaybeError ValidatePassResourceUsage(const PassResourceUsage& usage); } // namespace dawn_native diff --git a/src/dawn_native/ComputePassEncoder.cpp b/src/dawn_native/ComputePassEncoder.cpp index 3c0f442a68..cd88c875a1 100644 --- a/src/dawn_native/ComputePassEncoder.cpp +++ b/src/dawn_native/ComputePassEncoder.cpp @@ -48,7 +48,7 @@ namespace dawn_native { return {}; })) { - mEncodingContext->ExitPass(this); + mEncodingContext->ExitPass(this, mUsageTracker.AcquireResourceUsage()); } } @@ -77,6 +77,8 @@ namespace dawn_native { dispatch->indirectBuffer = indirectBuffer; dispatch->indirectOffset = indirectOffset; + mUsageTracker.BufferUsedAs(indirectBuffer, wgpu::BufferUsage::Indirect); + return {}; }); } diff --git a/src/dawn_native/EncodingContext.cpp b/src/dawn_native/EncodingContext.cpp index ea156a605b..b8be06990d 100644 --- a/src/dawn_native/EncodingContext.cpp +++ b/src/dawn_native/EncodingContext.cpp @@ -15,9 +15,11 @@ #include "dawn_native/EncodingContext.h" #include "common/Assert.h" +#include "dawn_native/CommandEncoder.h" #include "dawn_native/Commands.h" #include "dawn_native/Device.h" #include "dawn_native/ErrorData.h" +#include "dawn_native/RenderBundleEncoder.h" namespace dawn_native { @@ -32,17 +34,23 @@ namespace dawn_native { } CommandIterator EncodingContext::AcquireCommands() { + MoveToIterator(); ASSERT(!mWereCommandsAcquired); mWereCommandsAcquired = true; return std::move(mIterator); } CommandIterator* EncodingContext::GetIterator() { + MoveToIterator(); + ASSERT(!mWereCommandsAcquired); + return &mIterator; + } + + void EncodingContext::MoveToIterator() { if (!mWasMovedToIterator) { mIterator = std::move(mAllocator); mWasMovedToIterator = true; } - return &mIterator; } void EncodingContext::HandleError(wgpu::ErrorType type, const char* message) { @@ -66,13 +74,25 @@ namespace dawn_native { mCurrentEncoder = passEncoder; } - void EncodingContext::ExitPass(const ObjectBase* passEncoder) { + void EncodingContext::ExitPass(const ObjectBase* passEncoder, PassResourceUsage passUsage) { // Assert we're not at the top level. ASSERT(mCurrentEncoder != mTopLevelEncoder); // Assert the pass encoder is current. ASSERT(mCurrentEncoder == passEncoder); mCurrentEncoder = mTopLevelEncoder; + mPassUsages.push_back(std::move(passUsage)); + } + + const PerPassUsages& EncodingContext::GetPassUsages() const { + ASSERT(!mWerePassUsagesAcquired); + return mPassUsages; + } + + PerPassUsages EncodingContext::AcquirePassUsages() { + ASSERT(!mWerePassUsagesAcquired); + mWerePassUsagesAcquired = true; + return std::move(mPassUsages); } MaybeError EncodingContext::Finish() { diff --git a/src/dawn_native/EncodingContext.h b/src/dawn_native/EncodingContext.h index ecb0505745..c16d544c0f 100644 --- a/src/dawn_native/EncodingContext.h +++ b/src/dawn_native/EncodingContext.h @@ -18,14 +18,15 @@ #include "dawn_native/CommandAllocator.h" #include "dawn_native/Error.h" #include "dawn_native/ErrorData.h" +#include "dawn_native/PassResourceUsageTracker.h" #include "dawn_native/dawn_platform.h" #include namespace dawn_native { - class ObjectBase; class DeviceBase; + class ObjectBase; // Base class for allocating/iterating commands. // It performs error tracking as well as encoding state for render/compute passes. @@ -54,7 +55,7 @@ namespace dawn_native { } template - inline bool TryEncode(const void* encoder, EncodeFunction&& encodeFunction) { + inline bool TryEncode(const ObjectBase* encoder, EncodeFunction&& encodeFunction) { if (DAWN_UNLIKELY(encoder != mCurrentEncoder)) { if (mCurrentEncoder != mTopLevelEncoder) { // The top level encoder was used when a pass encoder was current. @@ -72,11 +73,15 @@ namespace dawn_native { // Functions to set current encoder state void EnterPass(const ObjectBase* passEncoder); - void ExitPass(const ObjectBase* passEncoder); + void ExitPass(const ObjectBase* passEncoder, PassResourceUsage passUsages); MaybeError Finish(); + const PerPassUsages& GetPassUsages() const; + PerPassUsages AcquirePassUsages(); + private: bool IsFinished() const; + void MoveToIterator(); DeviceBase* mDevice; @@ -90,6 +95,9 @@ namespace dawn_native { // CommandEncoder::Begin/EndPass. const ObjectBase* mCurrentEncoder; + PerPassUsages mPassUsages; + bool mWerePassUsagesAcquired = false; + CommandAllocator mAllocator; CommandIterator mIterator; bool mWasMovedToIterator = false; diff --git a/src/dawn_native/PassResourceUsage.h b/src/dawn_native/PassResourceUsage.h index 226ad3167d..c2a2071737 100644 --- a/src/dawn_native/PassResourceUsage.h +++ b/src/dawn_native/PassResourceUsage.h @@ -36,8 +36,10 @@ namespace dawn_native { std::vector textureUsages; }; + using PerPassUsages = std::vector; + struct CommandBufferResourceUsage { - std::vector perPass; + PerPassUsages perPass; std::set topLevelBuffers; std::set topLevelTextures; }; diff --git a/src/dawn_native/PassResourceUsageTracker.cpp b/src/dawn_native/PassResourceUsageTracker.cpp index 18980b8460..4831b36989 100644 --- a/src/dawn_native/PassResourceUsageTracker.cpp +++ b/src/dawn_native/PassResourceUsageTracker.cpp @@ -31,54 +31,6 @@ namespace dawn_native { mTextureUsages[texture] |= usage; } - MaybeError PassResourceUsageTracker::ValidateComputePassUsages() const { - return ValidateUsages(); - } - - MaybeError PassResourceUsageTracker::ValidateRenderPassUsages() const { - return ValidateUsages(); - } - - // Performs the per-pass usage validation checks - MaybeError PassResourceUsageTracker::ValidateUsages() const { - // Buffers can only be used as single-write or multiple read. - for (auto& it : mBufferUsages) { - BufferBase* buffer = it.first; - wgpu::BufferUsage usage = it.second; - - if (usage & ~buffer->GetUsage()) { - return DAWN_VALIDATION_ERROR("Buffer missing usage for the pass"); - } - - bool readOnly = (usage & kReadOnlyBufferUsages) == usage; - bool singleUse = wgpu::HasZeroOrOneBits(usage); - - if (!readOnly && !singleUse) { - return DAWN_VALIDATION_ERROR( - "Buffer used as writable usage and another usage in pass"); - } - } - - // Textures can only be used as single-write or multiple read. - // TODO(cwallez@chromium.org): implement per-subresource tracking - for (auto& it : mTextureUsages) { - TextureBase* texture = it.first; - wgpu::TextureUsage usage = it.second; - - if (usage & ~texture->GetUsage()) { - return DAWN_VALIDATION_ERROR("Texture missing usage for the pass"); - } - - // For textures the only read-only usage in a pass is Sampled, so checking the - // usage constraint simplifies to checking a single usage bit is set. - if (!wgpu::HasZeroOrOneBits(it.second)) { - return DAWN_VALIDATION_ERROR("Texture used with more than one usage in pass"); - } - } - - return {}; - } - // Returns the per-pass usage for use by backends for APIs with explicit barriers. PassResourceUsage PassResourceUsageTracker::AcquireResourceUsage() { PassResourceUsage result; @@ -97,6 +49,9 @@ namespace dawn_native { result.textureUsages.push_back(it.second); } + mBufferUsages.clear(); + mTextureUsages.clear(); + return result; } diff --git a/src/dawn_native/PassResourceUsageTracker.h b/src/dawn_native/PassResourceUsageTracker.h index ff168fb58c..458b175707 100644 --- a/src/dawn_native/PassResourceUsageTracker.h +++ b/src/dawn_native/PassResourceUsageTracker.h @@ -15,7 +15,6 @@ #ifndef DAWNNATIVE_PASSRESOURCEUSAGETRACKER_H_ #define DAWNNATIVE_PASSRESOURCEUSAGETRACKER_H_ -#include "dawn_native/Error.h" #include "dawn_native/PassResourceUsage.h" #include "dawn_native/dawn_platform.h" @@ -36,16 +35,10 @@ namespace dawn_native { void BufferUsedAs(BufferBase* buffer, wgpu::BufferUsage usage); void TextureUsedAs(TextureBase* texture, wgpu::TextureUsage usage); - MaybeError ValidateComputePassUsages() const; - MaybeError ValidateRenderPassUsages() const; - // Returns the per-pass usage for use by backends for APIs with explicit barriers. PassResourceUsage AcquireResourceUsage(); private: - // Performs the per-pass usage validation checks - MaybeError ValidateUsages() const; - std::map mBufferUsages; std::map mTextureUsages; }; diff --git a/src/dawn_native/ProgrammablePassEncoder.cpp b/src/dawn_native/ProgrammablePassEncoder.cpp index 8013ccb393..bedf0c4417 100644 --- a/src/dawn_native/ProgrammablePassEncoder.cpp +++ b/src/dawn_native/ProgrammablePassEncoder.cpp @@ -14,6 +14,7 @@ #include "dawn_native/ProgrammablePassEncoder.h" +#include "common/BitSetIterator.h" #include "dawn_native/BindGroup.h" #include "dawn_native/Buffer.h" #include "dawn_native/CommandBuffer.h" @@ -25,6 +26,46 @@ namespace dawn_native { + namespace { + void TrackBindGroupResourceUsage(PassResourceUsageTracker* usageTracker, + BindGroupBase* group) { + const auto& layoutInfo = group->GetLayout()->GetBindingInfo(); + + for (uint32_t i : IterateBitSet(layoutInfo.mask)) { + wgpu::BindingType type = layoutInfo.types[i]; + + switch (type) { + case wgpu::BindingType::UniformBuffer: { + BufferBase* buffer = group->GetBindingAsBufferBinding(i).buffer; + usageTracker->BufferUsedAs(buffer, wgpu::BufferUsage::Uniform); + } break; + + case wgpu::BindingType::StorageBuffer: { + BufferBase* buffer = group->GetBindingAsBufferBinding(i).buffer; + usageTracker->BufferUsedAs(buffer, wgpu::BufferUsage::Storage); + } break; + + case wgpu::BindingType::SampledTexture: { + TextureBase* texture = group->GetBindingAsTextureView(i)->GetTexture(); + usageTracker->TextureUsedAs(texture, wgpu::TextureUsage::Sampled); + } break; + + case wgpu::BindingType::ReadonlyStorageBuffer: { + BufferBase* buffer = group->GetBindingAsBufferBinding(i).buffer; + usageTracker->BufferUsedAs(buffer, kReadOnlyStorage); + } break; + + case wgpu::BindingType::Sampler: + break; + + case wgpu::BindingType::StorageTexture: + UNREACHABLE(); + break; + } + } + } + } // namespace + ProgrammablePassEncoder::ProgrammablePassEncoder(DeviceBase* device, EncodingContext* encodingContext) : ObjectBase(device), mEncodingContext(encodingContext) { @@ -75,34 +116,36 @@ namespace dawn_native { uint32_t dynamicOffsetCount, const uint32_t* dynamicOffsets) { mEncodingContext->TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError { - DAWN_TRY(GetDevice()->ValidateObject(group)); + if (GetDevice()->IsValidationEnabled()) { + DAWN_TRY(GetDevice()->ValidateObject(group)); - if (groupIndex >= kMaxBindGroups) { - return DAWN_VALIDATION_ERROR("Setting bind group over the max"); - } - - // Dynamic offsets count must match the number required by the layout perfectly. - const BindGroupLayoutBase* layout = group->GetLayout(); - if (layout->GetDynamicBufferCount() != dynamicOffsetCount) { - return DAWN_VALIDATION_ERROR("dynamicOffset count mismatch"); - } - - for (uint32_t i = 0; i < dynamicOffsetCount; ++i) { - if (dynamicOffsets[i] % kMinDynamicBufferOffsetAlignment != 0) { - return DAWN_VALIDATION_ERROR("Dynamic Buffer Offset need to be aligned"); + if (groupIndex >= kMaxBindGroups) { + return DAWN_VALIDATION_ERROR("Setting bind group over the max"); } - BufferBinding bufferBinding = group->GetBindingAsBufferBinding(i); + // Dynamic offsets count must match the number required by the layout perfectly. + const BindGroupLayoutBase* layout = group->GetLayout(); + if (layout->GetDynamicBufferCount() != dynamicOffsetCount) { + return DAWN_VALIDATION_ERROR("dynamicOffset count mismatch"); + } - // During BindGroup creation, validation ensures binding offset + binding size <= - // buffer size. - DAWN_ASSERT(bufferBinding.buffer->GetSize() >= bufferBinding.size); - DAWN_ASSERT(bufferBinding.buffer->GetSize() - bufferBinding.size >= - bufferBinding.offset); + for (uint32_t i = 0; i < dynamicOffsetCount; ++i) { + if (dynamicOffsets[i] % kMinDynamicBufferOffsetAlignment != 0) { + return DAWN_VALIDATION_ERROR("Dynamic Buffer Offset need to be aligned"); + } - if ((dynamicOffsets[i] > - bufferBinding.buffer->GetSize() - bufferBinding.offset - bufferBinding.size)) { - return DAWN_VALIDATION_ERROR("dynamic offset out of bounds"); + BufferBinding bufferBinding = group->GetBindingAsBufferBinding(i); + + // During BindGroup creation, validation ensures binding offset + binding size + // <= buffer size. + DAWN_ASSERT(bufferBinding.buffer->GetSize() >= bufferBinding.size); + DAWN_ASSERT(bufferBinding.buffer->GetSize() - bufferBinding.size >= + bufferBinding.offset); + + if ((dynamicOffsets[i] > bufferBinding.buffer->GetSize() - + bufferBinding.offset - bufferBinding.size)) { + return DAWN_VALIDATION_ERROR("dynamic offset out of bounds"); + } } } @@ -115,6 +158,8 @@ namespace dawn_native { memcpy(offsets, dynamicOffsets, dynamicOffsetCount * sizeof(uint32_t)); } + TrackBindGroupResourceUsage(&mUsageTracker, group); + return {}; }); } diff --git a/src/dawn_native/ProgrammablePassEncoder.h b/src/dawn_native/ProgrammablePassEncoder.h index 2508059be9..17bfeb413b 100644 --- a/src/dawn_native/ProgrammablePassEncoder.h +++ b/src/dawn_native/ProgrammablePassEncoder.h @@ -18,6 +18,7 @@ #include "dawn_native/CommandEncoder.h" #include "dawn_native/Error.h" #include "dawn_native/ObjectBase.h" +#include "dawn_native/PassResourceUsageTracker.h" #include "dawn_native/dawn_platform.h" @@ -47,6 +48,7 @@ namespace dawn_native { ErrorTag errorTag); EncodingContext* mEncodingContext = nullptr; + PassResourceUsageTracker mUsageTracker; }; } // namespace dawn_native diff --git a/src/dawn_native/Queue.cpp b/src/dawn_native/Queue.cpp index 41d16e9a50..0fbcdc7d05 100644 --- a/src/dawn_native/Queue.cpp +++ b/src/dawn_native/Queue.cpp @@ -35,7 +35,8 @@ namespace dawn_native { void QueueBase::Submit(uint32_t commandCount, CommandBufferBase* const* commands) { DeviceBase* device = GetDevice(); TRACE_EVENT0(device->GetPlatform(), General, "Queue::Submit"); - if (device->ConsumedError(ValidateSubmit(commandCount, commands))) { + if (device->IsValidationEnabled() && + device->ConsumedError(ValidateSubmit(commandCount, commands))) { return; } ASSERT(!IsError()); diff --git a/src/dawn_native/RenderBundleEncoder.cpp b/src/dawn_native/RenderBundleEncoder.cpp index 7d89876d60..956fa8b3a2 100644 --- a/src/dawn_native/RenderBundleEncoder.cpp +++ b/src/dawn_native/RenderBundleEncoder.cpp @@ -102,26 +102,27 @@ namespace dawn_native { } RenderBundleBase* RenderBundleEncoder::Finish(const RenderBundleDescriptor* descriptor) { - if (GetDevice()->ConsumedError(ValidateFinish(descriptor))) { - return RenderBundleBase::MakeError(GetDevice()); - } - ASSERT(!IsError()); + PassResourceUsage usages = mUsageTracker.AcquireResourceUsage(); - return new RenderBundleBase(this, descriptor, mAttachmentState.Get(), - std::move(mResourceUsage)); + DeviceBase* device = GetDevice(); + // Even if mEncodingContext.Finish() validation fails, calling it will mutate the internal + // state of the encoding context. Subsequent calls to encode commands will generate errors. + if (device->ConsumedError(mEncodingContext.Finish()) || + (device->IsValidationEnabled() && + device->ConsumedError(ValidateFinish(mEncodingContext.GetIterator(), usages)))) { + return RenderBundleBase::MakeError(device); + } + + ASSERT(!IsError()); + return new RenderBundleBase(this, descriptor, mAttachmentState.Get(), std::move(usages)); } - MaybeError RenderBundleEncoder::ValidateFinish(const RenderBundleDescriptor* descriptor) { + MaybeError RenderBundleEncoder::ValidateFinish(CommandIterator* commands, + const PassResourceUsage& usages) const { TRACE_EVENT0(GetDevice()->GetPlatform(), Validation, "RenderBundleEncoder::ValidateFinish"); DAWN_TRY(GetDevice()->ValidateObject(this)); - - // Even if Finish() validation fails, calling it will mutate the internal state of the - // encoding context. Subsequent calls to encode commands will generate errors. - DAWN_TRY(mEncodingContext.Finish()); - - CommandIterator* commands = mEncodingContext.GetIterator(); - - DAWN_TRY(ValidateRenderBundle(commands, mAttachmentState.Get(), &mResourceUsage)); + DAWN_TRY(ValidatePassResourceUsage(usages)); + DAWN_TRY(ValidateRenderBundle(commands, mAttachmentState.Get())); return {}; } diff --git a/src/dawn_native/RenderBundleEncoder.h b/src/dawn_native/RenderBundleEncoder.h index f3919b0434..0581719f1d 100644 --- a/src/dawn_native/RenderBundleEncoder.h +++ b/src/dawn_native/RenderBundleEncoder.h @@ -42,11 +42,10 @@ namespace dawn_native { private: RenderBundleEncoder(DeviceBase* device, ErrorTag errorTag); - MaybeError ValidateFinish(const RenderBundleDescriptor* descriptor); + MaybeError ValidateFinish(CommandIterator* commands, const PassResourceUsage& usages) const; EncodingContext mEncodingContext; Ref mAttachmentState; - PassResourceUsage mResourceUsage; }; } // namespace dawn_native diff --git a/src/dawn_native/RenderEncoderBase.cpp b/src/dawn_native/RenderEncoderBase.cpp index aecaf30afa..885f7a1180 100644 --- a/src/dawn_native/RenderEncoderBase.cpp +++ b/src/dawn_native/RenderEncoderBase.cpp @@ -81,6 +81,8 @@ namespace dawn_native { cmd->indirectBuffer = indirectBuffer; cmd->indirectOffset = indirectOffset; + mUsageTracker.BufferUsedAs(indirectBuffer, wgpu::BufferUsage::Indirect); + return {}; }); } @@ -100,6 +102,8 @@ namespace dawn_native { cmd->indirectBuffer = indirectBuffer; cmd->indirectOffset = indirectOffset; + mUsageTracker.BufferUsedAs(indirectBuffer, wgpu::BufferUsage::Indirect); + return {}; }); } @@ -125,6 +129,8 @@ namespace dawn_native { cmd->buffer = buffer; cmd->offset = offset; + mUsageTracker.BufferUsedAs(buffer, wgpu::BufferUsage::Index); + return {}; }); } @@ -139,6 +145,8 @@ namespace dawn_native { cmd->buffer = buffer; cmd->offset = offset; + mUsageTracker.BufferUsedAs(buffer, wgpu::BufferUsage::Vertex); + return {}; }); } diff --git a/src/dawn_native/RenderPassEncoder.cpp b/src/dawn_native/RenderPassEncoder.cpp index 5e74acbd51..9dbb2ee87c 100644 --- a/src/dawn_native/RenderPassEncoder.cpp +++ b/src/dawn_native/RenderPassEncoder.cpp @@ -27,10 +27,15 @@ namespace dawn_native { + // The usage tracker is passed in here, because it is prepopulated with usages from the + // BeginRenderPassCmd. If we had RenderPassEncoder responsible for recording the + // command, then this wouldn't be necessary. RenderPassEncoder::RenderPassEncoder(DeviceBase* device, CommandEncoder* commandEncoder, - EncodingContext* encodingContext) + EncodingContext* encodingContext, + PassResourceUsageTracker usageTracker) : RenderEncoderBase(device, encodingContext), mCommandEncoder(commandEncoder) { + mUsageTracker = std::move(usageTracker); } RenderPassEncoder::RenderPassEncoder(DeviceBase* device, @@ -52,7 +57,7 @@ namespace dawn_native { return {}; })) { - mEncodingContext->ExitPass(this); + mEncodingContext->ExitPass(this, mUsageTracker.AcquireResourceUsage()); } } @@ -143,6 +148,14 @@ namespace dawn_native { Ref* bundles = allocator->AllocateData>(count); for (uint32_t i = 0; i < count; ++i) { bundles[i] = renderBundles[i]; + + const PassResourceUsage& usages = bundles[i]->GetResourceUsage(); + for (uint32_t i = 0; i < usages.buffers.size(); ++i) { + mUsageTracker.BufferUsedAs(usages.buffers[i], usages.bufferUsages[i]); + } + for (uint32_t i = 0; i < usages.textures.size(); ++i) { + mUsageTracker.TextureUsedAs(usages.textures[i], usages.textureUsages[i]); + } } return {}; diff --git a/src/dawn_native/RenderPassEncoder.h b/src/dawn_native/RenderPassEncoder.h index 12b9342094..cd9ac017fb 100644 --- a/src/dawn_native/RenderPassEncoder.h +++ b/src/dawn_native/RenderPassEncoder.h @@ -26,7 +26,8 @@ namespace dawn_native { public: RenderPassEncoder(DeviceBase* device, CommandEncoder* commandEncoder, - EncodingContext* encodingContext); + EncodingContext* encodingContext, + PassResourceUsageTracker usageTracker); static RenderPassEncoder* MakeError(DeviceBase* device, CommandEncoder* commandEncoder, diff --git a/src/tests/perf_tests/DrawCallPerf.cpp b/src/tests/perf_tests/DrawCallPerf.cpp index e21c9ae749..662a3be227 100644 --- a/src/tests/perf_tests/DrawCallPerf.cpp +++ b/src/tests/perf_tests/DrawCallPerf.cpp @@ -605,7 +605,8 @@ TEST_P(DrawCallPerf, Run) { DAWN_INSTANTIATE_PERF_TEST_SUITE_P( DrawCallPerf, - {D3D12Backend, MetalBackend, OpenGLBackend, VulkanBackend}, + {D3D12Backend, MetalBackend, OpenGLBackend, VulkanBackend, + ForceWorkarounds(VulkanBackend, {"skip_validation"})}, { // Baseline MakeParam(),