From cac14e006784e6559b0a9d8f39cb0a234ef45f7a Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Mon, 28 Sep 2020 16:05:24 +0000 Subject: [PATCH] D3D12: Use typed integers for the HeapVersionID This will prevent mixing it up with other serial types in the future. Bug: dawn:442 Change-Id: I32f356c62f19ef29f3bf51c19873369fb817bb16 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/28960 Commit-Queue: Corentin Wallez Reviewed-by: Jiawei Shao Reviewed-by: Austin Eng --- src/dawn_native/BUILD.gn | 1 + src/dawn_native/CMakeLists.txt | 1 + .../GPUDescriptorHeapAllocationD3D12.cpp | 4 +- .../d3d12/GPUDescriptorHeapAllocationD3D12.h | 7 ++-- src/dawn_native/d3d12/IntegerTypes.h | 31 +++++++++++++++ .../ShaderVisibleDescriptorAllocatorD3D12.cpp | 2 +- .../ShaderVisibleDescriptorAllocatorD3D12.h | 5 ++- .../white_box/D3D12DescriptorHeapTests.cpp | 38 +++++++++++-------- src/tests/white_box/D3D12ResidencyTests.cpp | 6 ++- 9 files changed, 69 insertions(+), 26 deletions(-) create mode 100644 src/dawn_native/d3d12/IntegerTypes.h diff --git a/src/dawn_native/BUILD.gn b/src/dawn_native/BUILD.gn index b9dfd14000..0cfe3a745e 100644 --- a/src/dawn_native/BUILD.gn +++ b/src/dawn_native/BUILD.gn @@ -314,6 +314,7 @@ source_set("dawn_native_sources") { "d3d12/HeapAllocatorD3D12.h", "d3d12/HeapD3D12.cpp", "d3d12/HeapD3D12.h", + "d3d12/IntegerTypes.h", "d3d12/NativeSwapChainImplD3D12.cpp", "d3d12/NativeSwapChainImplD3D12.h", "d3d12/PageableD3D12.cpp", diff --git a/src/dawn_native/CMakeLists.txt b/src/dawn_native/CMakeLists.txt index e5df12a0f1..7aba5b1323 100644 --- a/src/dawn_native/CMakeLists.txt +++ b/src/dawn_native/CMakeLists.txt @@ -209,6 +209,7 @@ if (DAWN_ENABLE_D3D12) "d3d12/HeapAllocatorD3D12.h" "d3d12/HeapD3D12.cpp" "d3d12/HeapD3D12.h" + "d3d12/IntegerTypes.h" "d3d12/NativeSwapChainImplD3D12.cpp" "d3d12/NativeSwapChainImplD3D12.h" "d3d12/PageableD3D12.cpp" diff --git a/src/dawn_native/d3d12/GPUDescriptorHeapAllocationD3D12.cpp b/src/dawn_native/d3d12/GPUDescriptorHeapAllocationD3D12.cpp index 3d3ba20fad..24cb7759be 100644 --- a/src/dawn_native/d3d12/GPUDescriptorHeapAllocationD3D12.cpp +++ b/src/dawn_native/d3d12/GPUDescriptorHeapAllocationD3D12.cpp @@ -19,7 +19,7 @@ namespace dawn_native { namespace d3d12 { GPUDescriptorHeapAllocation::GPUDescriptorHeapAllocation( D3D12_GPU_DESCRIPTOR_HANDLE baseDescriptor, Serial lastUsageSerial, - Serial heapSerial) + HeapVersionID heapSerial) : mBaseDescriptor(baseDescriptor), mLastUsageSerial(lastUsageSerial), mHeapSerial(heapSerial) { @@ -33,7 +33,7 @@ namespace dawn_native { namespace d3d12 { return mLastUsageSerial; } - Serial GPUDescriptorHeapAllocation::GetHeapSerial() const { + HeapVersionID GPUDescriptorHeapAllocation::GetHeapSerial() const { return mHeapSerial; } }} // namespace dawn_native::d3d12 diff --git a/src/dawn_native/d3d12/GPUDescriptorHeapAllocationD3D12.h b/src/dawn_native/d3d12/GPUDescriptorHeapAllocationD3D12.h index 08930d79fc..fa1a5a3140 100644 --- a/src/dawn_native/d3d12/GPUDescriptorHeapAllocationD3D12.h +++ b/src/dawn_native/d3d12/GPUDescriptorHeapAllocationD3D12.h @@ -16,6 +16,7 @@ #define DAWNNATIVE_D3D12_GPUDESCRIPTORHEAPALLOCATION_H_ #include "common/Serial.h" +#include "dawn_native/d3d12/IntegerTypes.h" #include "dawn_native/d3d12/d3d12_platform.h" namespace dawn_native { namespace d3d12 { @@ -26,16 +27,16 @@ namespace dawn_native { namespace d3d12 { GPUDescriptorHeapAllocation() = default; GPUDescriptorHeapAllocation(D3D12_GPU_DESCRIPTOR_HANDLE baseDescriptor, Serial lastUsageSerial, - Serial heapSerial); + HeapVersionID heapSerial); D3D12_GPU_DESCRIPTOR_HANDLE GetBaseDescriptor() const; Serial GetLastUsageSerial() const; - Serial GetHeapSerial() const; + HeapVersionID GetHeapSerial() const; private: D3D12_GPU_DESCRIPTOR_HANDLE mBaseDescriptor = {0}; Serial mLastUsageSerial = 0; - Serial mHeapSerial = 0; + HeapVersionID mHeapSerial = HeapVersionID(0); }; }} // namespace dawn_native::d3d12 diff --git a/src/dawn_native/d3d12/IntegerTypes.h b/src/dawn_native/d3d12/IntegerTypes.h new file mode 100644 index 0000000000..1c16cfd47a --- /dev/null +++ b/src/dawn_native/d3d12/IntegerTypes.h @@ -0,0 +1,31 @@ +// 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. + +#ifndef DAWNNATIVE_D3D12_INTEGERTYPES_H_ +#define DAWNNATIVE_D3D12_INTEGERTYPES_H_ + +#include "common/Constants.h" +#include "common/TypedInteger.h" + +#include + +namespace dawn_native { namespace d3d12 { + + // An ID used to desambiguate between multiple uses of the same descriptor heap in the + // BindGroup allocations. + using HeapVersionID = TypedInteger; + +}} // namespace dawn_native::d3d12 + +#endif // DAWNNATIVE_D3D12_INTEGERTYPES_H_ diff --git a/src/dawn_native/d3d12/ShaderVisibleDescriptorAllocatorD3D12.cpp b/src/dawn_native/d3d12/ShaderVisibleDescriptorAllocatorD3D12.cpp index ba1b4939a2..b9053dc73a 100644 --- a/src/dawn_native/d3d12/ShaderVisibleDescriptorAllocatorD3D12.cpp +++ b/src/dawn_native/d3d12/ShaderVisibleDescriptorAllocatorD3D12.cpp @@ -209,7 +209,7 @@ namespace dawn_native { namespace d3d12 { return {}; } - Serial ShaderVisibleDescriptorAllocator::GetShaderVisibleHeapSerialForTesting() const { + HeapVersionID ShaderVisibleDescriptorAllocator::GetShaderVisibleHeapSerialForTesting() const { return mHeapSerial; } diff --git a/src/dawn_native/d3d12/ShaderVisibleDescriptorAllocatorD3D12.h b/src/dawn_native/d3d12/ShaderVisibleDescriptorAllocatorD3D12.h index d93e57a073..7bca87724d 100644 --- a/src/dawn_native/d3d12/ShaderVisibleDescriptorAllocatorD3D12.h +++ b/src/dawn_native/d3d12/ShaderVisibleDescriptorAllocatorD3D12.h @@ -17,6 +17,7 @@ #include "dawn_native/Error.h" #include "dawn_native/RingBufferAllocator.h" +#include "dawn_native/d3d12/IntegerTypes.h" #include "dawn_native/d3d12/PageableD3D12.h" #include "dawn_native/d3d12/d3d12_platform.h" @@ -64,7 +65,7 @@ namespace dawn_native { namespace d3d12 { MaybeError AllocateAndSwitchShaderVisibleHeap(); // For testing purposes only. - Serial GetShaderVisibleHeapSerialForTesting() const; + HeapVersionID GetShaderVisibleHeapSerialForTesting() const; uint64_t GetShaderVisibleHeapSizeForTesting() const; uint64_t GetShaderVisiblePoolSizeForTesting() const; bool IsShaderVisibleHeapLockedResidentForTesting() const; @@ -91,7 +92,7 @@ namespace dawn_native { namespace d3d12 { // The serial value of 0 means the shader-visible heaps have not been allocated. // This value is never returned in the GPUDescriptorHeapAllocation after // AllocateGPUDescriptors() is called. - Serial mHeapSerial = 0; + HeapVersionID mHeapSerial = HeapVersionID(0); uint32_t mSizeIncrement; diff --git a/src/tests/white_box/D3D12DescriptorHeapTests.cpp b/src/tests/white_box/D3D12DescriptorHeapTests.cpp index 0e6785d8cf..7293665e7b 100644 --- a/src/tests/white_box/D3D12DescriptorHeapTests.cpp +++ b/src/tests/white_box/D3D12DescriptorHeapTests.cpp @@ -134,7 +134,7 @@ TEST_P(D3D12DescriptorHeapTests, SwitchOverViewHeap) { d3dDevice->GetViewShaderVisibleDescriptorAllocator(); const uint64_t heapSize = allocator->GetShaderVisibleHeapSizeForTesting(); - const Serial heapSerial = allocator->GetShaderVisibleHeapSerialForTesting(); + const HeapVersionID heapSerial = allocator->GetShaderVisibleHeapSerialForTesting(); wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); { @@ -158,7 +158,7 @@ TEST_P(D3D12DescriptorHeapTests, SwitchOverViewHeap) { wgpu::CommandBuffer commands = encoder.Finish(); queue.Submit(1, &commands); - EXPECT_EQ(allocator->GetShaderVisibleHeapSerialForTesting(), heapSerial + 1); + EXPECT_EQ(allocator->GetShaderVisibleHeapSerialForTesting(), heapSerial + HeapVersionID(1)); } // Verify the shader visible sampler heaps does not switch over within a single submit. @@ -194,7 +194,7 @@ TEST_P(D3D12DescriptorHeapTests, NoSwitchOverSamplerHeap) { d3dDevice->GetSamplerShaderVisibleDescriptorAllocator(); const uint64_t samplerHeapSize = allocator->GetShaderVisibleHeapSizeForTesting(); - const Serial heapSerial = allocator->GetShaderVisibleHeapSerialForTesting(); + const HeapVersionID HeapVersionID = allocator->GetShaderVisibleHeapSerialForTesting(); wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); { @@ -214,7 +214,7 @@ TEST_P(D3D12DescriptorHeapTests, NoSwitchOverSamplerHeap) { wgpu::CommandBuffer commands = encoder.Finish(); queue.Submit(1, &commands); - EXPECT_EQ(allocator->GetShaderVisibleHeapSerialForTesting(), heapSerial); + EXPECT_EQ(allocator->GetShaderVisibleHeapSerialForTesting(), HeapVersionID); } // Verify shader-visible heaps can be recycled for multiple submits. @@ -265,7 +265,7 @@ TEST_P(D3D12DescriptorHeapTests, PoolHeapsInPendingSubmit) { ShaderVisibleDescriptorAllocator* allocator = mD3DDevice->GetSamplerShaderVisibleDescriptorAllocator(); - const Serial heapSerial = allocator->GetShaderVisibleHeapSerialForTesting(); + const HeapVersionID heapSerial = allocator->GetShaderVisibleHeapSerialForTesting(); std::set> heaps = {allocator->GetShaderVisibleHeap()}; @@ -280,7 +280,8 @@ TEST_P(D3D12DescriptorHeapTests, PoolHeapsInPendingSubmit) { } // After |kNumOfSwitches|, no heaps are recycled. - EXPECT_EQ(allocator->GetShaderVisibleHeapSerialForTesting(), heapSerial + kNumOfSwitches); + EXPECT_EQ(allocator->GetShaderVisibleHeapSerialForTesting(), + heapSerial + HeapVersionID(kNumOfSwitches)); EXPECT_EQ(allocator->GetShaderVisiblePoolSizeForTesting(), kNumOfSwitches); } @@ -295,7 +296,7 @@ TEST_P(D3D12DescriptorHeapTests, PoolHeapsInPendingAndMultipleSubmits) { ShaderVisibleDescriptorAllocator* allocator = mD3DDevice->GetSamplerShaderVisibleDescriptorAllocator(); - const Serial heapSerial = allocator->GetShaderVisibleHeapSerialForTesting(); + const HeapVersionID heapSerial = allocator->GetShaderVisibleHeapSerialForTesting(); std::set> heaps = {allocator->GetShaderVisibleHeap()}; @@ -309,7 +310,8 @@ TEST_P(D3D12DescriptorHeapTests, PoolHeapsInPendingAndMultipleSubmits) { heaps.insert(heap); } - EXPECT_EQ(allocator->GetShaderVisibleHeapSerialForTesting(), heapSerial + kNumOfSwitches); + EXPECT_EQ(allocator->GetShaderVisibleHeapSerialForTesting(), + heapSerial + HeapVersionID(kNumOfSwitches)); EXPECT_EQ(allocator->GetShaderVisiblePoolSizeForTesting(), kNumOfSwitches); // Ensure switched-over heaps can be recycled by advancing the GPU by at-least |kFrameDepth|. @@ -326,7 +328,8 @@ TEST_P(D3D12DescriptorHeapTests, PoolHeapsInPendingAndMultipleSubmits) { } // After switching-over |kNumOfSwitches| x 2, ensure no additional heaps exist. - EXPECT_EQ(allocator->GetShaderVisibleHeapSerialForTesting(), heapSerial + kNumOfSwitches * 2); + EXPECT_EQ(allocator->GetShaderVisibleHeapSerialForTesting(), + heapSerial + HeapVersionID(kNumOfSwitches * 2)); EXPECT_EQ(allocator->GetShaderVisiblePoolSizeForTesting(), kNumOfSwitches); } @@ -335,7 +338,7 @@ TEST_P(D3D12DescriptorHeapTests, GrowHeapsInMultipleSubmits) { ShaderVisibleDescriptorAllocator* allocator = mD3DDevice->GetSamplerShaderVisibleDescriptorAllocator(); - const Serial heapSerial = allocator->GetShaderVisibleHeapSerialForTesting(); + const HeapVersionID heapSerial = allocator->GetShaderVisibleHeapSerialForTesting(); std::set> heaps = {allocator->GetShaderVisibleHeap()}; @@ -352,7 +355,8 @@ TEST_P(D3D12DescriptorHeapTests, GrowHeapsInMultipleSubmits) { // Verify the number of switches equals the size of heaps allocated (minus the initial). EXPECT_EQ(allocator->GetShaderVisiblePoolSizeForTesting(), 1u); - EXPECT_EQ(allocator->GetShaderVisibleHeapSerialForTesting(), heapSerial + heaps.size() - 1); + EXPECT_EQ(allocator->GetShaderVisibleHeapSerialForTesting(), + heapSerial + HeapVersionID(heaps.size() - 1)); } // Verify shader-visible heaps do not recycle in a pending submit. @@ -360,7 +364,7 @@ TEST_P(D3D12DescriptorHeapTests, GrowHeapsInPendingSubmit) { ShaderVisibleDescriptorAllocator* allocator = mD3DDevice->GetSamplerShaderVisibleDescriptorAllocator(); - const Serial heapSerial = allocator->GetShaderVisibleHeapSerialForTesting(); + const HeapVersionID heapSerial = allocator->GetShaderVisibleHeapSerialForTesting(); std::set> heaps = {allocator->GetShaderVisibleHeap()}; @@ -376,7 +380,8 @@ TEST_P(D3D12DescriptorHeapTests, GrowHeapsInPendingSubmit) { // Verify the number of switches equals the size of heaps allocated (minus the initial). EXPECT_EQ(allocator->GetShaderVisiblePoolSizeForTesting(), 1u); - EXPECT_EQ(allocator->GetShaderVisibleHeapSerialForTesting(), heapSerial + heaps.size() - 1); + EXPECT_EQ(allocator->GetShaderVisibleHeapSerialForTesting(), + heapSerial + HeapVersionID(heaps.size() - 1)); } // Verify switching shader-visible heaps do not recycle in a pending submit but do so @@ -806,8 +811,9 @@ TEST_P(D3D12DescriptorHeapTests, EncodeManyUBOAndSamplers) { ShaderVisibleDescriptorAllocator* samplerAllocator = mD3DDevice->GetSamplerShaderVisibleDescriptorAllocator(); - const Serial viewHeapSerial = viewAllocator->GetShaderVisibleHeapSerialForTesting(); - const Serial samplerHeapSerial = samplerAllocator->GetShaderVisibleHeapSerialForTesting(); + const HeapVersionID viewHeapSerial = viewAllocator->GetShaderVisibleHeapSerialForTesting(); + const HeapVersionID samplerHeapSerial = + samplerAllocator->GetShaderVisibleHeapSerialForTesting(); const uint32_t viewHeapSize = viewAllocator->GetShaderVisibleHeapSizeForTesting(); @@ -867,7 +873,7 @@ TEST_P(D3D12DescriptorHeapTests, EncodeManyUBOAndSamplers) { EXPECT_EQ(viewAllocator->GetShaderVisiblePoolSizeForTesting(), kNumOfViewHeaps); EXPECT_EQ(viewAllocator->GetShaderVisibleHeapSerialForTesting(), - viewHeapSerial + kNumOfViewHeaps); + viewHeapSerial + HeapVersionID(kNumOfViewHeaps)); EXPECT_EQ(samplerAllocator->GetShaderVisiblePoolSizeForTesting(), 0u); EXPECT_EQ(samplerAllocator->GetShaderVisibleHeapSerialForTesting(), samplerHeapSerial); diff --git a/src/tests/white_box/D3D12ResidencyTests.cpp b/src/tests/white_box/D3D12ResidencyTests.cpp index b8b439c215..b9be954283 100644 --- a/src/tests/white_box/D3D12ResidencyTests.cpp +++ b/src/tests/white_box/D3D12ResidencyTests.cpp @@ -367,7 +367,8 @@ TEST_P(D3D12DescriptorResidencyTests, SwitchedViewHeapResidency) { d3dDevice->GetViewShaderVisibleDescriptorAllocator(); const uint64_t heapSize = allocator->GetShaderVisibleHeapSizeForTesting(); - const Serial heapSerial = allocator->GetShaderVisibleHeapSerialForTesting(); + const dawn_native::d3d12::HeapVersionID heapSerial = + allocator->GetShaderVisibleHeapSerialForTesting(); wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); { @@ -392,7 +393,8 @@ TEST_P(D3D12DescriptorResidencyTests, SwitchedViewHeapResidency) { queue.Submit(1, &commands); // Check the heap serial to ensure the heap has switched. - EXPECT_EQ(allocator->GetShaderVisibleHeapSerialForTesting(), heapSerial + 1); + EXPECT_EQ(allocator->GetShaderVisibleHeapSerialForTesting(), + heapSerial + dawn_native::d3d12::HeapVersionID(1)); // Check that currrently bound ShaderVisibleHeap is locked resident. EXPECT_TRUE(allocator->IsShaderVisibleHeapLockedResidentForTesting());