diff --git a/src/dawn_native/CommandValidation.cpp b/src/dawn_native/CommandValidation.cpp index 7fc8caeadf..818b503478 100644 --- a/src/dawn_native/CommandValidation.cpp +++ b/src/dawn_native/CommandValidation.cpp @@ -299,7 +299,7 @@ namespace dawn_native { bool readOnly = (usage & kReadOnlyBufferUsages) == usage; bool singleUse = wgpu::HasZeroOrOneBits(usage); - if (!readOnly && !singleUse) { + if (pass.passType == PassType::Render && !readOnly && !singleUse) { return DAWN_VALIDATION_ERROR( "Buffer used as writable usage and another usage in pass"); } diff --git a/src/tests/BUILD.gn b/src/tests/BUILD.gn index 7e41d4042c..aac386157d 100644 --- a/src/tests/BUILD.gn +++ b/src/tests/BUILD.gn @@ -151,7 +151,6 @@ test("dawn_unittests") { "unittests/validation/BufferValidationTests.cpp", "unittests/validation/CommandBufferValidationTests.cpp", "unittests/validation/ComputeIndirectValidationTests.cpp", - "unittests/validation/ComputePassValidationTests.cpp", "unittests/validation/ComputeValidationTests.cpp", "unittests/validation/CopyCommandsValidationTests.cpp", "unittests/validation/DebugMarkerValidationTests.cpp", diff --git a/src/tests/unittests/validation/ComputePassValidationTests.cpp b/src/tests/unittests/validation/ComputePassValidationTests.cpp deleted file mode 100644 index 851c51906d..0000000000 --- a/src/tests/unittests/validation/ComputePassValidationTests.cpp +++ /dev/null @@ -1,147 +0,0 @@ -// Copyright 2019 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 "tests/unittests/validation/ValidationTest.h" - -#include "utils/WGPUHelpers.h" - -class ComputePassValidationTest : public ValidationTest {}; - -// Test that it is invalid to use a buffer with both read and write usages in a compute pass. -TEST_F(ComputePassValidationTest, ReadWriteUsage) { - wgpu::BufferDescriptor bufferDesc = {}; - bufferDesc.usage = wgpu::BufferUsage::Storage | wgpu::BufferUsage::Uniform; - bufferDesc.size = 4; - wgpu::Buffer buffer = device.CreateBuffer(&bufferDesc); - - wgpu::ShaderModule module = - utils::CreateShaderModule(device, utils::SingleShaderStage::Compute, R"( - #version 450 - layout(std430, set = 0, binding = 0) buffer BufA { uint bufA; }; - layout(std140, set = 0, binding = 1) uniform BufB { uint bufB; }; - void main() {} - )"); - - wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Compute, wgpu::BindingType::StorageBuffer}, - {1, wgpu::ShaderStage::Compute, wgpu::BindingType::UniformBuffer}}); - - wgpu::BindGroup bindGroup = utils::MakeBindGroup(device, bgl, - { - {0, buffer, 0, 4}, - {1, buffer, 0, 4}, - }); - - wgpu::PipelineLayout layout = utils::MakeBasicPipelineLayout(device, &bgl); - - wgpu::ComputePipelineDescriptor pipelineDesc = {}; - pipelineDesc.layout = layout; - pipelineDesc.computeStage.module = module; - pipelineDesc.computeStage.entryPoint = "main"; - wgpu::ComputePipeline pipeline = device.CreateComputePipeline(&pipelineDesc); - - wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); - wgpu::ComputePassEncoder pass = encoder.BeginComputePass(); - pass.SetPipeline(pipeline); - pass.SetBindGroup(0, bindGroup); - - pass.Dispatch(1); - pass.Dispatch(1); - - pass.EndPass(); - ASSERT_DEVICE_ERROR(encoder.Finish()); -} - -// Test that it is valid to use a buffer with a single write usage multiple times in a compute pass. -TEST_F(ComputePassValidationTest, MultipleWrites) { - wgpu::BufferDescriptor bufferDesc = {}; - bufferDesc.usage = wgpu::BufferUsage::Storage; - bufferDesc.size = 4; - wgpu::Buffer buffer = device.CreateBuffer(&bufferDesc); - - wgpu::ShaderModule module = - utils::CreateShaderModule(device, utils::SingleShaderStage::Compute, R"( - #version 450 - layout(std430, set = 0, binding = 0) buffer Buf { uint buf; }; - void main() {} - )"); - - wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Compute, wgpu::BindingType::StorageBuffer}}); - - wgpu::BindGroup bindGroup = utils::MakeBindGroup(device, bgl, {{0, buffer, 0, 4}}); - - wgpu::PipelineLayout layout = utils::MakeBasicPipelineLayout(device, &bgl); - - wgpu::ComputePipelineDescriptor pipelineDesc = {}; - pipelineDesc.layout = layout; - pipelineDesc.computeStage.module = module; - pipelineDesc.computeStage.entryPoint = "main"; - wgpu::ComputePipeline pipeline = device.CreateComputePipeline(&pipelineDesc); - - wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); - wgpu::ComputePassEncoder pass = encoder.BeginComputePass(); - pass.SetPipeline(pipeline); - pass.SetBindGroup(0, bindGroup); - - pass.Dispatch(1); - pass.Dispatch(1); - - pass.EndPass(); - encoder.Finish(); -} - -// Test that it is valid to use a buffer with a single write usage multiple times in a compute pass, -// even if the buffer is referenced in separate bind groups. -TEST_F(ComputePassValidationTest, MultipleWritesSeparateBindGroups) { - wgpu::BufferDescriptor bufferDesc = {}; - bufferDesc.usage = wgpu::BufferUsage::Storage; - bufferDesc.size = 4; - wgpu::Buffer buffer = device.CreateBuffer(&bufferDesc); - - wgpu::ShaderModule module = - utils::CreateShaderModule(device, utils::SingleShaderStage::Compute, R"( - #version 450 - #define kNumValues 100 - layout(std430, set = 0, binding = 0) buffer Buf { uint buf; }; - void main() {} - )"); - - wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Compute, wgpu::BindingType::StorageBuffer}}); - - wgpu::BindGroup bindGroupA = utils::MakeBindGroup(device, bgl, {{0, buffer, 0, 4}}); - wgpu::BindGroup bindGroupB = utils::MakeBindGroup(device, bgl, {{0, buffer, 0, 4}}); - - wgpu::PipelineLayout layout = utils::MakeBasicPipelineLayout(device, &bgl); - - wgpu::ComputePipelineDescriptor pipelineDesc = {}; - pipelineDesc.layout = layout; - pipelineDesc.computeStage.module = module; - pipelineDesc.computeStage.entryPoint = "main"; - wgpu::ComputePipeline pipeline = device.CreateComputePipeline(&pipelineDesc); - - wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); - wgpu::ComputePassEncoder pass = encoder.BeginComputePass(); - pass.SetPipeline(pipeline); - - pass.SetBindGroup(0, bindGroupA); - pass.Dispatch(1); - - pass.SetBindGroup(0, bindGroupB); - pass.Dispatch(1); - - pass.EndPass(); - encoder.Finish(); -} diff --git a/src/tests/unittests/validation/ResourceUsageTrackingTests.cpp b/src/tests/unittests/validation/ResourceUsageTrackingTests.cpp index 741bb6bb32..04573acf59 100644 --- a/src/tests/unittests/validation/ResourceUsageTrackingTests.cpp +++ b/src/tests/unittests/validation/ResourceUsageTrackingTests.cpp @@ -82,7 +82,8 @@ namespace { } } - // Test that using the same buffer as both readable and writable in the same pass is disallowed + // Test that it is invalid to use the same buffer as both readable and writable in the same + // render pass. But it is valid in compute pass. TEST_F(ResourceUsageTrackingTest, BufferWithReadAndWriteUsage) { // test render pass for index buffer and storage buffer { @@ -94,7 +95,7 @@ namespace { device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::StorageBuffer}}); wgpu::BindGroup bg = utils::MakeBindGroup(device, bgl, {{0, buffer, 0, 4}}); - // Use the buffer as both index and storage in render pass + // It is invalid to use the buffer as both index and storage in render pass wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); DummyRenderPass dummyRenderPass(device); wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&dummyRenderPass); @@ -116,12 +117,12 @@ namespace { wgpu::BindGroup bg = utils::MakeBindGroup(device, bgl, {{0, buffer, 0, 4}, {1, buffer, 256, 4}}); - // Use the buffer as both storage and readonly storage in compute pass + // It is valid to use the buffer as both storage and readonly storage in compute pass. wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); wgpu::ComputePassEncoder pass = encoder.BeginComputePass(); pass.SetBindGroup(0, bg); pass.EndPass(); - ASSERT_DEVICE_ERROR(encoder.Finish()); + encoder.Finish(); } } @@ -455,12 +456,13 @@ namespace { {1, wgpu::ShaderStage::None, wgpu::BindingType::StorageBuffer}}); wgpu::BindGroup bg = utils::MakeBindGroup(device, bgl, {{0, buffer}, {1, buffer}}); - // These two bindings are invisible in compute pass. But we still track these bindings. + // These two bindings are invisible in compute pass. We still track these invisible + // bindings, but read and write usages in one compute pass is allowed. wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); wgpu::ComputePassEncoder pass = encoder.BeginComputePass(); pass.SetBindGroup(0, bg); pass.EndPass(); - ASSERT_DEVICE_ERROR(encoder.Finish()); + encoder.Finish(); } } @@ -499,13 +501,13 @@ namespace { wgpu::BindGroup bg = utils::MakeBindGroup(device, bgl, {{0, buffer}, {1, buffer}}); // Buffer usage in compute stage conflicts with buffer usage in fragment stage. And - // binding for fragment stage is not visible in compute pass. But we still track this - // binding. + // binding for fragment stage is not visible in compute pass. We still track this + // invisible binding, but read and write usages in one compute pass is allowed. wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); wgpu::ComputePassEncoder pass = encoder.BeginComputePass(); pass.SetBindGroup(0, bg); pass.EndPass(); - ASSERT_DEVICE_ERROR(encoder.Finish()); + encoder.Finish(); } } @@ -630,7 +632,8 @@ namespace { wgpu::ComputePipeline cp = device.CreateComputePipeline(&pipelineDescriptor); // Resource in bg1 conflicts with resources used in bg0. However, the binding in bg1 is - // not used in pipeline. But we still track this binding. + // not used in pipeline. But we still track this binding and read/write usage in one + // dispatch is not allowed. wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); wgpu::ComputePassEncoder pass = encoder.BeginComputePass(); pass.SetBindGroup(0, bg0); @@ -638,7 +641,9 @@ namespace { pass.SetPipeline(cp); pass.Dispatch(1); pass.EndPass(); - ASSERT_DEVICE_ERROR(encoder.Finish()); + encoder.Finish(); + // TODO (yunchao.he@intel.com): add resource tracking per dispatch for compute pass + // ASSERT_DEVICE_ERROR(encoder.Finish()); } }