encounter
/
dawn-cmake
mirror of https://github.com/encounter/dawn-cmake.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Improve validation errors for encoders 

Improves the validation messages in ComputePassEncoder.cpp,
ProgrammablePassEncoder.cpp, RenderBundleEncoder.cpp, and
EncodingContext.cpp/h to give them more contextual information.

Bug: dawn:563
Change-Id: I87c46c4bfda1375809fae93239029ea4e3b9c0a2
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/67000
Commit-Queue: Brandon Jones <bajones@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
This commit is contained in:
Brandon Jones 2021-10-20 17:13:48 +00:00 committed by Dawn LUCI CQ
parent aa8fcfc64b
commit c7e6bb0d8d
6 changed files with 88 additions and 81 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/dawn_native/CommandEncoder.cpp
View File

@ -891,8 +891,7 @@ namespace dawn_native {
if (GetDevice()->IsValidationEnabled()) { if (GetDevice()->IsValidationEnabled()) {
DAWN_INVALID_IF( DAWN_INVALID_IF(
mDebugGroupStackSize == 0, mDebugGroupStackSize == 0,
"Every call to PopDebugGroup must be balanced by a corresponding call to " "PopDebugGroup called when no debug groups are currently pushed.");
"PushDebugGroup.");
} }
allocator->Allocate<PopDebugGroupCmd>(Command::PopDebugGroup); allocator->Allocate<PopDebugGroupCmd>(Command::PopDebugGroup);
mDebugGroupStackSize--; mDebugGroupStackSize--;

58
src/dawn_native/ComputePassEncoder.cpp
View File

@ -26,18 +26,6 @@
namespace dawn_native { namespace dawn_native {
namespace {
MaybeError ValidatePerDimensionDispatchSizeLimit(const DeviceBase* device, uint32_t size) {
if (size > device->GetLimits().v1.maxComputeWorkgroupsPerDimension) {
return DAWN_VALIDATION_ERROR("Dispatch size exceeds defined limits");
}
return {};
}
} // namespace
ComputePassEncoder::ComputePassEncoder(DeviceBase* device, ComputePassEncoder::ComputePassEncoder(DeviceBase* device,
CommandEncoder* commandEncoder, CommandEncoder* commandEncoder,
EncodingContext* encodingContext) EncodingContext* encodingContext)
@ -85,9 +73,24 @@ namespace dawn_native {
[&](CommandAllocator* allocator) -> MaybeError { [&](CommandAllocator* allocator) -> MaybeError {
if (IsValidationEnabled()) { if (IsValidationEnabled()) {
DAWN_TRY(mCommandBufferState.ValidateCanDispatch()); DAWN_TRY(mCommandBufferState.ValidateCanDispatch());
DAWN_TRY(ValidatePerDimensionDispatchSizeLimit(GetDevice(), x));
DAWN_TRY(ValidatePerDimensionDispatchSizeLimit(GetDevice(), y)); uint32_t workgroupsPerDimension =
DAWN_TRY(ValidatePerDimensionDispatchSizeLimit(GetDevice(), z)); GetDevice()->GetLimits().v1.maxComputeWorkgroupsPerDimension;
DAWN_INVALID_IF(
x > workgroupsPerDimension,
"Dispatch size X (%u) exceeds max compute workgroups per dimension (%u).",
x, workgroupsPerDimension);
DAWN_INVALID_IF(
y > workgroupsPerDimension,
"Dispatch size Y (%u) exceeds max compute workgroups per dimension (%u).",
y, workgroupsPerDimension);
DAWN_INVALID_IF(
z > workgroupsPerDimension,
"Dispatch size Z (%u) exceeds max compute workgroups per dimension (%u).",
z, workgroupsPerDimension);
} }
// Record the synchronization scope for Dispatch, which is just the current // Record the synchronization scope for Dispatch, which is just the current
@ -117,21 +120,20 @@ namespace dawn_native {
// Indexed dispatches need a compute-shader based validation to check that the // Indexed dispatches need a compute-shader based validation to check that the
// dispatch sizes aren't too big. Disallow them as unsafe until the validation // dispatch sizes aren't too big. Disallow them as unsafe until the validation
// is implemented. // is implemented.
if (GetDevice()->IsToggleEnabled(Toggle::DisallowUnsafeAPIs)) { DAWN_INVALID_IF(
return DAWN_VALIDATION_ERROR( GetDevice()->IsToggleEnabled(Toggle::DisallowUnsafeAPIs),
"DispatchIndirect is disallowed because it doesn't validate that the " "DispatchIndirect is disallowed because it doesn't validate that the "
"dispatch " "dispatch size is valid yet.");
"size is valid yet.");
}
if (indirectOffset % 4 != 0) { DAWN_INVALID_IF(indirectOffset % 4 != 0,
return DAWN_VALIDATION_ERROR("Indirect offset must be a multiple of 4"); "Indirect offset (%u) is not a multiple of 4.", indirectOffset);
}
if (indirectOffset >= indirectBuffer->GetSize() || DAWN_INVALID_IF(
indirectOffset + kDispatchIndirectSize > indirectBuffer->GetSize()) { indirectOffset >= indirectBuffer->GetSize() ||
return DAWN_VALIDATION_ERROR("Indirect offset out of bounds"); indirectOffset + kDispatchIndirectSize > indirectBuffer->GetSize(),
} "Indirect offset (%u) and dispatch size (%u) exceeds the indirect buffer "
"size (%u).",
indirectOffset, kDispatchIndirectSize, indirectBuffer->GetSize());
} }
// Record the synchronization scope for Dispatch, both the bindgroups and the // Record the synchronization scope for Dispatch, both the bindgroups and the

21
src/dawn_native/EncodingContext.cpp
View File

@ -24,7 +24,7 @@
namespace dawn_native { namespace dawn_native {
EncodingContext::EncodingContext(DeviceBase* device, const ObjectBase* initialEncoder) EncodingContext::EncodingContext(DeviceBase* device, const ApiObjectBase* initialEncoder)
: mDevice(device), mTopLevelEncoder(initialEncoder), mCurrentEncoder(initialEncoder) { : mDevice(device), mTopLevelEncoder(initialEncoder), mCurrentEncoder(initialEncoder) {
} }
@ -87,7 +87,7 @@ namespace dawn_native {
} }
} }
void EncodingContext::EnterPass(const ObjectBase* passEncoder) { void EncodingContext::EnterPass(const ApiObjectBase* passEncoder) {
// Assert we're at the top level. // Assert we're at the top level.
ASSERT(mCurrentEncoder == mTopLevelEncoder); ASSERT(mCurrentEncoder == mTopLevelEncoder);
ASSERT(passEncoder != nullptr); ASSERT(passEncoder != nullptr);
@ -95,7 +95,7 @@ namespace dawn_native {
mCurrentEncoder = passEncoder; mCurrentEncoder = passEncoder;
} }
MaybeError EncodingContext::ExitRenderPass(const ObjectBase* passEncoder, MaybeError EncodingContext::ExitRenderPass(const ApiObjectBase* passEncoder,
RenderPassResourceUsageTracker usageTracker, RenderPassResourceUsageTracker usageTracker,
CommandEncoder* commandEncoder, CommandEncoder* commandEncoder,
IndirectDrawMetadata indirectDrawMetadata) { IndirectDrawMetadata indirectDrawMetadata) {
@ -121,7 +121,7 @@ namespace dawn_native {
return {}; return {};
} }
void EncodingContext::ExitComputePass(const ObjectBase* passEncoder, void EncodingContext::ExitComputePass(const ApiObjectBase* passEncoder,
ComputePassResourceUsage usages) { ComputePassResourceUsage usages) {
ASSERT(mCurrentEncoder != mTopLevelEncoder); ASSERT(mCurrentEncoder != mTopLevelEncoder);
ASSERT(mCurrentEncoder == passEncoder); ASSERT(mCurrentEncoder == passEncoder);
@ -161,12 +161,10 @@ namespace dawn_native {
} }
MaybeError EncodingContext::Finish() { MaybeError EncodingContext::Finish() {
if (IsFinished()) { DAWN_INVALID_IF(IsFinished(), "Command encoding already finished.");
return DAWN_VALIDATION_ERROR("Command encoding already finished");
}
const void* currentEncoder = mCurrentEncoder; const ApiObjectBase* currentEncoder = mCurrentEncoder;
const void* topLevelEncoder = mTopLevelEncoder; const ApiObjectBase* topLevelEncoder = mTopLevelEncoder;
// Even if finish validation fails, it is now invalid to call any encoding commands, // Even if finish validation fails, it is now invalid to call any encoding commands,
// so we clear the encoders. Note: mTopLevelEncoder == nullptr is used as a flag for // so we clear the encoders. Note: mTopLevelEncoder == nullptr is used as a flag for
@ -178,9 +176,8 @@ namespace dawn_native {
if (mError != nullptr) { if (mError != nullptr) {
return std::move(mError); return std::move(mError);
} }
if (currentEncoder != topLevelEncoder) { DAWN_INVALID_IF(currentEncoder != topLevelEncoder,
return DAWN_VALIDATION_ERROR("Command buffer recording ended mid-pass"); "Command buffer recording ended before %s was ended.", currentEncoder);
}
return {}; return {};
} }

27
src/dawn_native/EncodingContext.h
View File

@ -28,13 +28,13 @@ namespace dawn_native {
class CommandEncoder; class CommandEncoder;
class DeviceBase; class DeviceBase;
class ObjectBase; class ApiObjectBase;
// Base class for allocating/iterating commands. // Base class for allocating/iterating commands.
// It performs error tracking as well as encoding state for render/compute passes. // It performs error tracking as well as encoding state for render/compute passes.
class EncodingContext { class EncodingContext {
public: public:
EncodingContext(DeviceBase* device, const ObjectBase* initialEncoder); EncodingContext(DeviceBase* device, const ApiObjectBase* initialEncoder);
~EncodingContext(); ~EncodingContext();
CommandIterator AcquireCommands(); CommandIterator AcquireCommands();
@ -70,14 +70,15 @@ namespace dawn_native {
return false; return false;
} }
inline bool CheckCurrentEncoder(const ObjectBase* encoder) { inline bool CheckCurrentEncoder(const ApiObjectBase* encoder) {
if (DAWN_UNLIKELY(encoder != mCurrentEncoder)) { if (DAWN_UNLIKELY(encoder != mCurrentEncoder)) {
if (mCurrentEncoder != mTopLevelEncoder) { if (mCurrentEncoder != mTopLevelEncoder) {
// The top level encoder was used when a pass encoder was current. // The top level encoder was used when a pass encoder was current.
HandleError(DAWN_VALIDATION_ERROR("Command cannot be recorded inside a pass")); HandleError(DAWN_FORMAT_VALIDATION_ERROR(
"Command cannot be recorded while %s is active.", mCurrentEncoder));
} else { } else {
HandleError(DAWN_VALIDATION_ERROR( HandleError(DAWN_FORMAT_VALIDATION_ERROR(
"Recording in an error or already ended pass encoder")); "Recording in an error or already ended %s.", encoder));
} }
return false; return false;
} }
@ -85,7 +86,7 @@ namespace dawn_native {
} }
template <typename EncodeFunction> template <typename EncodeFunction>
inline bool TryEncode(const ObjectBase* encoder, EncodeFunction&& encodeFunction) { inline bool TryEncode(const ApiObjectBase* encoder, EncodeFunction&& encodeFunction) {
if (!CheckCurrentEncoder(encoder)) { if (!CheckCurrentEncoder(encoder)) {
return false; return false;
} }
@ -94,7 +95,7 @@ namespace dawn_native {
} }
template <typename EncodeFunction, typename... Args> template <typename EncodeFunction, typename... Args>
inline bool TryEncode(const ObjectBase* encoder, inline bool TryEncode(const ApiObjectBase* encoder,
EncodeFunction&& encodeFunction, EncodeFunction&& encodeFunction,
const char* formatStr, const char* formatStr,
const Args&... args) { const Args&... args) {
@ -111,12 +112,12 @@ namespace dawn_native {
void WillBeginRenderPass(); void WillBeginRenderPass();
// Functions to set current encoder state // Functions to set current encoder state
void EnterPass(const ObjectBase* passEncoder); void EnterPass(const ApiObjectBase* passEncoder);
MaybeError ExitRenderPass(const ObjectBase* passEncoder, MaybeError ExitRenderPass(const ApiObjectBase* passEncoder,
RenderPassResourceUsageTracker usageTracker, RenderPassResourceUsageTracker usageTracker,
CommandEncoder* commandEncoder, CommandEncoder* commandEncoder,
IndirectDrawMetadata indirectDrawMetadata); IndirectDrawMetadata indirectDrawMetadata);
void ExitComputePass(const ObjectBase* passEncoder, ComputePassResourceUsage usages); void ExitComputePass(const ApiObjectBase* passEncoder, ComputePassResourceUsage usages);
MaybeError Finish(); MaybeError Finish();
const RenderPassUsages& GetRenderPassUsages() const; const RenderPassUsages& GetRenderPassUsages() const;
@ -138,12 +139,12 @@ namespace dawn_native {
// There can only be two levels of encoders. Top-level and render/compute pass. // There can only be two levels of encoders. Top-level and render/compute pass.
// The top level encoder is the encoder the EncodingContext is created with. // The top level encoder is the encoder the EncodingContext is created with.
// It doubles as flag to check if encoding has been Finished. // It doubles as flag to check if encoding has been Finished.
const ObjectBase* mTopLevelEncoder; const ApiObjectBase* mTopLevelEncoder;
// The current encoder must be the same as the encoder provided to TryEncode, // The current encoder must be the same as the encoder provided to TryEncode,
// otherwise an error is produced. It may be nullptr if the EncodingContext is an error. // otherwise an error is produced. It may be nullptr if the EncodingContext is an error.
// The current encoder changes with Enter/ExitPass which should be called by // The current encoder changes with Enter/ExitPass which should be called by
// CommandEncoder::Begin/EndPass. // CommandEncoder::Begin/EndPass.
const ObjectBase* mCurrentEncoder; const ApiObjectBase* mCurrentEncoder;
RenderPassUsages mRenderPassUsages; RenderPassUsages mRenderPassUsages;
bool mWereRenderPassUsagesAcquired = false; bool mWereRenderPassUsagesAcquired = false;

57
src/dawn_native/ProgrammablePassEncoder.cpp
View File

@ -48,9 +48,9 @@ namespace dawn_native {
} }
MaybeError ProgrammablePassEncoder::ValidateProgrammableEncoderEnd() const { MaybeError ProgrammablePassEncoder::ValidateProgrammableEncoderEnd() const {
if (mDebugGroupStackSize != 0) { DAWN_INVALID_IF(mDebugGroupStackSize != 0,
return DAWN_VALIDATION_ERROR("Each Push must be balanced by a corresponding Pop."); "PushDebugGroup called %u time(s) without a corresponding PopDebugGroup.",
} mDebugGroupStackSize);
return {}; return {};
} }
@ -75,10 +75,9 @@ namespace dawn_native {
this, this,
[&](CommandAllocator* allocator) -> MaybeError { [&](CommandAllocator* allocator) -> MaybeError {
if (IsValidationEnabled()) { if (IsValidationEnabled()) {
if (mDebugGroupStackSize == 0) { DAWN_INVALID_IF(
return DAWN_VALIDATION_ERROR( mDebugGroupStackSize == 0,
"Pop must be balanced by a corresponding Push."); "PopDebugGroup called when no debug groups are currently pushed.");
}
} }
allocator->Allocate<PopDebugGroupCmd>(Command::PopDebugGroup); allocator->Allocate<PopDebugGroupCmd>(Command::PopDebugGroup);
mDebugGroupStackSize--; mDebugGroupStackSize--;
@ -115,18 +114,21 @@ namespace dawn_native {
const uint32_t* dynamicOffsetsIn) const { const uint32_t* dynamicOffsetsIn) const {
DAWN_TRY(GetDevice()->ValidateObject(group)); DAWN_TRY(GetDevice()->ValidateObject(group));
if (index >= kMaxBindGroupsTyped) { DAWN_INVALID_IF(index >= kMaxBindGroupsTyped,
return DAWN_VALIDATION_ERROR("Setting bind group over the max"); "Bind group index (%u) exceeds the maximum (%u).",
} static_cast<uint32_t>(index), kMaxBindGroups);
ityp::span<BindingIndex, const uint32_t> dynamicOffsets(dynamicOffsetsIn, ityp::span<BindingIndex, const uint32_t> dynamicOffsets(dynamicOffsetsIn,
BindingIndex(dynamicOffsetCountIn)); BindingIndex(dynamicOffsetCountIn));
// Dynamic offsets count must match the number required by the layout perfectly. // Dynamic offsets count must match the number required by the layout perfectly.
const BindGroupLayoutBase* layout = group->GetLayout(); const BindGroupLayoutBase* layout = group->GetLayout();
if (layout->GetDynamicBufferCount() != dynamicOffsets.size()) { DAWN_INVALID_IF(
return DAWN_VALIDATION_ERROR("dynamicOffset count mismatch"); layout->GetDynamicBufferCount() != dynamicOffsets.size(),
} "The number of dynamic offsets (%u) does not match the number of dynamic buffers (%u) "
"in %s.",
static_cast<uint32_t>(dynamicOffsets.size()),
static_cast<uint32_t>(layout->GetDynamicBufferCount()), layout);
for (BindingIndex i{0}; i < dynamicOffsets.size(); ++i) { for (BindingIndex i{0}; i < dynamicOffsets.size(); ++i) {
const BindingInfo& bindingInfo = layout->GetBindingInfo(i); const BindingInfo& bindingInfo = layout->GetBindingInfo(i);
@ -150,9 +152,9 @@ namespace dawn_native {
UNREACHABLE(); UNREACHABLE();
} }
if (!IsAligned(dynamicOffsets[i], requiredAlignment)) { DAWN_INVALID_IF(!IsAligned(dynamicOffsets[i], requiredAlignment),
return DAWN_VALIDATION_ERROR("Dynamic Buffer Offset need to be aligned"); "Dynamic Offset[%u] (%u) is not %u byte aligned.",
} static_cast<uint32_t>(i), dynamicOffsets[i], requiredAlignment);
BufferBinding bufferBinding = group->GetBindingAsBufferBinding(i); BufferBinding bufferBinding = group->GetBindingAsBufferBinding(i);
@ -163,15 +165,20 @@ namespace dawn_native {
if ((dynamicOffsets[i] > if ((dynamicOffsets[i] >
bufferBinding.buffer->GetSize() - bufferBinding.offset - bufferBinding.size)) { bufferBinding.buffer->GetSize() - bufferBinding.offset - bufferBinding.size)) {
if ((bufferBinding.buffer->GetSize() - bufferBinding.offset) == DAWN_INVALID_IF(
bufferBinding.size) { (bufferBinding.buffer->GetSize() - bufferBinding.offset) == bufferBinding.size,
return DAWN_VALIDATION_ERROR( "Dynamic Offset[%u] (%u) is out of bounds of %s with a size of %u and a bound "
"Dynamic offset out of bounds. The binding goes to the end of the " "range of (offset: %u, size: %u). The binding goes to the end of the buffer "
"buffer even with a dynamic offset of 0. Did you forget to specify " "even with a dynamic offset of 0. Did you forget to specify "
"the binding's size?"); "the binding's size?",
} else { static_cast<uint32_t>(i), dynamicOffsets[i], bufferBinding.buffer,
return DAWN_VALIDATION_ERROR("Dynamic offset out of bounds"); bufferBinding.buffer->GetSize(), bufferBinding.offset, bufferBinding.size);
}
return DAWN_FORMAT_VALIDATION_ERROR(
"Dynamic Offset[%u] (%u) is out of bounds of "
"%s with a size of %u and a bound range of (offset: %u, size: %u).",
static_cast<uint32_t>(i), dynamicOffsets[i], bufferBinding.buffer,
bufferBinding.buffer->GetSize(), bufferBinding.offset, bufferBinding.size);
} }
} }

3
src/dawn_native/RenderBundleEncoder.cpp
View File

@ -123,7 +123,8 @@ namespace dawn_native {
RenderBundleBase* RenderBundleEncoder::APIFinish(const RenderBundleDescriptor* descriptor) { RenderBundleBase* RenderBundleEncoder::APIFinish(const RenderBundleDescriptor* descriptor) {
RenderBundleBase* result = nullptr; RenderBundleBase* result = nullptr;
if (GetDevice()->ConsumedError(FinishImpl(descriptor), &result)) { if (GetDevice()->ConsumedError(FinishImpl(descriptor), &result, "calling Finish(%s).",
descriptor)) {
return RenderBundleBase::MakeError(GetDevice()); return RenderBundleBase::MakeError(GetDevice());
} }