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

Revert "Implement readonly storage buffer - validation at API side" 

This reverts commit 34ac535f02.

Reason for revert: Failing Dawn into Chromium roll due to failed ASSERT
https://ci.chromium.org/p/chromium/builders/try/dawn-linux-x64-deps-rel/3514

Original change's description:
> Implement readonly storage buffer - validation at API side
> 
> This patch adds validation code for API side for readonly storage
> buffer. It also adds unit tests for API validation.
> 
> BUG=dawn:180
> 
> Change-Id: I9a97c5f3aa23e720619d138ca55d7b17f08d64c9
> Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/12620
> Reviewed-by: Austin Eng <enga@chromium.org>
> Commit-Queue: Yunchao He <yunchao.he@intel.com>

TBR=cwallez@chromium.org,kainino@chromium.org,yunchao.he@intel.com,enga@chromium.org

Change-Id: I64e5dfd40e7aab982d97fb48544180bc9b8ea558
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: dawn:180
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/12981
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
This commit is contained in:
Austin Eng 2019-11-01 01:09:51 +00:00 committed by Commit Bot service account
parent 34ac535f02
commit ec9010eabc
7 changed files with 8 additions and 135 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/dawn_native/BindGroup.cpp
View File

@ -147,7 +147,6 @@ namespace dawn_native {
DAWN_TRY(ValidateBufferBinding(device, binding, wgpu::BufferUsage::Uniform)); DAWN_TRY(ValidateBufferBinding(device, binding, wgpu::BufferUsage::Uniform));
break; break;
case wgpu::BindingType::StorageBuffer: case wgpu::BindingType::StorageBuffer:
case wgpu::BindingType::ReadonlyStorageBuffer:
DAWN_TRY(ValidateBufferBinding(device, binding, wgpu::BufferUsage::Storage)); DAWN_TRY(ValidateBufferBinding(device, binding, wgpu::BufferUsage::Storage));
break; break;
case wgpu::BindingType::SampledTexture: case wgpu::BindingType::SampledTexture:
@ -160,6 +159,7 @@ namespace dawn_native {
DAWN_TRY(ValidateSamplerBinding(device, binding)); DAWN_TRY(ValidateSamplerBinding(device, binding));
break; break;
case wgpu::BindingType::StorageTexture: case wgpu::BindingType::StorageTexture:
case wgpu::BindingType::ReadonlyStorageBuffer:
UNREACHABLE(); UNREACHABLE();
break; break;
} }
@ -231,9 +231,7 @@ namespace dawn_native {
ASSERT(binding < kMaxBindingsPerGroup); ASSERT(binding < kMaxBindingsPerGroup);
ASSERT(mLayout->GetBindingInfo().mask[binding]); ASSERT(mLayout->GetBindingInfo().mask[binding]);
ASSERT(mLayout->GetBindingInfo().types[binding] == wgpu::BindingType::UniformBuffer || ASSERT(mLayout->GetBindingInfo().types[binding] == wgpu::BindingType::UniformBuffer ||
mLayout->GetBindingInfo().types[binding] == wgpu::BindingType::StorageBuffer || mLayout->GetBindingInfo().types[binding] == wgpu::BindingType::StorageBuffer);
mLayout->GetBindingInfo().types[binding] ==
wgpu::BindingType::ReadonlyStorageBuffer);
BufferBase* buffer = static_cast<BufferBase*>(mBindings[binding].Get()); BufferBase* buffer = static_cast<BufferBase*>(mBindings[binding].Get());
return {buffer, mOffsets[binding], mSizes[binding]}; return {buffer, mOffsets[binding], mSizes[binding]};
} }

5
src/dawn_native/BindGroupLayout.cpp
View File

