D3D12: Bucket CPU descriptor allocators on the device.

Move the CPU descriptor allocators to the device and bucket them to
ensure only kMaxBindingsPerGroup exist rather than create them per BGL.

Also, renames NonShaderVisible => Staging.

BUG=dawn:155

Change-Id: If6dae368e7e2a2b349343bdf898041a049159038
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/19001
Commit-Queue: Bryan Bernhart <bryan.bernhart@intel.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

src/dawn_native/BUILD.gn

@@ -302,8 +302,6 @@ source_set("dawn_native_sources") {
       "d3d12/HeapD3D12.h",
       "d3d12/NativeSwapChainImplD3D12.cpp",
       "d3d12/NativeSwapChainImplD3D12.h",
-      "d3d12/NonShaderVisibleDescriptorAllocatorD3D12.cpp",
-      "d3d12/NonShaderVisibleDescriptorAllocatorD3D12.h",
       "d3d12/PipelineLayoutD3D12.cpp",
       "d3d12/PipelineLayoutD3D12.h",
       "d3d12/PlatformFunctions.cpp",
@@ -328,6 +326,8 @@ source_set("dawn_native_sources") {
       "d3d12/ShaderVisibleDescriptorAllocatorD3D12.h",
       "d3d12/StagingBufferD3D12.cpp",
       "d3d12/StagingBufferD3D12.h",
+      "d3d12/StagingDescriptorAllocatorD3D12.cpp",
+      "d3d12/StagingDescriptorAllocatorD3D12.h",
       "d3d12/SwapChainD3D12.cpp",
       "d3d12/SwapChainD3D12.h",
       "d3d12/TextureCopySplitter.cpp",

src/dawn_native/CMakeLists.txt

@@ -191,8 +191,6 @@ if (DAWN_ENABLE_D3D12)
         "d3d12/HeapD3D12.h"
         "d3d12/NativeSwapChainImplD3D12.cpp"
         "d3d12/NativeSwapChainImplD3D12.h"
-        "d3d12/NonShaderVisibleDescriptorAllocatorD3D12.cpp"
-        "d3d12/NonShaderVisibleDescriptorAllocatorD3D12.h"
         "d3d12/PipelineLayoutD3D12.cpp"
         "d3d12/PipelineLayoutD3D12.h"
         "d3d12/PlatformFunctions.cpp"
@@ -217,6 +215,8 @@ if (DAWN_ENABLE_D3D12)
         "d3d12/ShaderVisibleDescriptorAllocatorD3D12.h"
         "d3d12/StagingBufferD3D12.cpp"
         "d3d12/StagingBufferD3D12.h"
+        "d3d12/StagingDescriptorAllocatorD3D12.cpp"
+        "d3d12/StagingDescriptorAllocatorD3D12.h"
         "d3d12/SwapChainD3D12.cpp"
         "d3d12/SwapChainD3D12.h"
         "d3d12/TextureCopySplitter.cpp"

src/dawn_native/d3d12/BindGroupLayoutD3D12.cpp

@@ -17,7 +17,7 @@
 #include "common/BitSetIterator.h"
 #include "dawn_native/d3d12/BindGroupD3D12.h"
 #include "dawn_native/d3d12/DeviceD3D12.h"
-#include "dawn_native/d3d12/NonShaderVisibleDescriptorAllocatorD3D12.h"
+#include "dawn_native/d3d12/StagingDescriptorAllocatorD3D12.h"
 
 namespace dawn_native { namespace d3d12 {
     namespace {
@@ -42,9 +42,6 @@ namespace dawn_native { namespace d3d12 {
         }
     }  // anonymous namespace
 
-    // TODO(dawn:155): Figure out this value.
-    static constexpr uint16_t kDescriptorHeapSize = 1024;
-
     BindGroupLayout::BindGroupLayout(Device* device, const BindGroupLayoutDescriptor* descriptor)
         : BindGroupLayoutBase(device, descriptor),
           mDescriptorCounts{},
@@ -133,19 +130,9 @@ namespace dawn_native { namespace d3d12 {
             mBindingOffsets[bindingIndex] += descriptorOffsets[descriptorType];
         }
 
-        const uint32_t viewDescriptorCount = GetCbvUavSrvDescriptorCount();
+        mViewAllocator = device->GetViewStagingDescriptorAllocator(GetCbvUavSrvDescriptorCount());
-        if (viewDescriptorCount > 0) {
+        mSamplerAllocator =
-            mViewAllocator = std::make_unique<NonShaderVisibleDescriptorAllocator>(
+            device->GetSamplerStagingDescriptorAllocator(GetSamplerDescriptorCount());
-                device, viewDescriptorCount, kDescriptorHeapSize,
-                D3D12_DESCRIPTOR_HEAP_TYPE_CBV_SRV_UAV);
-        }
-
-        const uint32_t samplerDescriptorCount = GetSamplerDescriptorCount();
-        if (samplerDescriptorCount > 0) {
-            mSamplerAllocator = std::make_unique<NonShaderVisibleDescriptorAllocator>(
-                device, samplerDescriptorCount, kDescriptorHeapSize,
-                D3D12_DESCRIPTOR_HEAP_TYPE_SAMPLER);
-        }
     }
 
     ResultOrError<BindGroup*> BindGroupLayout::AllocateBindGroup(

src/dawn_native/d3d12/BindGroupLayoutD3D12.h

@@ -23,9 +23,9 @@
 namespace dawn_native { namespace d3d12 {
 
     class BindGroup;
-    class Device;
-    class NonShaderVisibleDescriptorAllocator;
     class CPUDescriptorHeapAllocation;
+    class Device;
+    class StagingDescriptorAllocator;
 
     class BindGroupLayout final : public BindGroupLayoutBase {
       public:
@@ -61,9 +61,8 @@ namespace dawn_native { namespace d3d12 {
 
         SlabAllocator<BindGroup> mBindGroupAllocator;
 
-        // TODO(dawn:155): Store and bucket allocators by size on the device.
+        StagingDescriptorAllocator* mSamplerAllocator = nullptr;
-        std::unique_ptr<NonShaderVisibleDescriptorAllocator> mSamplerAllocator;
+        StagingDescriptorAllocator* mViewAllocator = nullptr;
-        std::unique_ptr<NonShaderVisibleDescriptorAllocator> mViewAllocator;
     };
 
 }}  // namespace dawn_native::d3d12

src/dawn_native/d3d12/DeviceD3D12.cpp

@@ -37,11 +37,15 @@
 #include "dawn_native/d3d12/ShaderModuleD3D12.h"
 #include "dawn_native/d3d12/ShaderVisibleDescriptorAllocatorD3D12.h"
 #include "dawn_native/d3d12/StagingBufferD3D12.h"
+#include "dawn_native/d3d12/StagingDescriptorAllocatorD3D12.h"
 #include "dawn_native/d3d12/SwapChainD3D12.h"
 #include "dawn_native/d3d12/TextureD3D12.h"
 
 namespace dawn_native { namespace d3d12 {
 
+    // TODO(dawn:155): Figure out this value.
+    static constexpr uint16_t kStagingDescriptorHeapSize = 1024;
+
     // static
     ResultOrError<Device*> Device::Create(Adapter* adapter, const DeviceDescriptor* descriptor) {
         Ref<Device> device = AcquireRef(new Device(adapter, descriptor));
@@ -83,6 +87,17 @@ namespace dawn_native { namespace d3d12 {
             std::make_unique<ShaderVisibleDescriptorAllocator>(this);
         DAWN_TRY(mShaderVisibleDescriptorAllocator->Initialize());
 
+        // Zero sized allocator is never requested and does not need to exist.
+        for (uint32_t countIndex = 1; countIndex < kNumOfStagingDescriptorAllocators;
+             countIndex++) {
+            mViewAllocators[countIndex] = std::make_unique<StagingDescriptorAllocator>(
+                this, countIndex, kStagingDescriptorHeapSize,
+                D3D12_DESCRIPTOR_HEAP_TYPE_CBV_SRV_UAV);
+
+            mSamplerAllocators[countIndex] = std::make_unique<StagingDescriptorAllocator>(
+                this, countIndex, kStagingDescriptorHeapSize, D3D12_DESCRIPTOR_HEAP_TYPE_SAMPLER);
+        }
+
         mMapRequestTracker = std::make_unique<MapRequestTracker>(this);
         mResidencyManager = std::make_unique<ResidencyManager>(this);
         mResourceAllocatorManager = std::make_unique<ResourceAllocatorManager>(this);
@@ -458,4 +473,16 @@ namespace dawn_native { namespace d3d12 {
     ShaderVisibleDescriptorAllocator* Device::GetShaderVisibleDescriptorAllocator() const {
         return mShaderVisibleDescriptorAllocator.get();
     }
+
+    StagingDescriptorAllocator* Device::GetViewStagingDescriptorAllocator(
+        uint32_t descriptorCount) const {
+        ASSERT(descriptorCount < kNumOfStagingDescriptorAllocators);
+        return mViewAllocators[descriptorCount].get();
+    }
+
+    StagingDescriptorAllocator* Device::GetSamplerStagingDescriptorAllocator(
+        uint32_t descriptorCount) const {
+        ASSERT(descriptorCount < kNumOfStagingDescriptorAllocators);
+        return mSamplerAllocators[descriptorCount].get();
+    }
 }}  // namespace dawn_native::d3d12

src/dawn_native/d3d12/DeviceD3D12.h

@@ -17,6 +17,7 @@
 
 #include "dawn_native/dawn_platform.h"
 
+#include "common/Constants.h"
 #include "common/SerialQueue.h"
 #include "dawn_native/Device.h"
 #include "dawn_native/d3d12/CommandRecordingContext.h"
@@ -30,11 +31,12 @@ namespace dawn_native { namespace d3d12 {
 
     class CommandAllocatorManager;
     class DescriptorHeapAllocator;
-    class ShaderVisibleDescriptorAllocator;
     class MapRequestTracker;
     class PlatformFunctions;
-    class ResourceAllocatorManager;
     class ResidencyManager;
+    class ResourceAllocatorManager;
+    class ShaderVisibleDescriptorAllocator;
+    class StagingDescriptorAllocator;
 
 #define ASSERT_SUCCESS(hr)            \
     {                                 \
@@ -101,6 +103,13 @@ namespace dawn_native { namespace d3d12 {
 
         ShaderVisibleDescriptorAllocator* GetShaderVisibleDescriptorAllocator() const;
 
+        // Returns nullptr when descriptor count is zero.
+        StagingDescriptorAllocator* GetViewStagingDescriptorAllocator(
+            uint32_t descriptorCount) const;
+
+        StagingDescriptorAllocator* GetSamplerStagingDescriptorAllocator(
+            uint32_t descriptorCount) const;
+
         TextureBase* WrapSharedHandle(const ExternalImageDescriptor* descriptor,
                                       HANDLE sharedHandle,
                                       uint64_t acquireMutexKey,
@@ -170,6 +179,15 @@ namespace dawn_native { namespace d3d12 {
         std::unique_ptr<ResourceAllocatorManager> mResourceAllocatorManager;
         std::unique_ptr<ResidencyManager> mResidencyManager;
         std::unique_ptr<ShaderVisibleDescriptorAllocator> mShaderVisibleDescriptorAllocator;
+
+        // Index corresponds to the descriptor count in the range [0, kMaxBindingsPerGroup].
+        static constexpr uint32_t kNumOfStagingDescriptorAllocators = kMaxBindingsPerGroup + 1;
+
+        std::array<std::unique_ptr<StagingDescriptorAllocator>, kNumOfStagingDescriptorAllocators>
+            mViewAllocators;
+
+        std::array<std::unique_ptr<StagingDescriptorAllocator>, kNumOfStagingDescriptorAllocators>
+            mSamplerAllocators;
     };
 
 }}  // namespace dawn_native::d3d12

src/dawn_native/d3d12/StagingDescriptorAllocatorD3D12.cpp

@@ -16,15 +16,14 @@
 
 #include "dawn_native/d3d12/D3D12Error.h"
 #include "dawn_native/d3d12/DeviceD3D12.h"
-#include "dawn_native/d3d12/NonShaderVisibleDescriptorAllocatorD3D12.h"
+#include "dawn_native/d3d12/StagingDescriptorAllocatorD3D12.h"
 
 namespace dawn_native { namespace d3d12 {
 
-    NonShaderVisibleDescriptorAllocator::NonShaderVisibleDescriptorAllocator(
+    StagingDescriptorAllocator::StagingDescriptorAllocator(Device* device,
-        Device* device,
+                                                           uint32_t descriptorCount,
-        uint32_t descriptorCount,
+                                                           uint32_t heapSize,
-        uint32_t heapSize,
+                                                           D3D12_DESCRIPTOR_HEAP_TYPE heapType)
-        D3D12_DESCRIPTOR_HEAP_TYPE heapType)
         : mDevice(device),
           mSizeIncrement(device->GetD3D12Device()->GetDescriptorHandleIncrementSize(heapType)),
           mBlockSize(descriptorCount * mSizeIncrement),
@@ -35,7 +34,7 @@ namespace dawn_native { namespace d3d12 {
         ASSERT(descriptorCount <= heapSize);
     }
 
-    NonShaderVisibleDescriptorAllocator::~NonShaderVisibleDescriptorAllocator() {
+    StagingDescriptorAllocator::~StagingDescriptorAllocator() {
         const Index freeBlockIndicesSize = GetFreeBlockIndicesSize();
         for (auto& buffer : mPool) {
             ASSERT(buffer.freeBlockIndices.size() == freeBlockIndicesSize);
@@ -44,7 +43,7 @@ namespace dawn_native { namespace d3d12 {
     }
 
     ResultOrError<CPUDescriptorHeapAllocation>
-    NonShaderVisibleDescriptorAllocator::AllocateCPUDescriptors() {
+    StagingDescriptorAllocator::AllocateCPUDescriptors() {
         if (mAvailableHeaps.empty()) {
             DAWN_TRY(AllocateCPUHeap());
         }
@@ -70,7 +69,7 @@ namespace dawn_native { namespace d3d12 {
         return CPUDescriptorHeapAllocation{baseCPUDescriptor, heapIndex};
     }
 
-    MaybeError NonShaderVisibleDescriptorAllocator::AllocateCPUHeap() {
+    MaybeError StagingDescriptorAllocator::AllocateCPUHeap() {
         D3D12_DESCRIPTOR_HEAP_DESC heapDescriptor;
         heapDescriptor.Type = mHeapType;
         heapDescriptor.NumDescriptors = mHeapSize;
@@ -98,7 +97,7 @@ namespace dawn_native { namespace d3d12 {
         return {};
     }
 
-    void NonShaderVisibleDescriptorAllocator::Deallocate(CPUDescriptorHeapAllocation* allocation) {
+    void StagingDescriptorAllocator::Deallocate(CPUDescriptorHeapAllocation* allocation) {
         const uint32_t heapIndex = allocation->GetHeapIndex();
 
         ASSERT(heapIndex < mPool.size());
@@ -125,12 +124,11 @@ namespace dawn_native { namespace d3d12 {
         allocation->Invalidate();
     }
 
-    uint32_t NonShaderVisibleDescriptorAllocator::GetSizeIncrement() const {
+    uint32_t StagingDescriptorAllocator::GetSizeIncrement() const {
         return mSizeIncrement;
     }
 
-    NonShaderVisibleDescriptorAllocator::Index
+    StagingDescriptorAllocator::Index StagingDescriptorAllocator::GetFreeBlockIndicesSize() const {
-    NonShaderVisibleDescriptorAllocator::GetFreeBlockIndicesSize() const {
         return ((mHeapSize * mSizeIncrement) / mBlockSize);
     }
 

src/dawn_native/d3d12/StagingDescriptorAllocatorD3D12.h

@@ -12,8 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#ifndef DAWNNATIVE_D3D12_NONSHADERVISIBLEDESCRIPTORALLOCATOR_H_
+#ifndef DAWNNATIVE_D3D12_STAGINGDESCRIPTORALLOCATOR_H_
-#define DAWNNATIVE_D3D12_NONSHADERVISIBLEDESCRIPTORALLOCATOR_H_
+#define DAWNNATIVE_D3D12_STAGINGDESCRIPTORALLOCATOR_H_
 
 #include "dawn_native/Error.h"
 
@@ -21,7 +21,7 @@
 
 #include <vector>
 
-// |NonShaderVisibleDescriptorAllocator| allocates a fixed-size block of descriptors from a CPU
+// |StagingDescriptorAllocator| allocates a fixed-size block of descriptors from a CPU
 // descriptor heap pool.
 // Internally, it manages a list of heaps using a fixed-size block allocator. The fixed-size
 // block allocator is backed by a list of free blocks (free-list). The heap is in one of two
@@ -34,14 +34,14 @@ namespace dawn_native { namespace d3d12 {
 
     class Device;
 
-    class NonShaderVisibleDescriptorAllocator {
+    class StagingDescriptorAllocator {
       public:
-        NonShaderVisibleDescriptorAllocator() = default;
+        StagingDescriptorAllocator() = default;
-        NonShaderVisibleDescriptorAllocator(Device* device,
+        StagingDescriptorAllocator(Device* device,
-                                            uint32_t descriptorCount,
+                                   uint32_t descriptorCount,
-                                            uint32_t heapSize,
+                                   uint32_t heapSize,
-                                            D3D12_DESCRIPTOR_HEAP_TYPE heapType);
+                                   D3D12_DESCRIPTOR_HEAP_TYPE heapType);
-        ~NonShaderVisibleDescriptorAllocator();
+        ~StagingDescriptorAllocator();
 
         ResultOrError<CPUDescriptorHeapAllocation> AllocateCPUDescriptors();
 
@@ -75,4 +75,4 @@ namespace dawn_native { namespace d3d12 {
 
 }}  // namespace dawn_native::d3d12
 
-#endif  // DAWNNATIVE_D3D12_NONSHADERVISIBLEDESCRIPTORALLOCATOR_H_
+#endif  // DAWNNATIVE_D3D12_STAGINGDESCRIPTORALLOCATOR_H_

src/tests/white_box/D3D12DescriptorHeapTests.cpp

@@ -17,8 +17,8 @@
 #include "dawn_native/Toggles.h"
 #include "dawn_native/d3d12/BindGroupLayoutD3D12.h"
 #include "dawn_native/d3d12/DeviceD3D12.h"
-#include "dawn_native/d3d12/NonShaderVisibleDescriptorAllocatorD3D12.h"
 #include "dawn_native/d3d12/ShaderVisibleDescriptorAllocatorD3D12.h"
+#include "dawn_native/d3d12/StagingDescriptorAllocatorD3D12.h"
 #include "utils/ComboRenderPipelineDescriptor.h"
 #include "utils/WGPUHelpers.h"
 
@@ -94,11 +94,11 @@ class D3D12DescriptorHeapTests : public DawnTest {
     wgpu::ShaderModule mSimpleFSModule;
 };
 
-class DummyNonShaderVisibleDescriptorAllocator {
+class DummyStagingDescriptorAllocator {
   public:
-    DummyNonShaderVisibleDescriptorAllocator(Device* device,
+    DummyStagingDescriptorAllocator(Device* device,
-                                             uint32_t descriptorCount,
+                                    uint32_t descriptorCount,
-                                             uint32_t allocationsPerHeap)
+                                    uint32_t allocationsPerHeap)
         : mAllocator(device,
                      descriptorCount,
                      allocationsPerHeap * descriptorCount,
@@ -116,7 +116,7 @@ class DummyNonShaderVisibleDescriptorAllocator {
     }
 
   private:
-    NonShaderVisibleDescriptorAllocator mAllocator;
+    StagingDescriptorAllocator mAllocator;
 };
 
 // Verify the shader visible heaps switch over within a single submit.
@@ -719,8 +719,7 @@ TEST_P(D3D12DescriptorHeapTests, EncodeManyUBOAndSamplers) {
 TEST_P(D3D12DescriptorHeapTests, Single) {
     constexpr uint32_t kDescriptorCount = 4;
     constexpr uint32_t kAllocationsPerHeap = 3;
-    DummyNonShaderVisibleDescriptorAllocator allocator(mD3DDevice, kDescriptorCount,
+    DummyStagingDescriptorAllocator allocator(mD3DDevice, kDescriptorCount, kAllocationsPerHeap);
-                                                       kAllocationsPerHeap);
 
     CPUDescriptorHeapAllocation allocation = allocator.AllocateCPUDescriptors();
     EXPECT_EQ(allocation.GetHeapIndex(), 0u);
@@ -735,8 +734,7 @@ TEST_P(D3D12DescriptorHeapTests, Single) {
 TEST_P(D3D12DescriptorHeapTests, Sequential) {
     constexpr uint32_t kDescriptorCount = 4;
     constexpr uint32_t kAllocationsPerHeap = 3;
-    DummyNonShaderVisibleDescriptorAllocator allocator(mD3DDevice, kDescriptorCount,
+    DummyStagingDescriptorAllocator allocator(mD3DDevice, kDescriptorCount, kAllocationsPerHeap);
-                                                       kAllocationsPerHeap);
 
     // Allocate |kNumOfHeaps| worth.
     constexpr uint32_t kNumOfHeaps = 2;
@@ -766,8 +764,7 @@ TEST_P(D3D12DescriptorHeapTests, Sequential) {
 TEST_P(D3D12DescriptorHeapTests, ReuseFreedHeaps) {
     constexpr uint32_t kDescriptorCount = 4;
     constexpr uint32_t kAllocationsPerHeap = 25;
-    DummyNonShaderVisibleDescriptorAllocator allocator(mD3DDevice, kDescriptorCount,
+    DummyStagingDescriptorAllocator allocator(mD3DDevice, kDescriptorCount, kAllocationsPerHeap);
-                                                       kAllocationsPerHeap);
 
     constexpr uint32_t kNumofHeaps = 10;
 
@@ -810,8 +807,7 @@ TEST_P(D3D12DescriptorHeapTests, ReuseFreedHeaps) {
 TEST_P(D3D12DescriptorHeapTests, AllocateDeallocateMany) {
     constexpr uint32_t kDescriptorCount = 4;
     constexpr uint32_t kAllocationsPerHeap = 25;
-    DummyNonShaderVisibleDescriptorAllocator allocator(mD3DDevice, kDescriptorCount,
+    DummyStagingDescriptorAllocator allocator(mD3DDevice, kDescriptorCount, kAllocationsPerHeap);
-                                                       kAllocationsPerHeap);
 
     std::list<CPUDescriptorHeapAllocation> list3;
     std::list<CPUDescriptorHeapAllocation> list5;