Skip to content

Commit

Permalink
Support validating buffer usage with usage validation mode
Browse files Browse the repository at this point in the history
This patch adds `UsageValidationMode` as the third parameter of
`ValidateCanUseAs` with a buffer as input to align with the
format of the function `ValidateCanUseAs` with a texture as input.

This patch also updates the test `InternalStorageBufferBindingTests`
by using classes in `native::` instead of the `wgpu::` to better
test the `internal usage` of the buffers.

Bug: chromium:42240463
Test: dawn_unittests
Change-Id: I2db2c922555ded050949da20828be17c95499152
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/204914
Commit-Queue: Jiawei Shao <[email protected]>
Reviewed-by: Corentin Wallez <[email protected]>
  • Loading branch information
Jiawei-Shao authored and Dawn LUCI CQ committed Sep 6, 2024
1 parent 7678e69 commit 1f3f5df
Show file tree
Hide file tree
Showing 9 changed files with 152 additions and 94 deletions.
10 changes: 4 additions & 6 deletions src/dawn/native/BindGroup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ namespace {

MaybeError ValidateBufferBinding(const DeviceBase* device,
const BindGroupEntry& entry,
const BufferBindingInfo& layout) {
const BufferBindingInfo& layout,
UsageValidationMode mode) {
DAWN_INVALID_IF(entry.buffer == nullptr, "Binding entry buffer not set.");

DAWN_INVALID_IF(entry.sampler != nullptr || entry.textureView != nullptr,
Expand Down Expand Up @@ -115,9 +116,7 @@ MaybeError ValidateBufferBinding(const DeviceBase* device,
"Offset (%u) of %s does not satisfy the minimum %s alignment (%u).",
entry.offset, entry.buffer, layout.type, requiredBindingAlignment);

DAWN_INVALID_IF(!(entry.buffer->GetInternalUsage() & requiredUsage),
"Binding usage (%s) of %s doesn't match expected usage (%s).",
entry.buffer->GetUsage(), entry.buffer, requiredUsage);
DAWN_TRY(ValidateCanUseAs(entry.buffer, requiredUsage, mode));

DAWN_INVALID_IF(bindingSize < layout.minBindingSize,
"Binding size (%u) of %s is smaller than the minimum binding size (%u).",
Expand Down Expand Up @@ -376,8 +375,7 @@ MaybeError ValidateBindGroupDescriptor(DeviceBase* device,
DAWN_TRY(MatchVariant(
bindingInfo.bindingLayout,
[&](const BufferBindingInfo& layout) -> MaybeError {
// TODO(dawn:1485): Validate buffer binding with usage validation mode.
DAWN_TRY_CONTEXT(ValidateBufferBinding(device, entry, layout),
DAWN_TRY_CONTEXT(ValidateBufferBinding(device, entry, layout, mode),
"validating entries[%u] as a Buffer."
"\nExpected entry layout: %s",
i, layout);
Expand Down
37 changes: 23 additions & 14 deletions src/dawn/native/CommandEncoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1452,10 +1452,12 @@ void CommandEncoder::APICopyBufferToBuffer(BufferBase* source,
"validating destination %s copy size.", destination);
DAWN_TRY(ValidateB2BCopyAlignment(size, sourceOffset, destinationOffset));

DAWN_TRY_CONTEXT(ValidateCanUseAs(source, wgpu::BufferUsage::CopySrc),
"validating source %s usage.", source);
DAWN_TRY_CONTEXT(ValidateCanUseAs(destination, wgpu::BufferUsage::CopyDst),
"validating destination %s usage.", destination);
DAWN_TRY_CONTEXT(
ValidateCanUseAs(source, wgpu::BufferUsage::CopySrc, mUsageValidationMode),
"validating source %s usage.", source);
DAWN_TRY_CONTEXT(
ValidateCanUseAs(destination, wgpu::BufferUsage::CopyDst, mUsageValidationMode),
"validating destination %s usage.", destination);
}

mTopLevelBuffers.insert(source);
Expand Down Expand Up @@ -1501,10 +1503,12 @@ void CommandEncoder::InternalCopyBufferToBufferWithAllocatedSize(BufferBase* sou
destination);
DAWN_TRY(ValidateB2BCopyAlignment(size, sourceOffset, destinationOffset));

DAWN_TRY_CONTEXT(ValidateCanUseAsInternal(
source, wgpu::BufferUsage::CopySrc | kInternalCopySrcBuffer),
"validating source %s usage.", source);
DAWN_TRY_CONTEXT(ValidateCanUseAs(destination, wgpu::BufferUsage::CopyDst),
DAWN_TRY_CONTEXT(
ValidateCanUseAs(source, wgpu::BufferUsage::CopySrc | kInternalCopySrcBuffer,
UsageValidationMode::Internal),
"validating source %s usage.", source);
DAWN_TRY_CONTEXT(ValidateCanUseAs(destination, wgpu::BufferUsage::CopyDst,
UsageValidationMode::Internal),
"validating destination %s usage.", destination);
}

Expand Down Expand Up @@ -1535,7 +1539,8 @@ void CommandEncoder::APICopyBufferToTexture(const ImageCopyBuffer* source,
[&](CommandAllocator* allocator) -> MaybeError {
if (GetDevice()->IsValidationEnabled()) {
DAWN_TRY(ValidateImageCopyBuffer(GetDevice(), *source));
DAWN_TRY_CONTEXT(ValidateCanUseAs(source->buffer, wgpu::BufferUsage::CopySrc),
DAWN_TRY_CONTEXT(ValidateCanUseAs(source->buffer, wgpu::BufferUsage::CopySrc,
mUsageValidationMode),
"validating source %s usage.", source->buffer);

DAWN_TRY(ValidateImageCopyTexture(GetDevice(), destination, *copySize));
Expand Down Expand Up @@ -1631,7 +1636,8 @@ void CommandEncoder::APICopyTextureToBuffer(const ImageCopyTexture* sourceOrig,
DAWN_TRY(ValidateTextureDepthStencilToBufferCopyRestrictions(source));

DAWN_TRY(ValidateImageCopyBuffer(GetDevice(), *destination));
DAWN_TRY_CONTEXT(ValidateCanUseAs(destination->buffer, wgpu::BufferUsage::CopyDst),
DAWN_TRY_CONTEXT(ValidateCanUseAs(destination->buffer, wgpu::BufferUsage::CopyDst,
mUsageValidationMode),
"validating destination %s usage.", destination->buffer);

// We validate texture copy range before validating linear texture data,
Expand Down Expand Up @@ -1862,8 +1868,9 @@ void CommandEncoder::APIClearBuffer(BufferBase* buffer, uint64_t offset, uint64_
offset, size, bufferSize, buffer);
}

DAWN_TRY_CONTEXT(ValidateCanUseAs(buffer, wgpu::BufferUsage::CopyDst),
"validating buffer %s usage.", buffer);
DAWN_TRY_CONTEXT(
ValidateCanUseAs(buffer, wgpu::BufferUsage::CopyDst, mUsageValidationMode),
"validating buffer %s usage.", buffer);

// Size must be a multiple of 4 bytes on macOS.
DAWN_INVALID_IF(size % 4 != 0, "Fill size (%u) is not a multiple of 4 bytes.",
Expand Down Expand Up @@ -1967,7 +1974,8 @@ void CommandEncoder::APIResolveQuerySet(QuerySetBase* querySet,
DAWN_TRY(ValidateQuerySetResolve(querySet, firstQuery, queryCount, destination,
destinationOffset));

DAWN_TRY(ValidateCanUseAs(destination, wgpu::BufferUsage::QueryResolve));
DAWN_TRY(ValidateCanUseAs(destination, wgpu::BufferUsage::QueryResolve,
mUsageValidationMode));

TrackUsedQuerySet(querySet);
}
Expand Down Expand Up @@ -2008,7 +2016,8 @@ void CommandEncoder::APIWriteBuffer(BufferBase* buffer,
this,
[&](CommandAllocator* allocator) -> MaybeError {
if (GetDevice()->IsValidationEnabled()) {
DAWN_TRY(ValidateWriteBuffer(GetDevice(), buffer, bufferOffset, size));
DAWN_TRY(ValidateWriteBuffer(GetDevice(), buffer, bufferOffset, size,
mUsageValidationMode));
}

WriteBufferCmd* cmd = allocator->Allocate<WriteBufferCmd>(Command::WriteBuffer);
Expand Down
31 changes: 18 additions & 13 deletions src/dawn/native/CommandValidation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,8 @@ MaybeError ValidatePassTimestampWrites(const DeviceBase* device,
MaybeError ValidateWriteBuffer(const DeviceBase* device,
const BufferBase* buffer,
uint64_t bufferOffset,
uint64_t size) {
uint64_t size,
UsageValidationMode mode) {
DAWN_TRY(device->ValidateObject(buffer));

DAWN_INVALID_IF(bufferOffset % 4 != 0, "BufferOffset (%u) is not a multiple of 4.",
Expand All @@ -236,7 +237,7 @@ MaybeError ValidateWriteBuffer(const DeviceBase* device,
"Write range (bufferOffset: %u, size: %u) does not fit in %s size (%u).",
bufferOffset, size, buffer, bufferSize);

DAWN_TRY(ValidateCanUseAs(buffer, wgpu::BufferUsage::CopyDst));
DAWN_TRY(ValidateCanUseAs(buffer, wgpu::BufferUsage::CopyDst, mode));

return {};
}
Expand Down Expand Up @@ -648,17 +649,21 @@ MaybeError ValidateCanUseAs(const TextureBase* texture,
return {};
}

MaybeError ValidateCanUseAs(const BufferBase* buffer, wgpu::BufferUsage usage) {
DAWN_ASSERT(wgpu::HasZeroOrOneBits(usage));
DAWN_INVALID_IF(!(buffer->GetUsage() & usage), "%s usage (%s) doesn't include %s.", buffer,
buffer->GetUsage(), usage);
return {};
}

MaybeError ValidateCanUseAsInternal(const BufferBase* buffer, wgpu::BufferUsage usage) {
DAWN_INVALID_IF(!(buffer->GetInternalUsage() & usage),
"%s internal usage (%s) doesn't include %s.", buffer,
buffer->GetInternalUsage(), usage);
MaybeError ValidateCanUseAs(const BufferBase* buffer,
wgpu::BufferUsage usage,
UsageValidationMode mode) {
switch (mode) {
case UsageValidationMode::Default:
DAWN_ASSERT(wgpu::HasZeroOrOneBits(usage));
DAWN_INVALID_IF(!(buffer->GetUsage() & usage), "%s usage (%s) doesn't include %s.",
buffer, buffer->GetUsage(), usage);
break;
case UsageValidationMode::Internal:
DAWN_INVALID_IF(!(buffer->GetInternalUsage() & usage),
"%s internal usage (%s) doesn't include %s.", buffer,
buffer->GetInternalUsage(), usage);
break;
}
return {};
}

Expand Down
8 changes: 5 additions & 3 deletions src/dawn/native/CommandValidation.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ MaybeError ValidatePassTimestampWrites(const DeviceBase* device,
MaybeError ValidateWriteBuffer(const DeviceBase* device,
const BufferBase* buffer,
uint64_t bufferOffset,
uint64_t size);
uint64_t size,
UsageValidationMode mode);

template <typename A, typename B>
DAWN_FORCE_INLINE uint64_t Safe32x32(A a, B b) {
Expand Down Expand Up @@ -113,8 +114,9 @@ MaybeError ValidateTextureToTextureCopyRestrictions(DeviceBase const* device,
MaybeError ValidateCanUseAs(const TextureBase* texture,
wgpu::TextureUsage usage,
UsageValidationMode mode);
MaybeError ValidateCanUseAs(const BufferBase* buffer, wgpu::BufferUsage usage);
MaybeError ValidateCanUseAsInternal(const BufferBase* buffer, wgpu::BufferUsage usage);
MaybeError ValidateCanUseAs(const BufferBase* buffer,
wgpu::BufferUsage usage,
UsageValidationMode mode);

using ColorAttachmentFormats = absl::InlinedVector<const Format*, kMaxColorAttachments>;
MaybeError ValidateColorAttachmentBytesPerSample(DeviceBase* device,
Expand Down
5 changes: 4 additions & 1 deletion src/dawn/native/ComputePassEncoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,10 @@ void ComputePassEncoder::APIDispatchWorkgroupsIndirect(BufferBase* indirectBuffe
[&](CommandAllocator* allocator) -> MaybeError {
if (IsValidationEnabled()) {
DAWN_TRY(GetDevice()->ValidateObject(indirectBuffer));
DAWN_TRY(ValidateCanUseAs(indirectBuffer, wgpu::BufferUsage::Indirect));
// TODO(chromium:42240463): validate indirectBuffer usage with the usage validation
// mode of mCommandEncoder
DAWN_TRY(ValidateCanUseAs(indirectBuffer, wgpu::BufferUsage::Indirect,
UsageValidationMode::Default));
DAWN_TRY(mCommandBufferState.ValidateCanDispatch());

DAWN_INVALID_IF(indirectOffset % 4 != 0,
Expand Down
6 changes: 4 additions & 2 deletions src/dawn/native/IndirectDrawValidationEncoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,8 @@ MaybeError EncodeIndirectDrawValidationCommands(DeviceBase* device,
outputParamsBinding.size = batch.outputParamsSize;

Ref<BindGroupBase> bindGroup;
DAWN_TRY_ASSIGN(bindGroup, device->CreateBindGroup(&bindGroupDescriptor));
DAWN_TRY_ASSIGN(bindGroup, device->CreateBindGroup(&bindGroupDescriptor,
UsageValidationMode::Internal));

const uint32_t numDrawsRoundedUp =
(batch.batchInfo->numDraws + kWorkgroupSize - 1) / kWorkgroupSize;
Expand Down Expand Up @@ -846,7 +847,8 @@ MaybeError EncodeIndirectDrawValidationCommands(DeviceBase* device,
}

Ref<BindGroupBase> bindGroup;
DAWN_TRY_ASSIGN(bindGroup, device->CreateBindGroup(&bindGroupDescriptor));
DAWN_TRY_ASSIGN(bindGroup, device->CreateBindGroup(&bindGroupDescriptor,
UsageValidationMode::Internal));

commandEncoder->APIWriteBuffer(drawConstantsBuffer.GetBuffer(), 0,
reinterpret_cast<const uint8_t*>(&drawConstants),
Expand Down
3 changes: 2 additions & 1 deletion src/dawn/native/Queue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,8 @@ MaybeError QueueBase::WriteBuffer(BufferBase* buffer,
size_t size) {
DAWN_TRY(GetDevice()->ValidateIsAlive());
DAWN_TRY(GetDevice()->ValidateObject(this));
DAWN_TRY(ValidateWriteBuffer(GetDevice(), buffer, bufferOffset, size));
DAWN_TRY(
ValidateWriteBuffer(GetDevice(), buffer, bufferOffset, size, UsageValidationMode::Default));
DAWN_TRY(buffer->ValidateCanUseOnQueueNow());
return WriteBufferImpl(buffer, bufferOffset, data, size);
}
Expand Down
Loading

0 comments on commit 1f3f5df

Please sign in to comment.