From 575729e8dd25e0ac4631c9739a82118ea3545c6b Mon Sep 17 00:00:00 2001 From: Hao Li Date: Mon, 16 Nov 2020 02:24:06 +0000 Subject: [PATCH] Add query availability tracking in render pass encoder The same query cannot be written twice in same render pass, so each render pass also need to have its own query availability map. Update timestamp query to only check the same query overwrite in same render pass. Bug: dawn:434 Change-Id: Icb070adf79a3d76c25367675f7432666eb0dd84f Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/31180 Reviewed-by: Corentin Wallez Commit-Queue: Hao Li --- src/dawn_native/CommandEncoder.cpp | 30 +++++---- src/dawn_native/CommandEncoder.h | 8 +-- src/dawn_native/CommandValidation.cpp | 15 +---- src/dawn_native/CommandValidation.h | 7 +-- src/dawn_native/ComputePassEncoder.cpp | 6 +- src/dawn_native/RenderPassEncoder.cpp | 40 ++++++++++-- src/dawn_native/RenderPassEncoder.h | 8 +++ .../validation/QuerySetValidationTests.cpp | 63 +++++++------------ 8 files changed, 91 insertions(+), 86 deletions(-) diff --git a/src/dawn_native/CommandEncoder.cpp b/src/dawn_native/CommandEncoder.cpp index 7b4c8584d7..e327e5e542 100644 --- a/src/dawn_native/CommandEncoder.cpp +++ b/src/dawn_native/CommandEncoder.cpp @@ -398,22 +398,21 @@ namespace dawn_native { mUsedQuerySets.insert(querySet); } - void CommandEncoder::TrackUsedQueryIndex(QuerySetBase* querySet, uint32_t queryIndex) { - UsedQueryMap::iterator it = mUsedQueryIndices.find(querySet); - if (it != mUsedQueryIndices.end()) { - // Record index on existing query set - std::vector& queryIndices = it->second; - queryIndices[queryIndex] = 1; - } else { - // Record index on new query set - std::vector queryIndices(querySet->GetQueryCount(), 0); - queryIndices[queryIndex] = 1; - mUsedQueryIndices.insert({querySet, std::move(queryIndices)}); + void CommandEncoder::TrackQueryAvailability(QuerySetBase* querySet, uint32_t queryIndex) { + DAWN_ASSERT(querySet != nullptr); + + if (GetDevice()->IsValidationEnabled()) { + TrackUsedQuerySet(querySet); } + + // Gets the iterator for that querySet or create a new vector of bool set to false + // if the querySet wasn't registered. + auto it = mQueryAvailabilityMap.emplace(querySet, querySet->GetQueryCount()).first; + it->second[queryIndex] = 1; } - const UsedQueryMap& CommandEncoder::GetUsedQueryIndices() const { - return mUsedQueryIndices; + const QueryAvailabilityMap& CommandEncoder::GetQueryAvailabilityMap() const { + return mQueryAvailabilityMap; } // Implementation of the API's command recording methods @@ -787,11 +786,10 @@ namespace dawn_native { mEncodingContext.TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError { if (GetDevice()->IsValidationEnabled()) { DAWN_TRY(GetDevice()->ValidateObject(querySet)); - DAWN_TRY(ValidateTimestampQuery(querySet, queryIndex, GetUsedQueryIndices())); - TrackUsedQuerySet(querySet); + DAWN_TRY(ValidateTimestampQuery(querySet, queryIndex)); } - TrackUsedQueryIndex(querySet, queryIndex); + TrackQueryAvailability(querySet, queryIndex); WriteTimestampCmd* cmd = allocator->Allocate(Command::WriteTimestamp); diff --git a/src/dawn_native/CommandEncoder.h b/src/dawn_native/CommandEncoder.h index 9b992b618e..cb02521f89 100644 --- a/src/dawn_native/CommandEncoder.h +++ b/src/dawn_native/CommandEncoder.h @@ -29,7 +29,7 @@ namespace dawn_native { struct BeginRenderPassCmd; - using UsedQueryMap = std::map>; + using QueryAvailabilityMap = std::map>; class CommandEncoder final : public ObjectBase { public: @@ -39,8 +39,8 @@ namespace dawn_native { CommandBufferResourceUsage AcquireResourceUsages(); void TrackUsedQuerySet(QuerySetBase* querySet); - void TrackUsedQueryIndex(QuerySetBase* querySet, uint32_t queryIndex); - const UsedQueryMap& GetUsedQueryIndices() const; + void TrackQueryAvailability(QuerySetBase* querySet, uint32_t queryIndex); + const QueryAvailabilityMap& GetQueryAvailabilityMap() const; // Dawn API ComputePassEncoder* BeginComputePass(const ComputePassDescriptor* descriptor); @@ -83,7 +83,7 @@ namespace dawn_native { std::set mTopLevelBuffers; std::set mTopLevelTextures; std::set mUsedQuerySets; - UsedQueryMap mUsedQueryIndices; + QueryAvailabilityMap mQueryAvailabilityMap; }; } // namespace dawn_native diff --git a/src/dawn_native/CommandValidation.cpp b/src/dawn_native/CommandValidation.cpp index 0cbfdcb5c4..354a113ee9 100644 --- a/src/dawn_native/CommandValidation.cpp +++ b/src/dawn_native/CommandValidation.cpp @@ -351,26 +351,15 @@ namespace dawn_native { return {}; } - MaybeError ValidateTimestampQuery(QuerySetBase* querySet, - uint32_t queryIndex, - const UsedQueryMap& usedQueryIndices) { + MaybeError ValidateTimestampQuery(QuerySetBase* querySet, uint32_t queryIndex) { if (querySet->GetQueryType() != wgpu::QueryType::Timestamp) { - return DAWN_VALIDATION_ERROR("The query type of query set must be Timestamp"); + return DAWN_VALIDATION_ERROR("The type of query set must be Timestamp"); } if (queryIndex >= querySet->GetQueryCount()) { return DAWN_VALIDATION_ERROR("Query index exceeds the number of queries in query set"); } - UsedQueryMap::const_iterator it = usedQueryIndices.find(querySet); - if (it != usedQueryIndices.end()) { - // Get the used query index records - const std::vector& queryIndices = it->second; - if (queryIndices[queryIndex] == 1) { - return DAWN_VALIDATION_ERROR("Duplicated query index writen"); - } - } - return {}; } diff --git a/src/dawn_native/CommandValidation.h b/src/dawn_native/CommandValidation.h index 3e46774c2f..d2794e21cd 100644 --- a/src/dawn_native/CommandValidation.h +++ b/src/dawn_native/CommandValidation.h @@ -19,7 +19,6 @@ #include "dawn_native/Error.h" #include "dawn_native/Texture.h" -#include #include namespace dawn_native { @@ -30,8 +29,6 @@ namespace dawn_native { struct PassResourceUsage; struct TexelBlockInfo; - using UsedQueryMap = std::map>; - MaybeError ValidateCanPopDebugGroup(uint64_t debugGroupStackSize); MaybeError ValidateFinalDebugGroupStackSize(uint64_t debugGroupStackSize); @@ -42,9 +39,7 @@ namespace dawn_native { MaybeError ValidatePassResourceUsage(const PassResourceUsage& usage); - MaybeError ValidateTimestampQuery(QuerySetBase* querySet, - uint32_t queryIndex, - const UsedQueryMap& usedQueryIndices); + MaybeError ValidateTimestampQuery(QuerySetBase* querySet, uint32_t queryIndex); ResultOrError ComputeRequiredBytesInCopy(const TexelBlockInfo& blockInfo, const Extent3D& copySize, diff --git a/src/dawn_native/ComputePassEncoder.cpp b/src/dawn_native/ComputePassEncoder.cpp index e421ea4aad..afa9df0a38 100644 --- a/src/dawn_native/ComputePassEncoder.cpp +++ b/src/dawn_native/ComputePassEncoder.cpp @@ -114,12 +114,10 @@ namespace dawn_native { mEncodingContext->TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError { if (GetDevice()->IsValidationEnabled()) { DAWN_TRY(GetDevice()->ValidateObject(querySet)); - DAWN_TRY(ValidateTimestampQuery(querySet, queryIndex, - mCommandEncoder->GetUsedQueryIndices())); - mCommandEncoder->TrackUsedQuerySet(querySet); + DAWN_TRY(ValidateTimestampQuery(querySet, queryIndex)); } - mCommandEncoder->TrackUsedQueryIndex(querySet, queryIndex); + mCommandEncoder->TrackQueryAvailability(querySet, queryIndex); WriteTimestampCmd* cmd = allocator->Allocate(Command::WriteTimestamp); diff --git a/src/dawn_native/RenderPassEncoder.cpp b/src/dawn_native/RenderPassEncoder.cpp index df995633c2..72dc079f44 100644 --- a/src/dawn_native/RenderPassEncoder.cpp +++ b/src/dawn_native/RenderPassEncoder.cpp @@ -28,6 +28,22 @@ #include namespace dawn_native { + namespace { + + // Check the query at queryIndex is unavailable, otherwise it cannot be written. + MaybeError ValidateQueryIndexOverwrite(QuerySetBase* querySet, + uint32_t queryIndex, + const QueryAvailabilityMap& queryAvailabilityMap) { + auto it = queryAvailabilityMap.find(querySet); + if (it != queryAvailabilityMap.end() && it->second[queryIndex]) { + return DAWN_VALIDATION_ERROR( + "The same query cannot be written twice in same render pass."); + } + + return {}; + } + + } // namespace // The usage tracker is passed in here, because it is prepopulated with usages from the // BeginRenderPassCmd. If we had RenderPassEncoder responsible for recording the @@ -58,6 +74,22 @@ namespace dawn_native { return new RenderPassEncoder(device, commandEncoder, encodingContext, ObjectBase::kError); } + void RenderPassEncoder::TrackQueryAvailability(QuerySetBase* querySet, uint32_t queryIndex) { + DAWN_ASSERT(querySet != nullptr); + + // Gets the iterator for that querySet or create a new vector of bool set to false + // if the querySet wasn't registered. + auto it = mQueryAvailabilityMap.emplace(querySet, querySet->GetQueryCount()).first; + it->second[queryIndex] = 1; + + // Track it again on command encoder for zero-initializing when resolving unused queries. + mCommandEncoder->TrackQueryAvailability(querySet, queryIndex); + } + + const QueryAvailabilityMap& RenderPassEncoder::GetQueryAvailabilityMap() const { + return mQueryAvailabilityMap; + } + void RenderPassEncoder::EndPass() { if (mEncodingContext->TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError { allocator->Allocate(Command::EndRenderPass); @@ -180,12 +212,12 @@ namespace dawn_native { mEncodingContext->TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError { if (GetDevice()->IsValidationEnabled()) { DAWN_TRY(GetDevice()->ValidateObject(querySet)); - DAWN_TRY(ValidateTimestampQuery(querySet, queryIndex, - mCommandEncoder->GetUsedQueryIndices())); - mCommandEncoder->TrackUsedQuerySet(querySet); + DAWN_TRY(ValidateTimestampQuery(querySet, queryIndex)); + DAWN_TRY( + ValidateQueryIndexOverwrite(querySet, queryIndex, GetQueryAvailabilityMap())); } - mCommandEncoder->TrackUsedQueryIndex(querySet, queryIndex); + TrackQueryAvailability(querySet, queryIndex); WriteTimestampCmd* cmd = allocator->Allocate(Command::WriteTimestamp); diff --git a/src/dawn_native/RenderPassEncoder.h b/src/dawn_native/RenderPassEncoder.h index 0fac863d82..7bda224e6f 100644 --- a/src/dawn_native/RenderPassEncoder.h +++ b/src/dawn_native/RenderPassEncoder.h @@ -35,6 +35,9 @@ namespace dawn_native { CommandEncoder* commandEncoder, EncodingContext* encodingContext); + void TrackQueryAvailability(QuerySetBase* querySet, uint32_t queryIndex); + const QueryAvailabilityMap& GetQueryAvailabilityMap() const; + void EndPass(); void SetStencilReference(uint32_t reference); @@ -63,6 +66,11 @@ namespace dawn_native { uint32_t mRenderTargetWidth; uint32_t mRenderTargetHeight; + + // This map is to indicate the availability of the queries used in render pass. The same + // query cannot be written twice in same render pass, so each render pass also need to have + // its own query availability map. + QueryAvailabilityMap mQueryAvailabilityMap; }; } // namespace dawn_native diff --git a/src/tests/unittests/validation/QuerySetValidationTests.cpp b/src/tests/unittests/validation/QuerySetValidationTests.cpp index 25ad1cda52..965db04bf1 100644 --- a/src/tests/unittests/validation/QuerySetValidationTests.cpp +++ b/src/tests/unittests/validation/QuerySetValidationTests.cpp @@ -120,14 +120,6 @@ TEST_F(TimestampQueryValidationTest, WriteTimestampOnCommandEncoder) { ASSERT_DEVICE_ERROR(encoder.Finish()); } - // Fail to write timestamp to the same index twice on command encoder - { - wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); - encoder.WriteTimestamp(timestampQuerySet, 0); - encoder.WriteTimestamp(timestampQuerySet, 0); - ASSERT_DEVICE_ERROR(encoder.Finish()); - } - // Fail to submit timestamp query with a destroyed query set { wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); @@ -172,26 +164,6 @@ TEST_F(TimestampQueryValidationTest, WriteTimestampOnComputePassEncoder) { ASSERT_DEVICE_ERROR(encoder.Finish()); } - // Fail to write timestamp to the same index twice on compute encoder - { - wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); - wgpu::ComputePassEncoder pass = encoder.BeginComputePass(); - pass.WriteTimestamp(timestampQuerySet, 0); - pass.WriteTimestamp(timestampQuerySet, 0); - pass.EndPass(); - ASSERT_DEVICE_ERROR(encoder.Finish()); - } - - // Fail to write timestamp to the same index twice on command encoder and compute encoder - { - wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); - encoder.WriteTimestamp(timestampQuerySet, 0); - wgpu::ComputePassEncoder pass = encoder.BeginComputePass(); - pass.WriteTimestamp(timestampQuerySet, 0); - pass.EndPass(); - ASSERT_DEVICE_ERROR(encoder.Finish()); - } - // Fail to submit timestamp query with a destroyed query set { wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); @@ -240,23 +212,36 @@ TEST_F(TimestampQueryValidationTest, WriteTimestampOnRenderPassEncoder) { ASSERT_DEVICE_ERROR(encoder.Finish()); } - // Fail to write timestamp to the same index twice on command encoder and render encoder - { - wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); - wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass); - pass.WriteTimestamp(timestampQuerySet, 0); - pass.WriteTimestamp(timestampQuerySet, 0); - pass.EndPass(); - ASSERT_DEVICE_ERROR(encoder.Finish()); - } - - // Fail to write timestamp to the same index twice on command encoder and render encoder + // Success to write timestamp to the same query index twice on command encoder and render + // encoder { wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); encoder.WriteTimestamp(timestampQuerySet, 0); wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass); pass.WriteTimestamp(timestampQuerySet, 0); pass.EndPass(); + encoder.Finish(); + } + + // Success to write timestamp to the same query index twice on different render encoder + { + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + wgpu::RenderPassEncoder pass0 = encoder.BeginRenderPass(&renderPass); + pass0.WriteTimestamp(timestampQuerySet, 0); + pass0.EndPass(); + wgpu::RenderPassEncoder pass1 = encoder.BeginRenderPass(&renderPass); + pass1.WriteTimestamp(timestampQuerySet, 0); + pass1.EndPass(); + encoder.Finish(); + } + + // Fail to write timestamp to the same query index twice on same render encoder + { + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass); + pass.WriteTimestamp(timestampQuerySet, 0); + pass.WriteTimestamp(timestampQuerySet, 0); + pass.EndPass(); ASSERT_DEVICE_ERROR(encoder.Finish()); }