@ -56,7 +56,6 @@ namespace dawn_native {
} }
break; break;
case wgpu::BindingType::StorageBuffer: case wgpu::BindingType::StorageBuffer:
case wgpu::BindingType::ReadonlyStorageBuffer:
if (binding.hasDynamicOffset) { if (binding.hasDynamicOffset) {
++dynamicStorageBufferCount; ++dynamicStorageBufferCount;
} }
@ -67,6 +66,8 @@ namespace dawn_native {
return DAWN_VALIDATION_ERROR("Samplers and textures cannot be dynamic"); return DAWN_VALIDATION_ERROR("Samplers and textures cannot be dynamic");
} }
break; break;
case wgpu::BindingType::ReadonlyStorageBuffer:
return DAWN_VALIDATION_ERROR("readonly storage buffers aren't supported (yet)");
case wgpu::BindingType::StorageTexture: case wgpu::BindingType::StorageTexture:
return DAWN_VALIDATION_ERROR("storage textures aren't supported (yet)"); return DAWN_VALIDATION_ERROR("storage textures aren't supported (yet)");
} }
@ -150,11 +151,11 @@ namespace dawn_native {
++mDynamicUniformBufferCount; ++mDynamicUniformBufferCount;
break; break;
case wgpu::BindingType::StorageBuffer: case wgpu::BindingType::StorageBuffer:
case wgpu::BindingType::ReadonlyStorageBuffer:
++mDynamicStorageBufferCount; ++mDynamicStorageBufferCount;
break; break;
case wgpu::BindingType::SampledTexture: case wgpu::BindingType::SampledTexture:
case wgpu::BindingType::Sampler: case wgpu::BindingType::Sampler:
case wgpu::BindingType::ReadonlyStorageBuffer:
case wgpu::BindingType::StorageTexture: case wgpu::BindingType::StorageTexture:
UNREACHABLE(); UNREACHABLE();
break; break;

6
src/dawn_native/Buffer.cpp
View File

@ -115,12 +115,6 @@ namespace dawn_native {
mSize(descriptor->size), mSize(descriptor->size),
mUsage(descriptor->usage), mUsage(descriptor->usage),
mState(BufferState::Unmapped) { mState(BufferState::Unmapped) {
// Add readonly storage usage if the buffer has a storage usage. The validation rules in
// PassResourceUsageTracker::ValidateUsages will make sure we don't use both at the same
// time.
if (mUsage & wgpu::BufferUsage::Storage) {
mUsage |= kReadOnlyStorage;
}
} }
BufferBase::BufferBase(DeviceBase* device, ObjectBase::ErrorTag tag) BufferBase::BufferBase(DeviceBase* device, ObjectBase::ErrorTag tag)

6
src/dawn_native/Buffer.h
View File

@ -27,13 +27,9 @@ namespace dawn_native {
MaybeError ValidateBufferDescriptor(DeviceBase* device, const BufferDescriptor* descriptor); MaybeError ValidateBufferDescriptor(DeviceBase* device, const BufferDescriptor* descriptor);
// Add an extra buffer usage (readonly storage buffer usage) for render pass resource tracking
static constexpr wgpu::BufferUsage kReadOnlyStorage =
static_cast<wgpu::BufferUsage>(0x80000000);
static constexpr wgpu::BufferUsage kReadOnlyBufferUsages = static constexpr wgpu::BufferUsage kReadOnlyBufferUsages =
wgpu::BufferUsage::MapRead | wgpu::BufferUsage::CopySrc | wgpu::BufferUsage::Index | wgpu::BufferUsage::MapRead | wgpu::BufferUsage::CopySrc | wgpu::BufferUsage::Index |
wgpu::BufferUsage::Vertex | wgpu::BufferUsage::Uniform | kReadOnlyStorage; wgpu::BufferUsage::Vertex | wgpu::BufferUsage::Uniform;
static constexpr wgpu::BufferUsage kWritableBufferUsages = static constexpr wgpu::BufferUsage kWritableBufferUsages =
wgpu::BufferUsage::MapWrite | wgpu::BufferUsage::CopyDst | wgpu::BufferUsage::Storage; wgpu::BufferUsage::MapWrite | wgpu::BufferUsage::CopyDst | wgpu::BufferUsage::Storage;

7
src/dawn_native/CommandValidation.cpp
View File

@ -16,7 +16,6 @@
#include "common/BitSetIterator.h" #include "common/BitSetIterator.h"
#include "dawn_native/BindGroup.h" #include "dawn_native/BindGroup.h"
#include "dawn_native/Buffer.h"
#include "dawn_native/CommandBufferStateTracker.h" #include "dawn_native/CommandBufferStateTracker.h"
#include "dawn_native/Commands.h" #include "dawn_native/Commands.h"
#include "dawn_native/PassResourceUsageTracker.h" #include "dawn_native/PassResourceUsageTracker.h"
@ -50,15 +49,11 @@ namespace dawn_native {
usageTracker->TextureUsedAs(texture, wgpu::TextureUsage::Sampled); usageTracker->TextureUsedAs(texture, wgpu::TextureUsage::Sampled);
} break; } break;
case wgpu::BindingType::ReadonlyStorageBuffer: {
BufferBase* buffer = group->GetBindingAsBufferBinding(i).buffer;
usageTracker->BufferUsedAs(buffer, kReadOnlyStorage);
} break;
case wgpu::BindingType::Sampler: case wgpu::BindingType::Sampler:
break; break;
case wgpu::BindingType::StorageTexture: case wgpu::BindingType::StorageTexture:
case wgpu::BindingType::ReadonlyStorageBuffer:
UNREACHABLE(); UNREACHABLE();
break; break;
} }

