D3D12: Fix pipeline layout overflow when using dynamic offsets.

Pipeline layout incorrectly indexes into a root table array
when there are more root descriptors than root tables.
To fix, the array is dynamically sized where parameters
are appended instead of indexed into the root signature.

Bug: dawn:449
Change-Id: I6d7f65fb791d323704b1c3a3af9c871a79e32a30
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/22960
Commit-Queue: Bryan Bernhart <bryan.bernhart@intel.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
This commit is contained in:
Bryan Bernhart 2020-06-10 16:09:45 +00:00 committed by Commit Bot service account
parent 4aca14ccb6
commit 18992f7be9
3 changed files with 83 additions and 22 deletions

View File

@ -69,22 +69,14 @@ namespace dawn_native { namespace d3d12 {
MaybeError PipelineLayout::Initialize() {
Device* device = ToBackend(GetDevice());
D3D12_ROOT_PARAMETER rootParameters[kMaxBindGroups * 2 + kMaxDynamicBufferCount];
// A root parameter is one of these types
union {
D3D12_ROOT_DESCRIPTOR_TABLE DescriptorTable;
D3D12_ROOT_CONSTANTS Constants;
D3D12_ROOT_DESCRIPTOR Descriptor;
} rootParameterValues[kMaxBindGroups * 2];
// samplers must be in a separate descriptor table so we need at most twice as many tables
// as bind groups
// Parameters are D3D12_ROOT_PARAMETER_TYPE which is either a root table, constant, or
// descriptor.
std::vector<D3D12_ROOT_PARAMETER> rootParameters;
// Ranges are D3D12_DESCRIPTOR_RANGE_TYPE_(SRV|UAV|CBV|SAMPLER)
// They are grouped together so each bind group has at most 4 ranges
D3D12_DESCRIPTOR_RANGE ranges[kMaxBindGroups * 4];
uint32_t parameterIndex = 0;
uint32_t rangeIndex = 0;
for (uint32_t group : IterateBitSet(GetBindGroupLayoutsMask())) {
@ -99,9 +91,8 @@ namespace dawn_native { namespace d3d12 {
return false;
}
auto& rootParameter = rootParameters[parameterIndex];
D3D12_ROOT_PARAMETER rootParameter = {};
rootParameter.ParameterType = D3D12_ROOT_PARAMETER_TYPE_DESCRIPTOR_TABLE;
rootParameter.DescriptorTable = rootParameterValues[parameterIndex].DescriptorTable;
rootParameter.ShaderVisibility = D3D12_SHADER_VISIBILITY_ALL;
rootParameter.DescriptorTable.NumDescriptorRanges = rangeCount;
rootParameter.DescriptorTable.pDescriptorRanges = &ranges[rangeIndex];
@ -112,17 +103,19 @@ namespace dawn_native { namespace d3d12 {
rangeIndex++;
}
rootParameters.emplace_back(rootParameter);
return true;
};
if (SetRootDescriptorTable(bindGroupLayout->GetCbvUavSrvDescriptorTableSize(),
bindGroupLayout->GetCbvUavSrvDescriptorRanges())) {
mCbvUavSrvRootParameterInfo[group] = parameterIndex++;
mCbvUavSrvRootParameterInfo[group] = rootParameters.size() - 1;
}
if (SetRootDescriptorTable(bindGroupLayout->GetSamplerDescriptorTableSize(),
bindGroupLayout->GetSamplerDescriptorRanges())) {
mSamplerRootParameterInfo[group] = parameterIndex++;
mSamplerRootParameterInfo[group] = rootParameters.size() - 1;
}
// Get calculated shader register for root descriptors
@ -142,7 +135,7 @@ namespace dawn_native { namespace d3d12 {
continue;
}
D3D12_ROOT_PARAMETER* rootParameter = &rootParameters[parameterIndex];
D3D12_ROOT_PARAMETER rootParameter = {};
// Setup root descriptor.
D3D12_ROOT_DESCRIPTOR rootDescriptor;
@ -150,20 +143,22 @@ namespace dawn_native { namespace d3d12 {
rootDescriptor.RegisterSpace = group;
// Set root descriptors in root signatures.
rootParameter->Descriptor = rootDescriptor;
mDynamicRootParameterIndices[group][dynamicBindingIndex] = parameterIndex++;
rootParameter.Descriptor = rootDescriptor;
mDynamicRootParameterIndices[group][dynamicBindingIndex] = rootParameters.size();
// Set parameter types according to bind group layout descriptor.
rootParameter->ParameterType = RootParameterType(bindingInfo.type);
rootParameter.ParameterType = RootParameterType(bindingInfo.type);
// Set visibilities according to bind group layout descriptor.
rootParameter->ShaderVisibility = ShaderVisibilityType(bindingInfo.visibility);
rootParameter.ShaderVisibility = ShaderVisibilityType(bindingInfo.visibility);
rootParameters.emplace_back(rootParameter);
}
}
D3D12_ROOT_SIGNATURE_DESC rootSignatureDescriptor;
rootSignatureDescriptor.NumParameters = parameterIndex;
rootSignatureDescriptor.pParameters = rootParameters;
rootSignatureDescriptor.NumParameters = rootParameters.size();
rootSignatureDescriptor.pParameters = rootParameters.data();
rootSignatureDescriptor.NumStaticSamplers = 0;
rootSignatureDescriptor.pStaticSamplers = nullptr;
rootSignatureDescriptor.Flags =

View File

@ -281,6 +281,7 @@ source_set("dawn_end2end_tests_sources") {
"end2end/NonzeroTextureCreationTests.cpp",
"end2end/ObjectCachingTests.cpp",
"end2end/OpArrayLengthTests.cpp",
"end2end/PipelineLayoutTests.cpp",
"end2end/PrimitiveTopologyTests.cpp",
"end2end/QueueTests.cpp",
"end2end/RenderBundleTests.cpp",

View File

@ -0,0 +1,65 @@
// Copyright 2020 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 "common/Constants.h"
#include "tests/DawnTest.h"
#include <vector>
class PipelineLayoutTests : public DawnTest {};
// Test creating a PipelineLayout with multiple BGLs where the first BGL uses the max number of
// dynamic buffers. This is a regression test for crbug.com/dawn/449 which would overflow when
// dynamic offset bindings were at max. Test is successful if the pipeline layout is created
// without error.
TEST_P(PipelineLayoutTests, DynamicBuffersOverflow) {
// Create the first bind group layout which uses max number of dynamic buffers bindings.
wgpu::BindGroupLayout bglA;
{
std::vector<wgpu::BindGroupLayoutEntry> entries;
for (uint32_t i = 0; i < kMaxDynamicStorageBufferCount; i++) {
entries.push_back(
{i, wgpu::ShaderStage::Compute, wgpu::BindingType::StorageBuffer, true});
}
wgpu::BindGroupLayoutDescriptor descriptor;
descriptor.entryCount = static_cast<uint32_t>(entries.size());
descriptor.entries = entries.data();
bglA = device.CreateBindGroupLayout(&descriptor);
}
// Create the second bind group layout that has one non-dynamic buffer binding.
wgpu::BindGroupLayout bglB;
{
wgpu::BindGroupLayoutDescriptor descriptor;
wgpu::BindGroupLayoutEntry entry = {0, wgpu::ShaderStage::Compute,
wgpu::BindingType::StorageBuffer, false};
descriptor.entryCount = 1;
descriptor.entries = &entry;
bglB = device.CreateBindGroupLayout(&descriptor);
}
// Create a pipeline layout using both bind group layouts.
wgpu::PipelineLayoutDescriptor descriptor;
std::vector<wgpu::BindGroupLayout> bindgroupLayouts = {bglA, bglB};
descriptor.bindGroupLayoutCount = bindgroupLayouts.size();
descriptor.bindGroupLayouts = bindgroupLayouts.data();
device.CreatePipelineLayout(&descriptor);
}
DAWN_INSTANTIATE_TEST(PipelineLayoutTests,
D3D12Backend(),
MetalBackend(),
OpenGLBackend(),
VulkanBackend());