From 2e56970932ec01ef2b030fe6b27f305c7733a47f Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Tue, 14 May 2019 03:53:26 +0000 Subject: [PATCH] CommandAllocator: Default initalize allocated data. BUG=dawn:21 Change-Id: I98499385d7397ab431e1bbe00add7ef01941cca6 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/7160 Reviewed-by: Austin Eng Reviewed-by: Kai Ninomiya Commit-Queue: Corentin Wallez --- src/dawn_native/CommandAllocator.h | 16 ++++++- src/dawn_native/CommandEncoder.cpp | 5 --- src/dawn_native/ComputePassEncoder.cpp | 2 - src/dawn_native/ProgrammablePassEncoder.cpp | 7 +-- src/dawn_native/RenderPassEncoder.cpp | 10 +---- src/tests/unittests/CommandAllocatorTests.cpp | 45 +++++++++++++++++++ 6 files changed, 61 insertions(+), 24 deletions(-) diff --git a/src/dawn_native/CommandAllocator.h b/src/dawn_native/CommandAllocator.h index ecede3ccbb..504ba7a6a3 100644 --- a/src/dawn_native/CommandAllocator.h +++ b/src/dawn_native/CommandAllocator.h @@ -113,14 +113,26 @@ namespace dawn_native { static_assert(sizeof(E) == sizeof(uint32_t), ""); static_assert(alignof(E) == alignof(uint32_t), ""); static_assert(alignof(T) <= kMaxSupportedAlignment, ""); - return reinterpret_cast( + T* result = reinterpret_cast( Allocate(static_cast(commandId), sizeof(T), alignof(T))); + if (!result) { + return nullptr; + } + new (result) T; + return result; } template T* AllocateData(size_t count) { static_assert(alignof(T) <= kMaxSupportedAlignment, ""); - return reinterpret_cast(AllocateData(sizeof(T) * count, alignof(T))); + T* result = reinterpret_cast(AllocateData(sizeof(T) * count, alignof(T))); + if (!result) { + return nullptr; + } + for (size_t i = 0; i < count; i++) { + new (result + i) T; + } + return result; } private: diff --git a/src/dawn_native/CommandEncoder.cpp b/src/dawn_native/CommandEncoder.cpp index 7b507770e2..cc13ec2b2a 100644 --- a/src/dawn_native/CommandEncoder.cpp +++ b/src/dawn_native/CommandEncoder.cpp @@ -643,7 +643,6 @@ namespace dawn_native { mEncodingState = EncodingState::RenderPass; BeginRenderPassCmd* cmd = mAllocator.Allocate(Command::BeginRenderPass); - new (cmd) BeginRenderPassCmd; for (uint32_t i = 0; i < info->colorAttachmentCount; ++i) { if (info->colorAttachments[i] != nullptr) { @@ -695,7 +694,6 @@ namespace dawn_native { CopyBufferToBufferCmd* copy = mAllocator.Allocate(Command::CopyBufferToBuffer); - new (copy) CopyBufferToBufferCmd; copy->source.buffer = source; copy->source.offset = sourceOffset; copy->destination.buffer = destination; @@ -720,7 +718,6 @@ namespace dawn_native { CopyBufferToTextureCmd* copy = mAllocator.Allocate(Command::CopyBufferToTexture); - new (copy) CopyBufferToTextureCmd; copy->source.buffer = source->buffer; copy->source.offset = source->offset; copy->destination.texture = destination->texture; @@ -757,7 +754,6 @@ namespace dawn_native { CopyTextureToBufferCmd* copy = mAllocator.Allocate(Command::CopyTextureToBuffer); - new (copy) CopyTextureToBufferCmd; copy->source.texture = source->texture; copy->source.origin = source->origin; copy->copySize = *copySize; @@ -794,7 +790,6 @@ namespace dawn_native { CopyTextureToTextureCmd* copy = mAllocator.Allocate(Command::CopyTextureToTexture); - new (copy) CopyTextureToTextureCmd; copy->source.texture = source->texture; copy->source.origin = source->origin; copy->source.level = source->level; diff --git a/src/dawn_native/ComputePassEncoder.cpp b/src/dawn_native/ComputePassEncoder.cpp index 406a656009..1217b92897 100644 --- a/src/dawn_native/ComputePassEncoder.cpp +++ b/src/dawn_native/ComputePassEncoder.cpp @@ -44,7 +44,6 @@ namespace dawn_native { } DispatchCmd* dispatch = mAllocator->Allocate(Command::Dispatch); - new (dispatch) DispatchCmd; dispatch->x = x; dispatch->y = y; dispatch->z = z; @@ -58,7 +57,6 @@ namespace dawn_native { SetComputePipelineCmd* cmd = mAllocator->Allocate(Command::SetComputePipeline); - new (cmd) SetComputePipelineCmd; cmd->pipeline = pipeline; } diff --git a/src/dawn_native/ProgrammablePassEncoder.cpp b/src/dawn_native/ProgrammablePassEncoder.cpp index 3c2b53a623..bf9c451a8a 100644 --- a/src/dawn_native/ProgrammablePassEncoder.cpp +++ b/src/dawn_native/ProgrammablePassEncoder.cpp @@ -53,7 +53,6 @@ namespace dawn_native { InsertDebugMarkerCmd* cmd = mAllocator->Allocate(Command::InsertDebugMarker); - new (cmd) InsertDebugMarkerCmd; cmd->length = strlen(groupLabel); char* label = mAllocator->AllocateData(cmd->length + 1); @@ -65,8 +64,7 @@ namespace dawn_native { return; } - PopDebugGroupCmd* cmd = mAllocator->Allocate(Command::PopDebugGroup); - new (cmd) PopDebugGroupCmd; + mAllocator->Allocate(Command::PopDebugGroup); } void ProgrammablePassEncoder::PushDebugGroup(const char* groupLabel) { @@ -75,7 +73,6 @@ namespace dawn_native { } PushDebugGroupCmd* cmd = mAllocator->Allocate(Command::PushDebugGroup); - new (cmd) PushDebugGroupCmd; cmd->length = strlen(groupLabel); char* label = mAllocator->AllocateData(cmd->length + 1); @@ -103,7 +100,6 @@ namespace dawn_native { } SetBindGroupCmd* cmd = mAllocator->Allocate(Command::SetBindGroup); - new (cmd) SetBindGroupCmd; cmd->index = groupIndex; cmd->group = group; } @@ -128,7 +124,6 @@ namespace dawn_native { SetPushConstantsCmd* cmd = mAllocator->Allocate(Command::SetPushConstants); - new (cmd) SetPushConstantsCmd; cmd->stages = stages; cmd->offset = offset; cmd->count = count; diff --git a/src/dawn_native/RenderPassEncoder.cpp b/src/dawn_native/RenderPassEncoder.cpp index 7470063487..c83666b28d 100644 --- a/src/dawn_native/RenderPassEncoder.cpp +++ b/src/dawn_native/RenderPassEncoder.cpp @@ -50,7 +50,6 @@ namespace dawn_native { } DrawCmd* draw = mAllocator->Allocate(Command::Draw); - new (draw) DrawCmd; draw->vertexCount = vertexCount; draw->instanceCount = instanceCount; draw->firstVertex = firstVertex; @@ -67,7 +66,6 @@ namespace dawn_native { } DrawIndexedCmd* draw = mAllocator->Allocate(Command::DrawIndexed); - new (draw) DrawIndexedCmd; draw->indexCount = indexCount; draw->instanceCount = instanceCount; draw->firstIndex = firstIndex; @@ -83,7 +81,6 @@ namespace dawn_native { SetRenderPipelineCmd* cmd = mAllocator->Allocate(Command::SetRenderPipeline); - new (cmd) SetRenderPipelineCmd; cmd->pipeline = pipeline; } @@ -94,7 +91,6 @@ namespace dawn_native { SetStencilReferenceCmd* cmd = mAllocator->Allocate(Command::SetStencilReference); - new (cmd) SetStencilReferenceCmd; cmd->reference = reference; } @@ -104,7 +100,6 @@ namespace dawn_native { } SetBlendColorCmd* cmd = mAllocator->Allocate(Command::SetBlendColor); - new (cmd) SetBlendColorCmd; cmd->color = *color; } @@ -121,7 +116,6 @@ namespace dawn_native { } SetScissorRectCmd* cmd = mAllocator->Allocate(Command::SetScissorRect); - new (cmd) SetScissorRectCmd; cmd->x = x; cmd->y = y; cmd->width = width; @@ -135,7 +129,6 @@ namespace dawn_native { } SetIndexBufferCmd* cmd = mAllocator->Allocate(Command::SetIndexBuffer); - new (cmd) SetIndexBufferCmd; cmd->buffer = buffer; cmd->offset = offset; } @@ -156,13 +149,12 @@ namespace dawn_native { SetVertexBuffersCmd* cmd = mAllocator->Allocate(Command::SetVertexBuffers); - new (cmd) SetVertexBuffersCmd; cmd->startSlot = startSlot; cmd->count = count; Ref* cmdBuffers = mAllocator->AllocateData>(count); for (size_t i = 0; i < count; ++i) { - new (&cmdBuffers[i]) Ref(buffers[i]); + cmdBuffers[i] = buffers[i]; } uint64_t* cmdOffsets = mAllocator->AllocateData(count); diff --git a/src/tests/unittests/CommandAllocatorTests.cpp b/src/tests/unittests/CommandAllocatorTests.cpp index 8e8b7a770d..206261fc0e 100644 --- a/src/tests/unittests/CommandAllocatorTests.cpp +++ b/src/tests/unittests/CommandAllocatorTests.cpp @@ -401,3 +401,48 @@ TEST(CommandAllocator, AllocationOverflow_8) { allocator.AllocateData>(std::numeric_limits::max() / 8); ASSERT_EQ(data, nullptr); } + +template +struct IntWithDefault { + IntWithDefault() : value(DefaultValue) { + } + + int value; +}; + +// Test that the allcator correctly defaults initalizes data for Allocate +TEST(CommandAllocator, AllocateDefaultInitializes) { + CommandAllocator allocator; + + IntWithDefault<42>* int42 = allocator.Allocate>(CommandType::Draw); + ASSERT_EQ(int42->value, 42); + + IntWithDefault<43>* int43 = allocator.Allocate>(CommandType::Draw); + ASSERT_EQ(int43->value, 43); + + IntWithDefault<44>* int44 = allocator.Allocate>(CommandType::Draw); + ASSERT_EQ(int44->value, 44); + + CommandIterator iterator(std::move(allocator)); + iterator.DataWasDestroyed(); +} + +// Test that the allcator correctly defaults initalizes data for AllocateData +TEST(CommandAllocator, AllocateDataDefaultInitializes) { + CommandAllocator allocator; + + IntWithDefault<33>* int33 = allocator.AllocateData>(1); + ASSERT_EQ(int33[0].value, 33); + + IntWithDefault<34>* int34 = allocator.AllocateData>(2); + ASSERT_EQ(int34[0].value, 34); + ASSERT_EQ(int34[0].value, 34); + + IntWithDefault<35>* int35 = allocator.AllocateData>(3); + ASSERT_EQ(int35[0].value, 35); + ASSERT_EQ(int35[1].value, 35); + ASSERT_EQ(int35[2].value, 35); + + CommandIterator iterator(std::move(allocator)); + iterator.DataWasDestroyed(); +}