Add Internal ReadOnly Storage Buffer binding
This is needed for the upcoming B2T emulation using shaders. The shaders
only need to read a src buffer as readonly storage. We don't need a
full-blown wgpu::BufferUsage::Storage for it.
However, the current standard buffer binding validation will require
wgpu::BufferUsage::Storage even if the buffer is readonly in the shader.
So an extra internal readonly binding is needed.
Bug: 348653642
Change-Id: I1541ebae4581b10fe4d88e2dcfaf0cfc1781857a
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/230418
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Auto-Submit: Quyen Le <lehoangquyen@chromium.org>
Reviewed-by: Loko Kung <lokokung@google.com>
Commit-Queue: Quyen Le <lehoangquyen@chromium.org>
diff --git a/src/dawn/native/BindGroup.cpp b/src/dawn/native/BindGroup.cpp
index 64e32b6..d066989 100644
--- a/src/dawn/native/BindGroup.cpp
+++ b/src/dawn/native/BindGroup.cpp
@@ -109,6 +109,15 @@
requiredUsage = kInternalStorageBuffer;
requiredBindingAlignment = device->GetLimits().v1.minStorageBufferOffsetAlignment;
break;
+ case kInternalReadOnlyStorageBufferBinding:
+ // This is needed for for some workarounds that read a buffer in shaders. The buffer
+ // only needs kReadOnlyStorageBuffer usage in this case. Unlike the standard
+ // wgpu::BufferBindingType::ReadOnlyStorage which requires the read-write Storage usage.
+ // On some backends such as D3D11, using only kReadOnlyStorageBuffer usage could avoid
+ // extra allocations.
+ requiredUsage = kReadOnlyStorageBuffer;
+ requiredBindingAlignment = device->GetLimits().v1.minStorageBufferOffsetAlignment;
+ break;
case wgpu::BufferBindingType::BindingNotUsed:
case wgpu::BufferBindingType::Undefined:
DAWN_UNREACHABLE();
@@ -141,6 +150,7 @@
case wgpu::BufferBindingType::Storage:
case wgpu::BufferBindingType::ReadOnlyStorage:
case kInternalStorageBufferBinding:
+ case kInternalReadOnlyStorageBufferBinding:
maxStorageBufferBindingSize = device->GetLimits().v1.maxStorageBufferBindingSize;
DAWN_INVALID_IF(bindingSize > maxStorageBufferBindingSize,
"Binding size (%u) of %s is larger than the maximum storage buffer "
diff --git a/src/dawn/native/BindGroupLayoutInternal.cpp b/src/dawn/native/BindGroupLayoutInternal.cpp
index e935e69..91f2e55 100644
--- a/src/dawn/native/BindGroupLayoutInternal.cpp
+++ b/src/dawn/native/BindGroupLayoutInternal.cpp
@@ -103,7 +103,8 @@
// The kInternalStorageBufferBinding is used internally and not a value
// in wgpu::BufferBindingType.
- if (buffer.type == kInternalStorageBufferBinding) {
+ if (buffer.type == kInternalStorageBufferBinding ||
+ buffer.type == kInternalReadOnlyStorageBufferBinding) {
DAWN_INVALID_IF(!allowInternalBinding, "Internal binding types are disallowed");
} else {
DAWN_TRY(ValidateBufferBindingType(buffer.type));
@@ -844,6 +845,7 @@
case wgpu::BufferBindingType::Uniform:
return false;
case kInternalStorageBufferBinding:
+ case kInternalReadOnlyStorageBufferBinding:
case wgpu::BufferBindingType::Storage:
case wgpu::BufferBindingType::ReadOnlyStorage:
return true;
diff --git a/src/dawn/native/BindingInfo.cpp b/src/dawn/native/BindingInfo.cpp
index 777313b..ca1c442 100644
--- a/src/dawn/native/BindingInfo.cpp
+++ b/src/dawn/native/BindingInfo.cpp
@@ -76,6 +76,7 @@
case wgpu::BufferBindingType::Storage:
case kInternalStorageBufferBinding:
+ case kInternalReadOnlyStorageBufferBinding:
case wgpu::BufferBindingType::ReadOnlyStorage:
if (buffer.hasDynamicOffset) {
++bindingCounts->dynamicStorageBufferCount;
diff --git a/src/dawn/native/PassResourceUsageTracker.cpp b/src/dawn/native/PassResourceUsageTracker.cpp
index 275a9a6..1139423 100644
--- a/src/dawn/native/PassResourceUsageTracker.cpp
+++ b/src/dawn/native/PassResourceUsageTracker.cpp
@@ -124,6 +124,7 @@
BufferUsedAs(buffer, kInternalStorageBuffer, bindingInfo.visibility);
break;
case wgpu::BufferBindingType::ReadOnlyStorage:
+ case kInternalReadOnlyStorageBufferBinding:
BufferUsedAs(buffer, kReadOnlyStorageBuffer, bindingInfo.visibility);
break;
case wgpu::BufferBindingType::BindingNotUsed:
diff --git a/src/dawn/native/ProgrammableEncoder.cpp b/src/dawn/native/ProgrammableEncoder.cpp
index c7a2352..3573edd 100644
--- a/src/dawn/native/ProgrammableEncoder.cpp
+++ b/src/dawn/native/ProgrammableEncoder.cpp
@@ -203,6 +203,7 @@
case wgpu::BufferBindingType::Storage:
case wgpu::BufferBindingType::ReadOnlyStorage:
case kInternalStorageBufferBinding:
+ case kInternalReadOnlyStorageBufferBinding:
requiredAlignment = GetDevice()->GetLimits().v1.minStorageBufferOffsetAlignment;
break;
case wgpu::BufferBindingType::BindingNotUsed:
diff --git a/src/dawn/native/ShaderModule.cpp b/src/dawn/native/ShaderModule.cpp
index 3929270..01e5fd6 100644
--- a/src/dawn/native/ShaderModule.cpp
+++ b/src/dawn/native/ShaderModule.cpp
@@ -563,8 +563,11 @@
// group layout is invalid. For internal usage with internal shaders, a storage
// binding in the shader with an internal storage buffer in the bind group
// layout is also valid.
- bool validBindingConversion = (bindingLayout.type == kInternalStorageBufferBinding &&
- bindingInfo.type == wgpu::BufferBindingType::Storage);
+ bool validBindingConversion =
+ (bindingLayout.type == kInternalStorageBufferBinding &&
+ bindingInfo.type == wgpu::BufferBindingType::Storage) ||
+ (bindingLayout.type == kInternalReadOnlyStorageBufferBinding &&
+ bindingInfo.type == wgpu::BufferBindingType::ReadOnlyStorage);
DAWN_INVALID_IF(
bindingLayout.type != bindingInfo.type && !validBindingConversion,
diff --git a/src/dawn/native/d3d11/BindGroupTrackerD3D11.cpp b/src/dawn/native/d3d11/BindGroupTrackerD3D11.cpp
index 3bdbd90..e39b66d 100644
--- a/src/dawn/native/d3d11/BindGroupTrackerD3D11.cpp
+++ b/src/dawn/native/d3d11/BindGroupTrackerD3D11.cpp
@@ -213,7 +213,8 @@
}
break;
}
- case wgpu::BufferBindingType::ReadOnlyStorage: {
+ case wgpu::BufferBindingType::ReadOnlyStorage:
+ case kInternalReadOnlyStorageBufferBinding: {
ComPtr<ID3D11ShaderResourceView> d3d11SRV;
DAWN_TRY_ASSIGN(d3d11SRV,
ToGPUUsableBuffer(binding.buffer)
@@ -366,7 +367,8 @@
deviceContext->CSSetUnorderedAccessViews(bindingSlot, 1, &nullUAV, nullptr);
break;
}
- case wgpu::BufferBindingType::ReadOnlyStorage: {
+ case wgpu::BufferBindingType::ReadOnlyStorage:
+ case kInternalReadOnlyStorageBufferBinding: {
ID3D11ShaderResourceView* nullSRV = nullptr;
deviceContext->CSSetShaderResources(bindingSlot, 1, &nullSRV);
break;
@@ -496,7 +498,8 @@
break;
}
case wgpu::BufferBindingType::Uniform:
- case wgpu::BufferBindingType::ReadOnlyStorage: {
+ case wgpu::BufferBindingType::ReadOnlyStorage:
+ case kInternalReadOnlyStorageBufferBinding: {
break;
}
case wgpu::BufferBindingType::BindingNotUsed:
diff --git a/src/dawn/native/d3d11/PipelineLayoutD3D11.cpp b/src/dawn/native/d3d11/PipelineLayoutD3D11.cpp
index 26450c6..1c3dde3 100644
--- a/src/dawn/native/d3d11/PipelineLayoutD3D11.cpp
+++ b/src/dawn/native/d3d11/PipelineLayoutD3D11.cpp
@@ -75,6 +75,7 @@
mUAVBindGroups.set(group);
break;
case wgpu::BufferBindingType::ReadOnlyStorage:
+ case kInternalReadOnlyStorageBufferBinding:
mIndexInfo[group][bindingIndex] = shaderResourceViewIndex++;
break;
case wgpu::BufferBindingType::BindingNotUsed:
diff --git a/src/dawn/native/d3d11/ShaderModuleD3D11.cpp b/src/dawn/native/d3d11/ShaderModuleD3D11.cpp
index 7d7cc68..575c1a4 100644
--- a/src/dawn/native/d3d11/ShaderModuleD3D11.cpp
+++ b/src/dawn/native/d3d11/ShaderModuleD3D11.cpp
@@ -142,6 +142,7 @@
case kInternalStorageBufferBinding:
case wgpu::BufferBindingType::Storage:
case wgpu::BufferBindingType::ReadOnlyStorage:
+ case kInternalReadOnlyStorageBufferBinding:
bindings.storage.emplace(
srcBindingPoint, tint::hlsl::writer::binding::Storage{
dstBindingPoint.group, dstBindingPoint.binding});
diff --git a/src/dawn/native/d3d12/BindGroupD3D12.cpp b/src/dawn/native/d3d12/BindGroupD3D12.cpp
index 014ddcf..3f2ee1a 100644
--- a/src/dawn/native/d3d12/BindGroupD3D12.cpp
+++ b/src/dawn/native/d3d12/BindGroupD3D12.cpp
@@ -124,7 +124,8 @@
descriptorHeapOffsets[bindingIndex]));
break;
}
- case wgpu::BufferBindingType::ReadOnlyStorage: {
+ case wgpu::BufferBindingType::ReadOnlyStorage:
+ case kInternalReadOnlyStorageBufferBinding: {
// Like StorageBuffer, Tint outputs HLSL shaders for readonly
// storage buffer with ByteAddressBuffer. So we must use
// D3D12_BUFFER_SRV_FLAG_RAW when making the SRV descriptor. And it has
diff --git a/src/dawn/native/d3d12/BindGroupLayoutD3D12.cpp b/src/dawn/native/d3d12/BindGroupLayoutD3D12.cpp
index 5c18227..261c13d 100644
--- a/src/dawn/native/d3d12/BindGroupLayoutD3D12.cpp
+++ b/src/dawn/native/d3d12/BindGroupLayoutD3D12.cpp
@@ -50,6 +50,7 @@
case kInternalStorageBufferBinding:
return D3D12_DESCRIPTOR_RANGE_TYPE_UAV;
case wgpu::BufferBindingType::ReadOnlyStorage:
+ case kInternalReadOnlyStorageBufferBinding:
return D3D12_DESCRIPTOR_RANGE_TYPE_SRV;
case wgpu::BufferBindingType::BindingNotUsed:
case wgpu::BufferBindingType::Undefined:
diff --git a/src/dawn/native/d3d12/CommandBufferD3D12.cpp b/src/dawn/native/d3d12/CommandBufferD3D12.cpp
index b0c1af7..9cae4d9 100644
--- a/src/dawn/native/d3d12/CommandBufferD3D12.cpp
+++ b/src/dawn/native/d3d12/CommandBufferD3D12.cpp
@@ -519,6 +519,7 @@
}
break;
case wgpu::BufferBindingType::ReadOnlyStorage:
+ case kInternalReadOnlyStorageBufferBinding:
if constexpr (kIsRenderPipeline) {
commandList->SetGraphicsRootShaderResourceView(parameterIndex, bufferLocation);
} else {
diff --git a/src/dawn/native/d3d12/PipelineLayoutD3D12.cpp b/src/dawn/native/d3d12/PipelineLayoutD3D12.cpp
index 3c636fc..3ee337f 100644
--- a/src/dawn/native/d3d12/PipelineLayoutD3D12.cpp
+++ b/src/dawn/native/d3d12/PipelineLayoutD3D12.cpp
@@ -64,6 +64,7 @@
case kInternalStorageBufferBinding:
return D3D12_ROOT_PARAMETER_TYPE_UAV;
case wgpu::BufferBindingType::ReadOnlyStorage:
+ case kInternalReadOnlyStorageBufferBinding:
return D3D12_ROOT_PARAMETER_TYPE_SRV;
case wgpu::BufferBindingType::BindingNotUsed:
case wgpu::BufferBindingType::Undefined:
diff --git a/src/dawn/native/d3d12/ShaderModuleD3D12.cpp b/src/dawn/native/d3d12/ShaderModuleD3D12.cpp
index c65729e..67dac1b 100644
--- a/src/dawn/native/d3d12/ShaderModuleD3D12.cpp
+++ b/src/dawn/native/d3d12/ShaderModuleD3D12.cpp
@@ -211,6 +211,7 @@
case kInternalStorageBufferBinding:
case wgpu::BufferBindingType::Storage:
case wgpu::BufferBindingType::ReadOnlyStorage:
+ case kInternalReadOnlyStorageBufferBinding:
bindings.storage.emplace(
srcBindingPoint, tint::hlsl::writer::binding::Storage{
dstBindingPoint.group, dstBindingPoint.binding});
@@ -263,7 +264,8 @@
// buffer bindings to be treated as UAV instead of SRV. Internal storage
// buffer is a storage buffer used in the internal pipeline.
const bool forceStorageBufferAsUAV =
- (bufferBindingInfo->type == wgpu::BufferBindingType::ReadOnlyStorage &&
+ ((bufferBindingInfo->type == wgpu::BufferBindingType::ReadOnlyStorage ||
+ bufferBindingInfo->type == kInternalReadOnlyStorageBufferBinding) &&
(bindingLayout.type == wgpu::BufferBindingType::Storage ||
bindingLayout.type == kInternalStorageBufferBinding));
if (forceStorageBufferAsUAV) {
diff --git a/src/dawn/native/dawn_platform.h b/src/dawn/native/dawn_platform.h
index fdfcfd4..548b0bf 100644
--- a/src/dawn/native/dawn_platform.h
+++ b/src/dawn/native/dawn_platform.h
@@ -103,6 +103,10 @@
static constexpr wgpu::BufferBindingType kInternalStorageBufferBinding =
static_cast<wgpu::BufferBindingType>(~0u);
+// Extra BufferBindingType for internal readonly storage buffer binding.
+static constexpr wgpu::BufferBindingType kInternalReadOnlyStorageBufferBinding =
+ static_cast<wgpu::BufferBindingType>(~0u - 1);
+
// Extra TextureSampleType for sampling from a resolve attachment.
static constexpr wgpu::TextureSampleType kInternalResolveAttachmentSampleType =
static_cast<wgpu::TextureSampleType>(~0u);
diff --git a/src/dawn/native/metal/ShaderModuleMTL.mm b/src/dawn/native/metal/ShaderModuleMTL.mm
index 311a9db..3c92839 100644
--- a/src/dawn/native/metal/ShaderModuleMTL.mm
+++ b/src/dawn/native/metal/ShaderModuleMTL.mm
@@ -154,6 +154,7 @@
case kInternalStorageBufferBinding:
case wgpu::BufferBindingType::Storage:
case wgpu::BufferBindingType::ReadOnlyStorage:
+ case kInternalReadOnlyStorageBufferBinding:
bindings.storage.emplace(
srcBindingPoint,
tint::msl::writer::binding::Storage{dstBindingPoint.binding});
diff --git a/src/dawn/native/opengl/CommandBufferGL.cpp b/src/dawn/native/opengl/CommandBufferGL.cpp
index 94e5a058..07bd68f 100644
--- a/src/dawn/native/opengl/CommandBufferGL.cpp
+++ b/src/dawn/native/opengl/CommandBufferGL.cpp
@@ -345,6 +345,7 @@
case wgpu::BufferBindingType::Storage:
case kInternalStorageBufferBinding:
case wgpu::BufferBindingType::ReadOnlyStorage:
+ case kInternalReadOnlyStorageBufferBinding:
target = GL_SHADER_STORAGE_BUFFER;
break;
case wgpu::BufferBindingType::BindingNotUsed:
diff --git a/src/dawn/native/opengl/PipelineLayoutGL.cpp b/src/dawn/native/opengl/PipelineLayoutGL.cpp
index 5532f87..9b5dc3e 100644
--- a/src/dawn/native/opengl/PipelineLayoutGL.cpp
+++ b/src/dawn/native/opengl/PipelineLayoutGL.cpp
@@ -60,6 +60,7 @@
case wgpu::BufferBindingType::Storage:
case kInternalStorageBufferBinding:
case wgpu::BufferBindingType::ReadOnlyStorage:
+ case kInternalReadOnlyStorageBufferBinding:
mIndexInfo[group][bindingIndex] = ssboIndex;
ssboIndex++;
break;
diff --git a/src/dawn/native/opengl/ShaderModuleGL.cpp b/src/dawn/native/opengl/ShaderModuleGL.cpp
index 4c88566..d29dbeb 100644
--- a/src/dawn/native/opengl/ShaderModuleGL.cpp
+++ b/src/dawn/native/opengl/ShaderModuleGL.cpp
@@ -338,6 +338,7 @@
case kInternalStorageBufferBinding:
case wgpu::BufferBindingType::Storage:
case wgpu::BufferBindingType::ReadOnlyStorage:
+ case kInternalReadOnlyStorageBufferBinding:
bindings.storage.emplace(
srcBindingPoint,
tint::glsl::writer::binding::Storage{dstBindingPoint.binding});
diff --git a/src/dawn/native/vulkan/BindGroupLayoutVk.cpp b/src/dawn/native/vulkan/BindGroupLayoutVk.cpp
index 6e32b39..cecfcf8 100644
--- a/src/dawn/native/vulkan/BindGroupLayoutVk.cpp
+++ b/src/dawn/native/vulkan/BindGroupLayoutVk.cpp
@@ -78,6 +78,7 @@
case wgpu::BufferBindingType::Storage:
case kInternalStorageBufferBinding:
case wgpu::BufferBindingType::ReadOnlyStorage:
+ case kInternalReadOnlyStorageBufferBinding:
if (layout.hasDynamicOffset) {
return VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC;
}
diff --git a/src/dawn/native/vulkan/ShaderModuleVk.cpp b/src/dawn/native/vulkan/ShaderModuleVk.cpp
index 358f8b1..1f3036f 100644
--- a/src/dawn/native/vulkan/ShaderModuleVk.cpp
+++ b/src/dawn/native/vulkan/ShaderModuleVk.cpp
@@ -267,6 +267,7 @@
case kInternalStorageBufferBinding:
case wgpu::BufferBindingType::Storage:
case wgpu::BufferBindingType::ReadOnlyStorage:
+ case kInternalReadOnlyStorageBufferBinding:
bindings.storage.emplace(
srcBindingPoint,
tint::spirv::writer::binding::Storage{dstBindingPoint.group,