64
src/tests/unittests/validation/BindGroupValidationTests.cpp
View File

@ -372,18 +372,6 @@ TEST_F(BindGroupValidationTest, BufferUsageSSBO) {
ASSERT_DEVICE_ERROR(utils::MakeBindGroup(device, layout, {{0, mUBO, 0, 256}})); ASSERT_DEVICE_ERROR(utils::MakeBindGroup(device, layout, {{0, mUBO, 0, 256}}));
} }
// Check that a readonly SSBO must have the correct usage
TEST_F(BindGroupValidationTest, BufferUsageReadonlySSBO) {
wgpu::BindGroupLayout layout = utils::MakeBindGroupLayout(
device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::ReadonlyStorageBuffer}});
// Control case: using a buffer with the storage usage works
utils::MakeBindGroup(device, layout, {{0, mSSBO, 0, 256}});
// Using a buffer without the storage usage fails
ASSERT_DEVICE_ERROR(utils::MakeBindGroup(device, layout, {{0, mUBO, 0, 256}}));
}
// Tests constraints on the buffer offset for bind groups. // Tests constraints on the buffer offset for bind groups.
TEST_F(BindGroupValidationTest, BufferOffsetAlignment) { TEST_F(BindGroupValidationTest, BufferOffsetAlignment) {
wgpu::BindGroupLayout layout = utils::MakeBindGroupLayout( wgpu::BindGroupLayout layout = utils::MakeBindGroupLayout(
@ -527,11 +515,6 @@ TEST_F(BindGroupLayoutValidationTest, DynamicAndTypeCompatibility) {
{0, wgpu::ShaderStage::Compute, wgpu::BindingType::StorageBuffer, true}, {0, wgpu::ShaderStage::Compute, wgpu::BindingType::StorageBuffer, true},
}); });
utils::MakeBindGroupLayout(
device, {
{0, wgpu::ShaderStage::Compute, wgpu::BindingType::ReadonlyStorageBuffer, true},
});
ASSERT_DEVICE_ERROR(utils::MakeBindGroupLayout( ASSERT_DEVICE_ERROR(utils::MakeBindGroupLayout(
device, { device, {
{0, wgpu::ShaderStage::Compute, wgpu::BindingType::SampledTexture, true}, {0, wgpu::ShaderStage::Compute, wgpu::BindingType::SampledTexture, true},
@ -563,7 +546,6 @@ TEST_F(BindGroupLayoutValidationTest, DynamicBufferNumberLimit) {
wgpu::BindGroupLayout bgl[2]; wgpu::BindGroupLayout bgl[2];
std::vector<wgpu::BindGroupLayoutBinding> maxUniformDB; std::vector<wgpu::BindGroupLayoutBinding> maxUniformDB;
std::vector<wgpu::BindGroupLayoutBinding> maxStorageDB; std::vector<wgpu::BindGroupLayoutBinding> maxStorageDB;
std::vector<wgpu::BindGroupLayoutBinding> maxReadonlyStorageDB;
for (uint32_t i = 0; i < kMaxDynamicUniformBufferCount; ++i) { for (uint32_t i = 0; i < kMaxDynamicUniformBufferCount; ++i) {
maxUniformDB.push_back( maxUniformDB.push_back(
@ -575,11 +557,6 @@ TEST_F(BindGroupLayoutValidationTest, DynamicBufferNumberLimit) {
{i, wgpu::ShaderStage::Compute, wgpu::BindingType::StorageBuffer, true}); {i, wgpu::ShaderStage::Compute, wgpu::BindingType::StorageBuffer, true});
} }
for (uint32_t i = 0; i < kMaxDynamicStorageBufferCount; ++i) {
maxReadonlyStorageDB.push_back(
{i, wgpu::ShaderStage::Compute, wgpu::BindingType::ReadonlyStorageBuffer, true});
}
auto MakeBindGroupLayout = [&](wgpu::BindGroupLayoutBinding* binding, auto MakeBindGroupLayout = [&](wgpu::BindGroupLayoutBinding* binding,
uint32_t count) -> wgpu::BindGroupLayout { uint32_t count) -> wgpu::BindGroupLayout {
wgpu::BindGroupLayoutDescriptor descriptor; wgpu::BindGroupLayoutDescriptor descriptor;
@ -595,14 +572,7 @@ TEST_F(BindGroupLayoutValidationTest, DynamicBufferNumberLimit) {
TestCreatePipelineLayout(bgl, 2, true); TestCreatePipelineLayout(bgl, 2, true);
} }
{ // Check dynamic uniform buffers excedd maximum in pipeline layout.
bgl[0] = MakeBindGroupLayout(maxUniformDB.data(), maxUniformDB.size());
bgl[1] = MakeBindGroupLayout(maxReadonlyStorageDB.data(), maxReadonlyStorageDB.size());
TestCreatePipelineLayout(bgl, 2, true);
}
// Check dynamic uniform buffers exceed maximum in pipeline layout.
{ {
bgl[0] = MakeBindGroupLayout(maxUniformDB.data(), maxUniformDB.size()); bgl[0] = MakeBindGroupLayout(maxUniformDB.data(), maxUniformDB.size());
bgl[1] = utils::MakeBindGroupLayout( bgl[1] = utils::MakeBindGroupLayout(
@ -624,31 +594,6 @@ TEST_F(BindGroupLayoutValidationTest, DynamicBufferNumberLimit) {
TestCreatePipelineLayout(bgl, 2, false); TestCreatePipelineLayout(bgl, 2, false);
} }
// Check dynamic readonly storage buffers exceed maximum in pipeline layout
{
bgl[0] = MakeBindGroupLayout(maxReadonlyStorageDB.data(), maxReadonlyStorageDB.size());
bgl[1] = utils::MakeBindGroupLayout(
device,
{
{0, wgpu::ShaderStage::Compute, wgpu::BindingType::ReadonlyStorageBuffer, true},
});
TestCreatePipelineLayout(bgl, 2, false);
}
// Check dynamic storage buffers + dynamic readonly storage buffers exceed maximum storage
// buffers in pipeline layout
{
bgl[0] = MakeBindGroupLayout(maxStorageDB.data(), maxStorageDB.size());
bgl[1] = utils::MakeBindGroupLayout(
device,
{
{0, wgpu::ShaderStage::Compute, wgpu::BindingType::ReadonlyStorageBuffer, true},
});
TestCreatePipelineLayout(bgl, 2, false);
}
// Check dynamic uniform buffers exceed maximum in bind group layout. // Check dynamic uniform buffers exceed maximum in bind group layout.
{ {
maxUniformDB.push_back({kMaxDynamicUniformBufferCount, wgpu::ShaderStage::Compute, maxUniformDB.push_back({kMaxDynamicUniformBufferCount, wgpu::ShaderStage::Compute,
@ -662,13 +607,6 @@ TEST_F(BindGroupLayoutValidationTest, DynamicBufferNumberLimit) {
wgpu::BindingType::StorageBuffer, true}); wgpu::BindingType::StorageBuffer, true});
TestCreateBindGroupLayout(maxStorageDB.data(), maxStorageDB.size(), false); TestCreateBindGroupLayout(maxStorageDB.data(), maxStorageDB.size(), false);
} }
// Check dynamic readonly storage buffers exceed maximum in bind group layout.
{
maxReadonlyStorageDB.push_back({kMaxDynamicStorageBufferCount, wgpu::ShaderStage::Compute,
wgpu::BindingType::ReadonlyStorageBuffer, true});
TestCreateBindGroupLayout(maxReadonlyStorageDB.data(), maxReadonlyStorageDB.size(), false);
}
} }
constexpr uint64_t kBufferSize = 2 * kMinDynamicBufferOffsetAlignment + 8; constexpr uint64_t kBufferSize = 2 * kMinDynamicBufferOffsetAlignment + 8;

49
src/tests/unittests/validation/CommandBufferValidationTests.cpp
View File

@ -223,55 +223,6 @@ TEST_F(CommandBufferValidationTest, BufferWithReadAndWriteUsage) {
ASSERT_DEVICE_ERROR(encoder.Finish()); ASSERT_DEVICE_ERROR(encoder.Finish());
} }
// Test that using a single buffer in multiple read usages which include readonly storage usage in
// the same pass is allowed.
TEST_F(CommandBufferValidationTest, BufferWithMultipleReadAndReadOnlyStorageUsage) {
// Create a buffer that will be used as an index buffer and as a storage buffer
wgpu::BufferDescriptor bufferDescriptor;
bufferDescriptor.usage = wgpu::BufferUsage::Storage | wgpu::BufferUsage::Index;
bufferDescriptor.size = 4;
wgpu::Buffer buffer = device.CreateBuffer(&bufferDescriptor);
// Create the bind group to use the buffer as storage
wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout(
device, {{0, wgpu::ShaderStage::Vertex, wgpu::BindingType::ReadonlyStorageBuffer}});
wgpu::BindGroup bg = utils::MakeBindGroup(device, bgl, {{0, buffer, 0, 4}});
// Use the buffer as both index and storage in the same pass
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
DummyRenderPass dummyRenderPass(device);
wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&dummyRenderPass);
pass.SetIndexBuffer(buffer);
pass.SetBindGroup(0, bg);
pass.EndPass();
encoder.Finish();
}
// Test that using the same storage buffer as both readable and writable in the same pass is
// disallowed
TEST_F(CommandBufferValidationTest, BufferWithReadAndWriteStorageBufferUsage) {
// Create a buffer that will be used as an storage buffer and as a readonly storage buffer
wgpu::BufferDescriptor bufferDescriptor;
bufferDescriptor.usage = wgpu::BufferUsage::Storage;
bufferDescriptor.size = 512;
wgpu::Buffer buffer = device.CreateBuffer(&bufferDescriptor);
// Create the bind group to use the buffer as storage
wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout(
device, {{0, wgpu::ShaderStage::Vertex, wgpu::BindingType::StorageBuffer},
{1, wgpu::ShaderStage::Vertex, wgpu::BindingType::ReadonlyStorageBuffer}});
wgpu::BindGroup bg =
utils::MakeBindGroup(device, bgl, {{0, buffer, 0, 4}, {1, buffer, 256, 4}});
// Use the buffer as both index and storage in the same pass
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
DummyRenderPass dummyRenderPass(device);
wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&dummyRenderPass);
pass.SetBindGroup(0, bg);
pass.EndPass();
ASSERT_DEVICE_ERROR(encoder.Finish());
}
// Test that using the same texture as both readable and writable in the same pass is disallowed // Test that using the same texture as both readable and writable in the same pass is disallowed
TEST_F(CommandBufferValidationTest, TextureWithReadAndWriteUsage) { TEST_F(CommandBufferValidationTest, TextureWithReadAndWriteUsage) {
// Create a texture that will be used both as a sampled texture and a render target // Create a texture that will be used both as a sampled texture and a render target