D3D12: Fix RO storage buffer mismatches.
If the shader declares a storage buffer RO but uses storage buffer in
the BGL, the shader compiler will be told to treat these bindings as
UAV instead of SRV to avoid PSO mismatches.
Bug: dawn:410
Change-Id: I3be3257449de55fd2d35e914233b48c6f7121b58
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/22322
Commit-Queue: Bryan Bernhart <bryan.bernhart@intel.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/dawn_native/d3d12/ShaderModuleD3D12.cpp b/src/dawn_native/d3d12/ShaderModuleD3D12.cpp
index 6dc3238f..87f1e03 100644
--- a/src/dawn_native/d3d12/ShaderModuleD3D12.cpp
+++ b/src/dawn_native/d3d12/ShaderModuleD3D12.cpp
@@ -164,6 +164,13 @@
BindingNumber bindingNumber = it.first;
BindingIndex bindingIndex = bgl->GetBindingIndex(bindingNumber);
+ // Declaring a read-only storage buffer in HLSL but specifying a storage buffer in
+ // the BGL produces the wrong output. Force read-only storage buffer bindings to
+ // be treated as UAV instead of SRV.
+ const bool forceStorageBufferAsUAV =
+ (bindingInfo.type == wgpu::BindingType::ReadonlyStorageBuffer &&
+ bgl->GetBindingInfo(bindingIndex).type == wgpu::BindingType::StorageBuffer);
+
uint32_t bindingOffset = bindingOffsets[bindingIndex];
if (GetDevice()->IsToggleEnabled(Toggle::UseSpvc)) {
DAWN_TRY(CheckSpvcSuccess(
@@ -171,8 +178,16 @@
bindingOffset),
"Unable to set decorating binding before generating HLSL shader w/ "
"spvc"));
+ if (forceStorageBufferAsUAV) {
+ DAWN_TRY(CheckSpvcSuccess(
+ mSpvcContext.SetHLSLForceStorageBufferAsUAV(group, bindingNumber),
+ "Unable to force read-only storage buffer as UAV w/ spvc"));
+ }
} else {
compiler->set_decoration(bindingInfo.id, spv::DecorationBinding, bindingOffset);
+ if (forceStorageBufferAsUAV) {
+ compiler->set_hlsl_force_storage_buffer_as_uav(group, bindingNumber);
+ }
}
}
}
diff --git a/src/tests/end2end/BindGroupTests.cpp b/src/tests/end2end/BindGroupTests.cpp
index c288844..99d5c0c 100644
--- a/src/tests/end2end/BindGroupTests.cpp
+++ b/src/tests/end2end/BindGroupTests.cpp
@@ -1038,4 +1038,58 @@
queue.Submit(1, &commands);
}
+// Test creating a BGL with a storage buffer binding but declared readonly in the shader works.
+// This is a regression test for crbug.com/dawn/410 which tests that it can successfully compile and
+// execute the shader.
+TEST_P(BindGroupTests, ReadonlyStorage) {
+ utils::ComboRenderPipelineDescriptor pipelineDescriptor(device);
+
+ pipelineDescriptor.vertexStage.module =
+ utils::CreateShaderModule(device, utils::SingleShaderStage::Vertex, R"(
+ #version 450
+ void main() {
+ const vec2 pos[3] = vec2[3](vec2(-1.f, 1.f), vec2(1.f, 1.f), vec2(-1.f, -1.f));
+ gl_Position = vec4(pos[gl_VertexIndex], 0.f, 1.f);
+ })");
+
+ pipelineDescriptor.cFragmentStage.module =
+ utils::CreateShaderModule(device, utils::SingleShaderStage::Fragment, R"(
+ #version 450
+ layout(set = 0, binding = 0) readonly buffer buffer0 {
+ vec4 color;
+ };
+ layout(location = 0) out vec4 fragColor;
+ void main() {
+ fragColor = color;
+ })");
+
+ constexpr uint32_t kRTSize = 4;
+ utils::BasicRenderPass renderPass = utils::CreateBasicRenderPass(device, kRTSize, kRTSize);
+ pipelineDescriptor.cColorStates[0].format = renderPass.colorFormat;
+
+ wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout(
+ device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::StorageBuffer}});
+
+ pipelineDescriptor.layout = utils::MakeBasicPipelineLayout(device, &bgl);
+
+ wgpu::RenderPipeline renderPipeline = device.CreateRenderPipeline(&pipelineDescriptor);
+
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass.renderPassInfo);
+
+ std::array<float, 4> greenColor = {0, 1, 0, 1};
+ wgpu::Buffer storageBuffer = utils::CreateBufferFromData(
+ device, &greenColor, sizeof(greenColor), wgpu::BufferUsage::Storage);
+
+ pass.SetPipeline(renderPipeline);
+ pass.SetBindGroup(0, utils::MakeBindGroup(device, bgl, {{0, storageBuffer}}));
+ pass.Draw(3);
+ pass.EndPass();
+
+ wgpu::CommandBuffer commands = encoder.Finish();
+ queue.Submit(1, &commands);
+
+ EXPECT_PIXEL_RGBA8_EQ(RGBA8::kGreen, renderPass.color, 0, 0);
+}
+
DAWN_INSTANTIATE_TEST(BindGroupTests, D3D12Backend(), MetalBackend(), OpenGLBackend(), VulkanBackend());
