From 44d729fc8cf50d7dac6bbcdb378d0f6d26eb015e Mon Sep 17 00:00:00 2001 From: Ken Rockot Date: Mon, 20 Sep 2021 20:39:15 +0000 Subject: [PATCH] Introduce BufferLocation This is a simple RefCounted holder of a Buffer ref and offset. Encoded commands can store a ref to this object instead of directly storing an inline Buffer ref and offset, allowing other commands to dynamically patch in a different buffer+offset as needed, in a memory-safe way; as opposed to e.g. retaining an unmanaged pointer to the encoded command itself and modifying it in-place. Validation commands will use this to rewrite buffer references in encoded commands, so that they execute over validated inputs rather than over their original client-provided inputs. No net functional changes in this CL, just some groundwork for indirect draw/dispatch validation. Bug: dawn:809 Change-Id: I0570521a610fe3ea08190a525b4904749d7b7f24 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/64420 Reviewed-by: Austin Eng Reviewed-by: Corentin Wallez Commit-Queue: Ken Rockot --- src/dawn_native/BUILD.gn | 2 + src/dawn_native/Buffer.cpp | 1 + src/dawn_native/BufferLocation.cpp | 54 ++++++++++++++++++++ src/dawn_native/BufferLocation.h | 49 ++++++++++++++++++ src/dawn_native/Commands.h | 4 +- src/dawn_native/RenderEncoderBase.cpp | 4 +- src/dawn_native/d3d12/CommandBufferD3D12.cpp | 6 ++- src/dawn_native/metal/CommandBufferMTL.mm | 7 +-- src/dawn_native/opengl/CommandBufferGL.cpp | 9 ++-- src/dawn_native/vulkan/CommandBufferVk.cpp | 10 ++-- 10 files changed, 129 insertions(+), 17 deletions(-) create mode 100644 src/dawn_native/BufferLocation.cpp create mode 100644 src/dawn_native/BufferLocation.h diff --git a/src/dawn_native/BUILD.gn b/src/dawn_native/BUILD.gn index ad46ffc25f..b3b1f6e60b 100644 --- a/src/dawn_native/BUILD.gn +++ b/src/dawn_native/BUILD.gn @@ -186,6 +186,8 @@ source_set("dawn_native_sources") { "BuddyMemoryAllocator.h", "Buffer.cpp", "Buffer.h", + "BufferLocation.cpp", + "BufferLocation.h", "CachedObject.cpp", "CachedObject.h", "CallbackTaskManager.cpp", diff --git a/src/dawn_native/Buffer.cpp b/src/dawn_native/Buffer.cpp index d9ca585fc7..7698c978aa 100644 --- a/src/dawn_native/Buffer.cpp +++ b/src/dawn_native/Buffer.cpp @@ -589,4 +589,5 @@ namespace dawn_native { bool BufferBase::IsFullBufferRange(uint64_t offset, uint64_t size) const { return offset == 0 && size == GetSize(); } + } // namespace dawn_native diff --git a/src/dawn_native/BufferLocation.cpp b/src/dawn_native/BufferLocation.cpp new file mode 100644 index 0000000000..5ee24f2ff8 --- /dev/null +++ b/src/dawn_native/BufferLocation.cpp @@ -0,0 +1,54 @@ +// Copyright 2021 The Dawn Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "dawn_native/BufferLocation.h" + +namespace dawn_native { + + BufferLocation::BufferLocation() = default; + + BufferLocation::BufferLocation(BufferBase* buffer, uint64_t offset) + : mBuffer(buffer), mOffset(offset) { + } + + BufferLocation::~BufferLocation() = default; + + // static + Ref BufferLocation::New() { + return AcquireRef(new BufferLocation()); + } + + // static + Ref BufferLocation::New(BufferBase* buffer, uint64_t offset) { + return AcquireRef(new BufferLocation(buffer, offset)); + } + + bool BufferLocation::IsNull() const { + return mBuffer.Get() == nullptr; + } + + BufferBase* BufferLocation::GetBuffer() const { + return mBuffer.Get(); + } + + uint64_t BufferLocation::GetOffset() const { + return mOffset; + } + + void BufferLocation::Set(BufferBase* buffer, uint64_t offset) { + mBuffer = buffer; + mOffset = offset; + } + +} // namespace dawn_native diff --git a/src/dawn_native/BufferLocation.h b/src/dawn_native/BufferLocation.h new file mode 100644 index 0000000000..4ff733d972 --- /dev/null +++ b/src/dawn_native/BufferLocation.h @@ -0,0 +1,49 @@ +// Copyright 2021 The Dawn Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef DAWNNATIVE_BUFFERLOCATION_H_ +#define DAWNNATIVE_BUFFERLOCATION_H_ + +#include "common/RefCounted.h" +#include "dawn_native/Buffer.h" + +#include + +namespace dawn_native { + + // A ref-counted wrapper around a Buffer ref and an offset into the buffer. + class BufferLocation : public RefCounted { + public: + BufferLocation(); + BufferLocation(BufferBase* buffer, uint64_t offset = 0); + ~BufferLocation(); + + static Ref New(); + static Ref New(BufferBase* buffer, uint64_t offset = 0); + + bool IsNull() const; + + BufferBase* GetBuffer() const; + uint64_t GetOffset() const; + + void Set(BufferBase* buffer, uint64_t offset); + + private: + Ref mBuffer; + uint64_t mOffset = 0; + }; + +} // namespace dawn_native + +#endif // DAWNNATIVE_BUFFERLOCATION_H_ diff --git a/src/dawn_native/Commands.h b/src/dawn_native/Commands.h index cee9bbd52e..333d19ba10 100644 --- a/src/dawn_native/Commands.h +++ b/src/dawn_native/Commands.h @@ -19,6 +19,7 @@ #include "dawn_native/AttachmentState.h" #include "dawn_native/BindingInfo.h" +#include "dawn_native/BufferLocation.h" #include "dawn_native/Texture.h" #include "dawn_native/dawn_platform.h" @@ -177,8 +178,7 @@ namespace dawn_native { }; struct DrawIndexedIndirectCmd { - Ref indirectBuffer; - uint64_t indirectOffset; + Ref indirectBufferLocation; }; struct EndComputePassCmd {}; diff --git a/src/dawn_native/RenderEncoderBase.cpp b/src/dawn_native/RenderEncoderBase.cpp index e98c45a2b5..06b0f9b690 100644 --- a/src/dawn_native/RenderEncoderBase.cpp +++ b/src/dawn_native/RenderEncoderBase.cpp @@ -17,6 +17,7 @@ #include "common/Constants.h" #include "common/Log.h" #include "dawn_native/Buffer.h" +#include "dawn_native/BufferLocation.h" #include "dawn_native/CommandEncoder.h" #include "dawn_native/CommandValidation.h" #include "dawn_native/Commands.h" @@ -178,8 +179,7 @@ namespace dawn_native { DrawIndexedIndirectCmd* cmd = allocator->Allocate(Command::DrawIndexedIndirect); - cmd->indirectBuffer = indirectBuffer; - cmd->indirectOffset = indirectOffset; + cmd->indirectBufferLocation = BufferLocation::New(indirectBuffer, indirectOffset); mUsageTracker.BufferUsedAs(indirectBuffer, wgpu::BufferUsage::Indirect); diff --git a/src/dawn_native/d3d12/CommandBufferD3D12.cpp b/src/dawn_native/d3d12/CommandBufferD3D12.cpp index 72e9191172..175e3dd411 100644 --- a/src/dawn_native/d3d12/CommandBufferD3D12.cpp +++ b/src/dawn_native/d3d12/CommandBufferD3D12.cpp @@ -1383,14 +1383,16 @@ namespace dawn_native { namespace d3d12 { case Command::DrawIndexedIndirect: { DrawIndexedIndirectCmd* draw = iter->NextCommand(); + ASSERT(!draw->indirectBufferLocation->IsNull()); DAWN_TRY(bindingTracker->Apply(commandContext)); vertexBufferTracker.Apply(commandList, lastPipeline); - Buffer* buffer = ToBackend(draw->indirectBuffer.Get()); + Buffer* buffer = ToBackend(draw->indirectBufferLocation->GetBuffer()); ComPtr signature = ToBackend(GetDevice())->GetDrawIndexedIndirectSignature(); commandList->ExecuteIndirect(signature.Get(), 1, buffer->GetD3D12Resource(), - draw->indirectOffset, nullptr, 0); + draw->indirectBufferLocation->GetOffset(), nullptr, + 0); break; } diff --git a/src/dawn_native/metal/CommandBufferMTL.mm b/src/dawn_native/metal/CommandBufferMTL.mm index e9825a824f..4dd47a5704 100644 --- a/src/dawn_native/metal/CommandBufferMTL.mm +++ b/src/dawn_native/metal/CommandBufferMTL.mm @@ -1332,20 +1332,21 @@ namespace dawn_native { namespace metal { } case Command::DrawIndexedIndirect: { - DrawIndirectCmd* draw = iter->NextCommand(); + DrawIndexedIndirectCmd* draw = iter->NextCommand(); vertexBuffers.Apply(encoder, lastPipeline, enableVertexPulling); bindGroups.Apply(encoder); storageBufferLengths.Apply(encoder, lastPipeline, enableVertexPulling); - Buffer* buffer = ToBackend(draw->indirectBuffer.Get()); + ASSERT(!draw->indirectBufferLocation->IsNull()); + Buffer* buffer = ToBackend(draw->indirectBufferLocation->GetBuffer()); id indirectBuffer = buffer->GetMTLBuffer(); [encoder drawIndexedPrimitives:lastPipeline->GetMTLPrimitiveTopology() indexType:indexBufferType indexBuffer:indexBuffer indexBufferOffset:indexBufferBaseOffset indirectBuffer:indirectBuffer - indirectBufferOffset:draw->indirectOffset]; + indirectBufferOffset:draw->indirectBufferLocation->GetOffset()]; break; } diff --git a/src/dawn_native/opengl/CommandBufferGL.cpp b/src/dawn_native/opengl/CommandBufferGL.cpp index 7d2014db1b..099d590a68 100644 --- a/src/dawn_native/opengl/CommandBufferGL.cpp +++ b/src/dawn_native/opengl/CommandBufferGL.cpp @@ -1182,16 +1182,17 @@ namespace dawn_native { namespace opengl { case Command::DrawIndexedIndirect: { DrawIndexedIndirectCmd* draw = iter->NextCommand(); + ASSERT(!draw->indirectBufferLocation->IsNull()); + vertexStateBufferBindingTracker.Apply(gl); bindGroupTracker.Apply(gl); - uint64_t indirectBufferOffset = draw->indirectOffset; - Buffer* indirectBuffer = ToBackend(draw->indirectBuffer.Get()); - + Buffer* indirectBuffer = ToBackend(draw->indirectBufferLocation->GetBuffer()); gl.BindBuffer(GL_DRAW_INDIRECT_BUFFER, indirectBuffer->GetHandle()); gl.DrawElementsIndirect( lastPipeline->GetGLPrimitiveTopology(), indexBufferFormat, - reinterpret_cast(static_cast(indirectBufferOffset))); + reinterpret_cast( + static_cast(draw->indirectBufferLocation->GetOffset()))); break; } diff --git a/src/dawn_native/vulkan/CommandBufferVk.cpp b/src/dawn_native/vulkan/CommandBufferVk.cpp index 72a7f8b336..cfc2a71fa8 100644 --- a/src/dawn_native/vulkan/CommandBufferVk.cpp +++ b/src/dawn_native/vulkan/CommandBufferVk.cpp @@ -1079,13 +1079,15 @@ namespace dawn_native { namespace vulkan { } case Command::DrawIndexedIndirect: { - DrawIndirectCmd* draw = iter->NextCommand(); - VkBuffer indirectBuffer = ToBackend(draw->indirectBuffer)->GetHandle(); + DrawIndexedIndirectCmd* draw = iter->NextCommand(); + ASSERT(!draw->indirectBufferLocation->IsNull()); + VkBuffer indirectBuffer = + ToBackend(draw->indirectBufferLocation->GetBuffer())->GetHandle(); descriptorSets.Apply(device, recordingContext, VK_PIPELINE_BIND_POINT_GRAPHICS); device->fn.CmdDrawIndexedIndirect( - commands, indirectBuffer, static_cast(draw->indirectOffset), - 1, 0); + commands, indirectBuffer, + static_cast(draw->indirectBufferLocation->GetOffset()), 1, 0); break; }