Remove deferred BufferLocation updates for drawIndexedIndirect

Instead of using BufferLocation as another layer of indirection,
the indirectBuffer can be set directly on the indirect command.
This makes the indirect validation a bit simpler, but introduces
additional lifetime dependencies in that the indirect draw validation
MUST be encoded while the DrawIndexedIndirectCmds it references
are still valid.

Bug: dawn:809
Change-Id: I1ef084622d8737ad5ec1b0247bf9062712e35008
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/67241
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
This commit is contained in:
Austin Eng 2021-11-02 18:23:49 +00:00 committed by Dawn LUCI CQ
parent d4b9cda056
commit 6abf1a1adb
18 changed files with 45 additions and 218 deletions

View File

@ -205,8 +205,6 @@ source_set("dawn_native_sources") {
"BuddyMemoryAllocator.h",
"Buffer.cpp",
"Buffer.h",
"BufferLocation.cpp",
"BufferLocation.h",
"CachedObject.cpp",
"CachedObject.h",
"CallbackTaskManager.cpp",

View File

@ -1,54 +0,0 @@
// 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> BufferLocation::New() {
return AcquireRef(new BufferLocation());
}
// static
Ref<BufferLocation> 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

View File

@ -1,49 +0,0 @@
// 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 <cstdint>
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<BufferLocation> New();
static Ref<BufferLocation> 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<BufferBase> mBuffer;
uint64_t mOffset = 0;
};
} // namespace dawn_native
#endif // DAWNNATIVE_BUFFERLOCATION_H_

View File

