From e4c0a82ecfd5f3c78dc90e7ea3c57568f7ac44ae Mon Sep 17 00:00:00 2001 From: Jiawei Shao Date: Thu, 16 May 2019 05:56:49 +0000 Subject: [PATCH] D3D12: Create descriptor heaps after computing the required size This patch fixes a crash issue that on D3D12 backends Dawn will crash when we create too many views from cbvSrvUav or sampler descriptor heaps as both of the heaps have fixed sizes. To fix this issue we count the number of views that are required in these two heaps before allocating memory for them, so that the memory allocation from these two heaps will always succeed. BUG=dawn:149 Change-Id: I7d42bf2ad677c6ecfa3ce1b3e471bc0d9258f266 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/7260 Reviewed-by: Corentin Wallez Reviewed-by: Kai Ninomiya Commit-Queue: Jiawei Shao --- src/dawn_native/d3d12/BindGroupD3D12.cpp | 23 +-- src/dawn_native/d3d12/BindGroupD3D12.h | 17 +- src/dawn_native/d3d12/CommandBufferD3D12.cpp | 173 ++++++++++--------- src/dawn_native/d3d12/CommandBufferD3D12.h | 3 +- 4 files changed, 110 insertions(+), 106 deletions(-) diff --git a/src/dawn_native/d3d12/BindGroupD3D12.cpp b/src/dawn_native/d3d12/BindGroupD3D12.cpp index 36935184ac..1c5862cf0e 100644 --- a/src/dawn_native/d3d12/BindGroupD3D12.cpp +++ b/src/dawn_native/d3d12/BindGroupD3D12.cpp @@ -27,15 +27,10 @@ namespace dawn_native { namespace d3d12 { : BindGroupBase(device, descriptor) { } - void BindGroup::RecordDescriptors(const DescriptorHeapHandle& cbvUavSrvHeapStart, - uint32_t* cbvUavSrvHeapOffset, - const DescriptorHeapHandle& samplerHeapStart, - uint32_t* samplerHeapOffset, - uint64_t serial, - uint32_t indexInSubmit) { - mHeapSerial = serial; - mIndexInSubmit = indexInSubmit; - + void BindGroup::AllocateDescriptors(const DescriptorHeapHandle& cbvUavSrvHeapStart, + uint32_t* cbvUavSrvHeapOffset, + const DescriptorHeapHandle& samplerHeapStart, + uint32_t* samplerHeapOffset) { const auto* bgl = ToBackend(GetLayout()); const auto& layout = bgl->GetBindingInfo(); @@ -117,11 +112,11 @@ namespace dawn_native { namespace d3d12 { return mSamplerHeapOffset; } - uint64_t BindGroup::GetHeapSerial() const { - return mHeapSerial; - } - uint32_t BindGroup::GetIndexInSubmit() const { - return mIndexInSubmit; + bool BindGroup::TestAndSetCounted(uint64_t heapSerial, uint32_t indexInSubmit) { + bool isCounted = (mHeapSerial == heapSerial && mIndexInSubmit == indexInSubmit); + mHeapSerial = heapSerial; + mIndexInSubmit = indexInSubmit; + return isCounted; } }} // namespace dawn_native::d3d12 diff --git a/src/dawn_native/d3d12/BindGroupD3D12.h b/src/dawn_native/d3d12/BindGroupD3D12.h index f8d5fbfdd7..458e9926f7 100644 --- a/src/dawn_native/d3d12/BindGroupD3D12.h +++ b/src/dawn_native/d3d12/BindGroupD3D12.h @@ -29,22 +29,21 @@ namespace dawn_native { namespace d3d12 { public: BindGroup(Device* device, const BindGroupDescriptor* descriptor); - void RecordDescriptors(const DescriptorHeapHandle& cbvSrvUavHeapStart, - uint32_t* cbvUavSrvHeapOffset, - const DescriptorHeapHandle& samplerHeapStart, - uint32_t* samplerHeapOffset, - uint64_t serial, - uint32_t indexInSubmit); + void AllocateDescriptors(const DescriptorHeapHandle& cbvSrvUavHeapStart, + uint32_t* cbvUavSrvHeapOffset, + const DescriptorHeapHandle& samplerHeapStart, + uint32_t* samplerHeapOffset); uint32_t GetCbvUavSrvHeapOffset() const; uint32_t GetSamplerHeapOffset() const; - uint64_t GetHeapSerial() const; - uint32_t GetIndexInSubmit() const; + + bool TestAndSetCounted(uint64_t heapSerial, uint32_t indexInSubmit); private: uint32_t mCbvUavSrvHeapOffset; uint32_t mSamplerHeapOffset; + uint64_t mHeapSerial = 0; - uint32_t mIndexInSubmit; + uint32_t mIndexInSubmit = 0; }; }} // namespace dawn_native::d3d12 diff --git a/src/dawn_native/d3d12/CommandBufferD3D12.cpp b/src/dawn_native/d3d12/CommandBufferD3D12.cpp index b0603a22db..f6fd304e82 100644 --- a/src/dawn_native/d3d12/CommandBufferD3D12.cpp +++ b/src/dawn_native/d3d12/CommandBufferD3D12.cpp @@ -30,6 +30,8 @@ #include "dawn_native/d3d12/TextureCopySplitter.h" #include "dawn_native/d3d12/TextureD3D12.h" +#include + namespace dawn_native { namespace d3d12 { namespace { @@ -71,41 +73,66 @@ namespace dawn_native { namespace d3d12 { } // anonymous namespace - struct BindGroupStateTracker { - uint32_t cbvSrvUavDescriptorIndex = 0; - uint32_t samplerDescriptorIndex = 0; - DescriptorHeapHandle cbvSrvUavCPUDescriptorHeap = {}; - DescriptorHeapHandle samplerCPUDescriptorHeap = {}; - DescriptorHeapHandle cbvSrvUavGPUDescriptorHeap = {}; - DescriptorHeapHandle samplerGPUDescriptorHeap = {}; - std::array bindGroups = {}; - bool inCompute = false; - - Device* device; - - BindGroupStateTracker(Device* device) : device(device) { + class BindGroupStateTracker { + public: + BindGroupStateTracker(Device* device) : mDevice(device) { } void SetInComputePass(bool inCompute_) { - inCompute = inCompute_; + mInCompute = inCompute_; } - void TrackSetBindGroup(BindGroup* group, uint32_t index, uint32_t indexInSubmit) { - if (bindGroups[index] != group) { - bindGroups[index] = group; + void AllocateDescriptorHeaps(Device* device) { + // This function should only be called once. + ASSERT(mCbvSrvUavGPUDescriptorHeap.Get() == nullptr && + mSamplerGPUDescriptorHeap.Get() == nullptr); - // Descriptors don't need to be recorded if they have already been recorded in - // the heap. Indices are only updated when descriptors are recorded - const uint64_t serial = device->GetPendingCommandSerial(); - if (group->GetHeapSerial() != serial || - group->GetIndexInSubmit() != indexInSubmit) { - group->RecordDescriptors(cbvSrvUavCPUDescriptorHeap, &cbvSrvUavDescriptorIndex, - samplerCPUDescriptorHeap, &samplerDescriptorIndex, - serial, indexInSubmit); + DescriptorHeapAllocator* descriptorHeapAllocator = device->GetDescriptorHeapAllocator(); + + if (mCbvSrvUavDescriptorHeapSize > 0) { + mCbvSrvUavGPUDescriptorHeap = descriptorHeapAllocator->AllocateGPUHeap( + D3D12_DESCRIPTOR_HEAP_TYPE_CBV_SRV_UAV, mCbvSrvUavDescriptorHeapSize); + } + + if (mSamplerDescriptorHeapSize > 0) { + mSamplerGPUDescriptorHeap = descriptorHeapAllocator->AllocateGPUHeap( + D3D12_DESCRIPTOR_HEAP_TYPE_SAMPLER, mSamplerDescriptorHeapSize); + } + + uint32_t cbvSrvUavDescriptorIndex = 0; + uint32_t samplerDescriptorIndex = 0; + for (BindGroup* group : mBindGroupsList) { + ASSERT(group); + ASSERT(cbvSrvUavDescriptorIndex + + ToBackend(group->GetLayout())->GetCbvUavSrvDescriptorCount() <= + mCbvSrvUavDescriptorHeapSize); + ASSERT(samplerDescriptorIndex + + ToBackend(group->GetLayout())->GetSamplerDescriptorCount() <= + mSamplerDescriptorHeapSize); + group->AllocateDescriptors(mCbvSrvUavGPUDescriptorHeap, &cbvSrvUavDescriptorIndex, + mSamplerGPUDescriptorHeap, &samplerDescriptorIndex); + } + + ASSERT(cbvSrvUavDescriptorIndex == mCbvSrvUavDescriptorHeapSize); + ASSERT(samplerDescriptorIndex == mSamplerDescriptorHeapSize); + } + + // This function must only be called before calling AllocateDescriptorHeaps(). + void TrackSetBindGroup(BindGroup* group, uint32_t index, uint32_t indexInSubmit) { + if (mBindGroups[index] != group) { + mBindGroups[index] = group; + + if (!group->TestAndSetCounted(mDevice->GetPendingCommandSerial(), indexInSubmit)) { + const BindGroupLayout* layout = ToBackend(group->GetLayout()); + + mCbvSrvUavDescriptorHeapSize += layout->GetCbvUavSrvDescriptorCount(); + mSamplerDescriptorHeapSize += layout->GetSamplerDescriptorCount(); + mBindGroupsList.push_back(group); } } } + // This function must only be called before calling AllocateDescriptorHeaps(). void TrackInheritedGroups(PipelineLayout* oldLayout, PipelineLayout* newLayout, uint32_t indexInSubmit) { @@ -115,7 +142,7 @@ namespace dawn_native { namespace d3d12 { uint32_t inheritUntil = oldLayout->GroupsInheritUpTo(newLayout); for (uint32_t i = 0; i < inheritUntil; ++i) { - TrackSetBindGroup(bindGroups[i], i, indexInSubmit); + TrackSetBindGroup(mBindGroups[i], i, indexInSubmit); } } @@ -124,8 +151,8 @@ namespace dawn_native { namespace d3d12 { BindGroup* group, uint32_t index, bool force = false) { - if (bindGroups[index] != group || force) { - bindGroups[index] = group; + if (mBindGroups[index] != group || force) { + mBindGroups[index] = group; uint32_t cbvUavSrvCount = ToBackend(group->GetLayout())->GetCbvUavSrvDescriptorCount(); @@ -134,13 +161,13 @@ namespace dawn_native { namespace d3d12 { if (cbvUavSrvCount > 0) { uint32_t parameterIndex = pipelineLayout->GetCbvUavSrvRootParameterIndex(index); - if (inCompute) { + if (mInCompute) { commandList->SetComputeRootDescriptorTable( - parameterIndex, cbvSrvUavGPUDescriptorHeap.GetGPUHandle( + parameterIndex, mCbvSrvUavGPUDescriptorHeap.GetGPUHandle( group->GetCbvUavSrvHeapOffset())); } else { commandList->SetGraphicsRootDescriptorTable( - parameterIndex, cbvSrvUavGPUDescriptorHeap.GetGPUHandle( + parameterIndex, mCbvSrvUavGPUDescriptorHeap.GetGPUHandle( group->GetCbvUavSrvHeapOffset())); } } @@ -148,14 +175,14 @@ namespace dawn_native { namespace d3d12 { if (samplerCount > 0) { uint32_t parameterIndex = pipelineLayout->GetSamplerRootParameterIndex(index); - if (inCompute) { + if (mInCompute) { commandList->SetComputeRootDescriptorTable( parameterIndex, - samplerGPUDescriptorHeap.GetGPUHandle(group->GetSamplerHeapOffset())); + mSamplerGPUDescriptorHeap.GetGPUHandle(group->GetSamplerHeapOffset())); } else { commandList->SetGraphicsRootDescriptorTable( parameterIndex, - samplerGPUDescriptorHeap.GetGPUHandle(group->GetSamplerHeapOffset())); + mSamplerGPUDescriptorHeap.GetGPUHandle(group->GetSamplerHeapOffset())); } } } @@ -170,15 +197,40 @@ namespace dawn_native { namespace d3d12 { uint32_t inheritUntil = oldLayout->GroupsInheritUpTo(newLayout); for (uint32_t i = 0; i < inheritUntil; ++i) { - SetBindGroup(commandList, newLayout, bindGroups[i], i, true); + SetBindGroup(commandList, newLayout, mBindGroups[i], i, true); } } void Reset() { for (uint32_t i = 0; i < kMaxBindGroups; ++i) { - bindGroups[i] = nullptr; + mBindGroups[i] = nullptr; } } + + void SetID3D12DescriptorHeaps(ComPtr commandList) { + ASSERT(commandList != nullptr); + ID3D12DescriptorHeap* descriptorHeaps[2] = {mCbvSrvUavGPUDescriptorHeap.Get(), + mSamplerGPUDescriptorHeap.Get()}; + if (descriptorHeaps[0] && descriptorHeaps[1]) { + commandList->SetDescriptorHeaps(2, descriptorHeaps); + } else if (descriptorHeaps[0]) { + commandList->SetDescriptorHeaps(1, descriptorHeaps); + } else if (descriptorHeaps[1]) { + commandList->SetDescriptorHeaps(1, &descriptorHeaps[1]); + } + } + + private: + uint32_t mCbvSrvUavDescriptorHeapSize = 0; + uint32_t mSamplerDescriptorHeapSize = 0; + std::array mBindGroups = {}; + std::deque mBindGroupsList = {}; + bool mInCompute = false; + + DescriptorHeapHandle mCbvSrvUavGPUDescriptorHeap = {}; + DescriptorHeapHandle mSamplerGPUDescriptorHeap = {}; + + Device* mDevice; }; struct OMSetRenderTargetArgs { @@ -195,6 +247,7 @@ namespace dawn_native { namespace d3d12 { // This function must only be called before calling AllocateRTVAndDSVHeaps(). void TrackRenderPass(const BeginRenderPassCmd* renderPass) { DAWN_ASSERT(mRTVHeap.Get() == nullptr && mDSVHeap.Get() == nullptr); + mNumRTVs += static_cast(renderPass->colorAttachmentsSet.count()); if (renderPass->hasDepthStencilAttachment) { ++mNumDSVs; @@ -270,16 +323,7 @@ namespace dawn_native { namespace d3d12 { BindGroupStateTracker* bindingTracker, RenderPassDescriptorHeapTracker* renderPassTracker, CommandIterator* commands, - int indexInSubmit) { - auto* descriptorHeapAllocator = device->GetDescriptorHeapAllocator(); - - // TODO(enga@google.com): This currently allocates CPU heaps of arbitrarily chosen sizes - // This will not work if there are too many descriptors - bindingTracker->cbvSrvUavCPUDescriptorHeap = descriptorHeapAllocator->AllocateCPUHeap( - D3D12_DESCRIPTOR_HEAP_TYPE_CBV_SRV_UAV, 8192); - bindingTracker->samplerCPUDescriptorHeap = - descriptorHeapAllocator->AllocateCPUHeap(D3D12_DESCRIPTOR_HEAP_TYPE_SAMPLER, 2048); - + uint32_t indexInSubmit) { { Command type; PipelineLayout* lastLayout = nullptr; @@ -320,30 +364,7 @@ namespace dawn_native { namespace d3d12 { } renderPassTracker->AllocateRTVAndDSVHeaps(); - - if (bindingTracker->cbvSrvUavDescriptorIndex > 0) { - // Allocate a GPU-visible heap and copy from the CPU-only heap to the GPU-visible - // heap - bindingTracker->cbvSrvUavGPUDescriptorHeap = - descriptorHeapAllocator->AllocateGPUHeap( - D3D12_DESCRIPTOR_HEAP_TYPE_CBV_SRV_UAV, - bindingTracker->cbvSrvUavDescriptorIndex); - device->GetD3D12Device()->CopyDescriptorsSimple( - bindingTracker->cbvSrvUavDescriptorIndex, - bindingTracker->cbvSrvUavGPUDescriptorHeap.GetCPUHandle(0), - bindingTracker->cbvSrvUavCPUDescriptorHeap.GetCPUHandle(0), - D3D12_DESCRIPTOR_HEAP_TYPE_CBV_SRV_UAV); - } - - if (bindingTracker->samplerDescriptorIndex > 0) { - bindingTracker->samplerGPUDescriptorHeap = descriptorHeapAllocator->AllocateGPUHeap( - D3D12_DESCRIPTOR_HEAP_TYPE_SAMPLER, bindingTracker->samplerDescriptorIndex); - device->GetD3D12Device()->CopyDescriptorsSimple( - bindingTracker->samplerDescriptorIndex, - bindingTracker->samplerGPUDescriptorHeap.GetCPUHandle(0), - bindingTracker->samplerCPUDescriptorHeap.GetCPUHandle(0), - D3D12_DESCRIPTOR_HEAP_TYPE_SAMPLER); - } + bindingTracker->AllocateDescriptorHeaps(device); } void ResolveMultisampledRenderPass(ComPtr commandList, @@ -401,17 +422,7 @@ namespace dawn_native { namespace d3d12 { AllocateAndSetDescriptorHeaps(device, &bindingTracker, &renderPassTracker, &mCommands, indexInSubmit); bindingTracker.Reset(); - - ID3D12DescriptorHeap* descriptorHeaps[2] = { - bindingTracker.cbvSrvUavGPUDescriptorHeap.Get(), - bindingTracker.samplerGPUDescriptorHeap.Get()}; - if (descriptorHeaps[0] && descriptorHeaps[1]) { - commandList->SetDescriptorHeaps(2, descriptorHeaps); - } else if (descriptorHeaps[0]) { - commandList->SetDescriptorHeaps(1, descriptorHeaps); - } else if (descriptorHeaps[1]) { - commandList->SetDescriptorHeaps(1, &descriptorHeaps[1]); - } + bindingTracker.SetID3D12DescriptorHeaps(commandList); } // Records the necessary barriers for the resource usage pre-computed by the frontend diff --git a/src/dawn_native/d3d12/CommandBufferD3D12.h b/src/dawn_native/d3d12/CommandBufferD3D12.h index 5a958016d4..694f98c9f0 100644 --- a/src/dawn_native/d3d12/CommandBufferD3D12.h +++ b/src/dawn_native/d3d12/CommandBufferD3D12.h @@ -30,12 +30,11 @@ namespace dawn_native { namespace dawn_native { namespace d3d12 { + class BindGroupStateTracker; class Device; class RenderPassDescriptorHeapTracker; class RenderPipeline; - struct BindGroupStateTracker; - struct VertexBuffersInfo { // startSlot and endSlot indicate the range of dirty vertex buffers. // If there are multiple calls to SetVertexBuffers, the start and end