@ -50,8 +50,6 @@ target_sources(dawn_native PRIVATE
"BuddyMemoryAllocator.h"
"Buffer.cpp"
"Buffer.h"
"BufferLocation.cpp"
"BufferLocation.h"
"CachedObject.cpp"
"CachedObject.h"
"CallbackTaskManager.cpp"

View File

@ -39,14 +39,6 @@ namespace dawn_native {
Destroy();
}
void CommandBufferBase::DoNextSetValidatedBufferLocationsInternal() {
SetValidatedBufferLocationsInternalCmd* cmd =
mCommands.NextCommand<SetValidatedBufferLocationsInternalCmd>();
for (const DeferredBufferLocationUpdate& update : cmd->updates) {
update.location->Set(update.buffer.Get(), update.offset);
}
}
// static
CommandBufferBase* CommandBufferBase::MakeError(DeviceBase* device) {
return new CommandBufferBase(device, ObjectBase::kError);

View File

@ -48,8 +48,6 @@ namespace dawn_native {
protected:
~CommandBufferBase() override;
void DoNextSetValidatedBufferLocationsInternal();
CommandIterator mCommands;
private:

View File

@ -1018,18 +1018,6 @@ namespace dawn_native {
return commandBuffer.Detach();
}
void CommandEncoder::EncodeSetValidatedBufferLocationsInternal(
std::vector<DeferredBufferLocationUpdate> updates) {
ASSERT(GetDevice()->IsValidationEnabled());
mEncodingContext.TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError {
SetValidatedBufferLocationsInternalCmd* cmd =
allocator->Allocate<SetValidatedBufferLocationsInternalCmd>(
Command::SetValidatedBufferLocationsInternal);
cmd->updates = std::move(updates);
return {};
});
}
ResultOrError<Ref<CommandBufferBase>> CommandEncoder::FinishInternal(
const CommandBufferDescriptor* descriptor) {
DeviceBase* device = GetDevice();

View File

@ -78,9 +78,6 @@ namespace dawn_native {
CommandBufferBase* APIFinish(const CommandBufferDescriptor* descriptor = nullptr);
void EncodeSetValidatedBufferLocationsInternal(
std::vector<DeferredBufferLocationUpdate> updates);
private:
ResultOrError<Ref<CommandBufferBase>> FinishInternal(
const CommandBufferDescriptor* descriptor);

View File

@ -158,12 +158,6 @@ namespace dawn_native {
cmd->~SetStencilReferenceCmd();
break;
}
case Command::SetValidatedBufferLocationsInternal: {
SetValidatedBufferLocationsInternalCmd* cmd =
commands->NextCommand<SetValidatedBufferLocationsInternalCmd>();
cmd->~SetValidatedBufferLocationsInternalCmd();
break;
}
case Command::SetViewport: {
SetViewportCmd* cmd = commands->NextCommand<SetViewportCmd>();
cmd->~SetViewportCmd();
@ -319,10 +313,6 @@ namespace dawn_native {
commands->NextCommand<SetStencilReferenceCmd>();
break;
case Command::SetValidatedBufferLocationsInternal:
commands->NextCommand<SetValidatedBufferLocationsInternalCmd>();
break;
case Command::SetViewport:
commands->NextCommand<SetViewportCmd>();
break;

View File

@ -19,7 +19,6 @@
#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"
@ -63,7 +62,6 @@ namespace dawn_native {
SetBlendConstant,
SetBindGroup,
SetIndexBuffer,
SetValidatedBufferLocationsInternal,
SetVertexBuffer,
WriteBuffer,
WriteTimestamp,
@ -179,7 +177,8 @@ namespace dawn_native {
};
struct DrawIndexedIndirectCmd {
Ref<BufferLocation> indirectBufferLocation;
Ref<BufferBase> indirectBuffer;
uint64_t indirectOffset;
};
struct EndComputePassCmd {};
@ -225,16 +224,6 @@ namespace dawn_native {
uint32_t reference;
};
struct DeferredBufferLocationUpdate {
Ref<BufferLocation> location;
Ref<BufferBase> buffer;
uint64_t offset;
};
struct SetValidatedBufferLocationsInternalCmd {
std::vector<DeferredBufferLocationUpdate> updates;
};
struct SetViewportCmd {
float x, y, width, height, minDepth, maxDepth;
};

View File

@ -159,12 +159,11 @@ namespace dawn_native {
}
}
void IndirectDrawMetadata::AddIndexedIndirectDraw(
wgpu::IndexFormat indexFormat,
uint64_t indexBufferSize,
BufferBase* indirectBuffer,
uint64_t indirectOffset,
BufferLocation* drawCmdIndirectBufferLocation) {
void IndirectDrawMetadata::AddIndexedIndirectDraw(wgpu::IndexFormat indexFormat,
uint64_t indexBufferSize,
BufferBase* indirectBuffer,
uint64_t indirectOffset,
DrawIndexedIndirectCmd* cmd) {
uint64_t numIndexBufferElements;
switch (indexFormat) {
case wgpu::IndexFormat::Uint16:
@ -187,7 +186,7 @@ namespace dawn_native {
IndexedIndirectDraw draw;
draw.clientBufferOffset = indirectOffset;
draw.bufferLocation = drawCmdIndirectBufferLocation;
draw.cmd = cmd;
it->second.AddIndexedIndirectDraw(mMaxDrawCallsPerBatch, mMaxBatchOffsetRange,
std::move(draw));
}

View File

@ -18,7 +18,6 @@
#include "common/NonCopyable.h"
#include "common/RefCounted.h"
#include "dawn_native/Buffer.h"
#include "dawn_native/BufferLocation.h"
#include "dawn_native/CommandBufferStateTracker.h"
#include "dawn_native/Commands.h"
@ -45,7 +44,10 @@ namespace dawn_native {
public:
struct IndexedIndirectDraw {
uint64_t clientBufferOffset;
Ref<BufferLocation> bufferLocation;
// This is a pointer to the command that should be populated with the validated
// indirect scratch buffer. It is only valid up until the encoded command buffer
// is submitted.
DrawIndexedIndirectCmd* cmd;
};
struct IndexedIndirectValidationBatch {
@ -109,7 +111,7 @@ namespace dawn_native {
uint64_t indexBufferSize,
BufferBase* indirectBuffer,
uint64_t indirectOffset,
BufferLocation* drawCmdIndirectBufferLocation);
DrawIndexedIndirectCmd* cmd);
private:
IndexedIndirectBufferValidationInfoMap mIndexedIndirectBufferValidationInfo;

View File

@ -226,7 +226,6 @@ namespace dawn_native {
// single pass as possible. Batches can be grouped together as long as they're validating
// data from the same indirect buffer, but they may still be split into multiple passes if
// the number of draw calls in a pass would exceed some (very high) upper bound.
uint64_t numTotalDrawCalls = 0;
size_t validatedParamsSize = 0;
std::vector<Pass> passes;
IndirectDrawMetadata::IndexedIndirectBufferValidationInfoMap& bufferInfoMap =
@ -257,7 +256,6 @@ namespace dawn_native {
newBatch.clientIndirectOffset = minOffsetAlignedDown;
newBatch.clientIndirectSize =
batch.maxOffset + kDrawIndexedIndirectSize - minOffsetAlignedDown;
numTotalDrawCalls += batch.draws.size();
newBatch.validatedParamsSize = batch.draws.size() * kDrawIndexedIndirectSize;
newBatch.validatedParamsOffset =
@ -306,10 +304,7 @@ namespace dawn_native {
DAWN_TRY(validatedParamsBuffer.EnsureCapacity(validatedParamsSize));
usageTracker->BufferUsedAs(validatedParamsBuffer.GetBuffer(), wgpu::BufferUsage::Indirect);
// Now we allocate and populate host-side batch data to be copied to the GPU, and prepare to
// update all DrawIndexedIndirectCmd buffer references.
std::vector<DeferredBufferLocationUpdate> deferredBufferLocationUpdates;
deferredBufferLocationUpdates.reserve(numTotalDrawCalls);
// Now we allocate and populate host-side batch data to be copied to the GPU.
for (Pass& pass : passes) {
// We use std::malloc here because it guarantees maximal scalar alignment.
pass.batchData = {std::malloc(pass.batchDataSize), std::free};
@ -322,16 +317,13 @@ namespace dawn_native {
uint32_t* indirectOffsets = reinterpret_cast<uint32_t*>(batch.batchInfo + 1);
uint64_t validatedParamsOffset = batch.validatedParamsOffset;
for (const auto& draw : batch.metadata->draws) {
for (auto& draw : batch.metadata->draws) {
// The shader uses this to index an array of u32, hence the division by 4 bytes.
*indirectOffsets++ = static_cast<uint32_t>(
(draw.clientBufferOffset - batch.clientIndirectOffset) / 4);
DeferredBufferLocationUpdate deferredUpdate;
deferredUpdate.location = draw.bufferLocation;
deferredUpdate.buffer = validatedParamsBuffer.GetBuffer();
deferredUpdate.offset = validatedParamsOffset;
deferredBufferLocationUpdates.push_back(std::move(deferredUpdate));
draw.cmd->indirectBuffer = validatedParamsBuffer.GetBuffer();
draw.cmd->indirectOffset = validatedParamsOffset;
validatedParamsOffset += kDrawIndexedIndirectSize;
}
@ -364,8 +356,6 @@ namespace dawn_native {
// Finally, we can now encode our validation passes. Each pass first does a single
// WriteBuffer to get batch data over to the GPU, followed by a single compute pass. The
// compute pass encodes a separate SetBindGroup and Dispatch command for each batch.
commandEncoder->EncodeSetValidatedBufferLocationsInternal(
std::move(deferredBufferLocationUpdates));
for (const Pass& pass : passes) {
commandEncoder->APIWriteBuffer(batchDataBuffer.GetBuffer(), 0,
static_cast<const uint8_t*>(pass.batchData.get()),

View File

@ -17,7 +17,6 @@
#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"
@ -198,14 +197,20 @@ namespace dawn_native {
DrawIndexedIndirectCmd* cmd =
allocator->Allocate<DrawIndexedIndirectCmd>(Command::DrawIndexedIndirect);
if (IsValidationEnabled()) {
cmd->indirectBufferLocation = BufferLocation::New();
// Later, EncodeIndirectDrawValidationCommands will allocate a scratch storage
// buffer which will store the validated indirect data. The buffer and offset
// will be updated to point to it.
// |EncodeIndirectDrawValidationCommands| is called at the end of encoding the
// render pass, while the |cmd| pointer is still valid.
cmd->indirectBuffer = nullptr;
mIndirectDrawMetadata.AddIndexedIndirectDraw(
mCommandBufferState.GetIndexFormat(),
mCommandBufferState.GetIndexBufferSize(), indirectBuffer, indirectOffset,
cmd->indirectBufferLocation.Get());
cmd);
} else {
cmd->indirectBufferLocation =
BufferLocation::New(indirectBuffer, indirectOffset);
cmd->indirectBuffer = indirectBuffer;
cmd->indirectOffset = indirectOffset;
}
// TODO(crbug.com/dawn/1166): Adding the indirectBuffer is needed for correct usage

View File

@ -993,10 +993,6 @@ namespace dawn_native { namespace d3d12 {
break;
}
case Command::SetValidatedBufferLocationsInternal:
DoNextSetValidatedBufferLocationsInternal();
break;
case Command::WriteBuffer: {
WriteBufferCmd* write = mCommands.NextCommand<WriteBufferCmd>();
const uint64_t offset = write->offset;
@ -1414,7 +1410,6 @@ namespace dawn_native { namespace d3d12 {
case Command::DrawIndexedIndirect: {
DrawIndexedIndirectCmd* draw = iter->NextCommand<DrawIndexedIndirectCmd>();
ASSERT(!draw->indirectBufferLocation->IsNull());
DAWN_TRY(bindingTracker->Apply(commandContext));
vertexBufferTracker.Apply(commandList, lastPipeline);
@ -1423,12 +1418,13 @@ namespace dawn_native { namespace d3d12 {
// Zero the index offset values to avoid reusing values from the previous draw
RecordFirstIndexOffset(commandList, lastPipeline, 0, 0);
Buffer* buffer = ToBackend(draw->indirectBufferLocation->GetBuffer());
Buffer* buffer = ToBackend(draw->indirectBuffer.Get());
ASSERT(buffer != nullptr);
ComPtr<ID3D12CommandSignature> signature =
ToBackend(GetDevice())->GetDrawIndexedIndirectSignature();
commandList->ExecuteIndirect(signature.Get(), 1, buffer->GetD3D12Resource(),
draw->indirectBufferLocation->GetOffset(), nullptr,
0);
draw->indirectOffset, nullptr, 0);
break;
}

View File

@ -986,10 +986,6 @@ namespace dawn_native { namespace metal {
break;
}
case Command::SetValidatedBufferLocationsInternal:
DoNextSetValidatedBufferLocationsInternal();
break;
case Command::WriteBuffer: {
WriteBufferCmd* write = mCommands.NextCommand<WriteBufferCmd>();
const uint64_t offset = write->offset;
@ -1341,15 +1337,16 @@ namespace dawn_native { namespace metal {
bindGroups.Apply(encoder);
storageBufferLengths.Apply(encoder, lastPipeline, enableVertexPulling);
ASSERT(!draw->indirectBufferLocation->IsNull());
Buffer* buffer = ToBackend(draw->indirectBufferLocation->GetBuffer());
Buffer* buffer = ToBackend(draw->indirectBuffer.Get());
ASSERT(buffer != nullptr);
id<MTLBuffer> indirectBuffer = buffer->GetMTLBuffer();
[encoder drawIndexedPrimitives:lastPipeline->GetMTLPrimitiveTopology()
indexType:indexBufferType
indexBuffer:indexBuffer
indexBufferOffset:indexBufferBaseOffset
indirectBuffer:indirectBuffer
indirectBufferOffset:draw->indirectBufferLocation->GetOffset()];
indirectBufferOffset:draw->indirectOffset];
break;
}

View File

@ -844,10 +844,6 @@ namespace dawn_native { namespace opengl {
break;
}
case Command::SetValidatedBufferLocationsInternal:
DoNextSetValidatedBufferLocationsInternal();
break;
case Command::WriteBuffer: {
WriteBufferCmd* write = mCommands.NextCommand<WriteBufferCmd>();
uint64_t offset = write->offset;
@ -1187,17 +1183,17 @@ namespace dawn_native { namespace opengl {
case Command::DrawIndexedIndirect: {
DrawIndexedIndirectCmd* draw = iter->NextCommand<DrawIndexedIndirectCmd>();
ASSERT(!draw->indirectBufferLocation->IsNull());
vertexStateBufferBindingTracker.Apply(gl);
bindGroupTracker.Apply(gl);
Buffer* indirectBuffer = ToBackend(draw->indirectBufferLocation->GetBuffer());
Buffer* indirectBuffer = ToBackend(draw->indirectBuffer.Get());
ASSERT(indirectBuffer != nullptr);
gl.BindBuffer(GL_DRAW_INDIRECT_BUFFER, indirectBuffer->GetHandle());
gl.DrawElementsIndirect(
lastPipeline->GetGLPrimitiveTopology(), indexBufferFormat,
reinterpret_cast<void*>(
static_cast<intptr_t>(draw->indirectBufferLocation->GetOffset())));
reinterpret_cast<void*>(static_cast<intptr_t>(draw->indirectOffset)));
break;
}

View File

@ -826,10 +826,6 @@ namespace dawn_native { namespace vulkan {
break;
}
case Command::SetValidatedBufferLocationsInternal:
DoNextSetValidatedBufferLocationsInternal();
break;
case Command::WriteBuffer: {
WriteBufferCmd* write = mCommands.NextCommand<WriteBufferCmd>();
const uint64_t offset = write->offset;
@ -1075,10 +1071,10 @@ namespace dawn_native { namespace vulkan {
case Command::DrawIndirect: {
DrawIndirectCmd* draw = iter->NextCommand<DrawIndirectCmd>();
VkBuffer indirectBuffer = ToBackend(draw->indirectBuffer)->GetHandle();
Buffer* buffer = ToBackend(draw->indirectBuffer.Get());
descriptorSets.Apply(device, recordingContext, VK_PIPELINE_BIND_POINT_GRAPHICS);
device->fn.CmdDrawIndirect(commands, indirectBuffer,
device->fn.CmdDrawIndirect(commands, buffer->GetHandle(),
static_cast<VkDeviceSize>(draw->indirectOffset), 1,
0);
break;
@ -1086,14 +1082,13 @@ namespace dawn_native { namespace vulkan {
case Command::DrawIndexedIndirect: {
DrawIndexedIndirectCmd* draw = iter->NextCommand<DrawIndexedIndirectCmd>();
ASSERT(!draw->indirectBufferLocation->IsNull());
VkBuffer indirectBuffer =
ToBackend(draw->indirectBufferLocation->GetBuffer())->GetHandle();
Buffer* buffer = ToBackend(draw->indirectBuffer.Get());
ASSERT(buffer != nullptr);
descriptorSets.Apply(device, recordingContext, VK_PIPELINE_BIND_POINT_GRAPHICS);
device->fn.CmdDrawIndexedIndirect(
commands, indirectBuffer,
static_cast<VkDeviceSize>(draw->indirectBufferLocation->GetOffset()), 1, 0);
commands, buffer->GetHandle(),
static_cast<VkDeviceSize>(draw->indirectOffset), 1, 0);
break;
}