diff --git a/src/common/Math.cpp b/src/common/Math.cpp index 8e5985f7e7..80b8438d65 100644 --- a/src/common/Math.cpp +++ b/src/common/Math.cpp @@ -98,14 +98,6 @@ bool IsAligned(uint32_t value, size_t alignment) { return (value & (alignment32 - 1)) == 0; } -uint32_t Align(uint32_t value, size_t alignment) { - ASSERT(alignment <= UINT32_MAX); - ASSERT(IsPowerOfTwo(alignment)); - ASSERT(alignment != 0); - uint32_t alignment32 = static_cast(alignment); - return (value + (alignment32 - 1)) & ~(alignment32 - 1); -} - uint16_t Float32ToFloat16(float fp32) { uint32_t fp32i = BitCast(fp32); uint32_t sign16 = (fp32i & 0x80000000) >> 16; diff --git a/src/common/Math.h b/src/common/Math.h index c673785e26..d29a88c5fa 100644 --- a/src/common/Math.h +++ b/src/common/Math.h @@ -51,7 +51,15 @@ uint64_t NextPowerOfTwo(uint64_t n); bool IsPtrAligned(const void* ptr, size_t alignment); void* AlignVoidPtr(void* ptr, size_t alignment); bool IsAligned(uint32_t value, size_t alignment); -uint32_t Align(uint32_t value, size_t alignment); + +template +T Align(T value, size_t alignment) { + ASSERT(value <= std::numeric_limits::max() - (alignment - 1)); + ASSERT(IsPowerOfTwo(alignment)); + ASSERT(alignment != 0); + T alignmentT = static_cast(alignment); + return (value + (alignmentT - 1)) & ~(alignmentT - 1); +} template DAWN_FORCE_INLINE T* AlignPtr(T* ptr, size_t alignment) { diff --git a/src/dawn_native/d3d12/BufferD3D12.cpp b/src/dawn_native/d3d12/BufferD3D12.cpp index da4be7a465..0d8bb6a3a1 100644 --- a/src/dawn_native/d3d12/BufferD3D12.cpp +++ b/src/dawn_native/d3d12/BufferD3D12.cpp @@ -83,6 +83,15 @@ namespace dawn_native { namespace d3d12 { return D3D12_HEAP_TYPE_DEFAULT; } } + + size_t D3D12BufferSizeAlignment(wgpu::BufferUsage usage) { + switch (usage) { + case wgpu::BufferUsage::Uniform: + return D3D12_CONSTANT_BUFFER_DATA_PLACEMENT_ALIGNMENT; + default: + return 1; + } + } } // namespace Buffer::Buffer(Device* device, const BufferDescriptor* descriptor) @@ -95,7 +104,14 @@ namespace dawn_native { namespace d3d12 { resourceDescriptor.Alignment = 0; // TODO(cwallez@chromium.org): Have a global "zero" buffer that can do everything instead // of creating a new 4-byte buffer? - resourceDescriptor.Width = std::max(GetSize(), uint64_t(4u)); + // D3D buffers are always resource size aligned to 64KB. However, D3D12's validation forbids + // binding a CBV to an unaligned size. To prevent, one can always safely align the buffer + // desc size to the CBV data alignment as other buffer usages ignore it (no size check). + // The validation will still enforce bound checks with the unaligned size returned by + // GetSize(). + // https://docs.microsoft.com/en-us/windows/win32/direct3d12/uploading-resources#buffer-alignment + resourceDescriptor.Width = + Align(std::max(GetSize(), uint64_t(4u)), D3D12BufferSizeAlignment(GetUsage())); resourceDescriptor.Height = 1; resourceDescriptor.DepthOrArraySize = 1; resourceDescriptor.MipLevels = 1; diff --git a/src/tests/BUILD.gn b/src/tests/BUILD.gn index b701984065..0077695768 100644 --- a/src/tests/BUILD.gn +++ b/src/tests/BUILD.gn @@ -396,7 +396,7 @@ source_set("dawn_white_box_tests_sources") { sources += [ "white_box/D3D12DescriptorHeapTests.cpp", "white_box/D3D12ResidencyTests.cpp", - "white_box/D3D12SmallTextureTests.cpp", + "white_box/D3D12ResourceHeapTests.cpp", ] } diff --git a/src/tests/unittests/MathTests.cpp b/src/tests/unittests/MathTests.cpp index 2294fe1aa0..71136dc425 100644 --- a/src/tests/unittests/MathTests.cpp +++ b/src/tests/unittests/MathTests.cpp @@ -136,17 +136,17 @@ TEST(Math, AlignPtr) { // Tests for Align TEST(Math, Align) { // 0 aligns to 0 - ASSERT_EQ(Align(0, 4), 0u); - ASSERT_EQ(Align(0, 256), 0u); - ASSERT_EQ(Align(0, 512), 0u); + ASSERT_EQ(Align(0u, 4), 0u); + ASSERT_EQ(Align(0u, 256), 0u); + ASSERT_EQ(Align(0u, 512), 0u); // Multiples align to self - ASSERT_EQ(Align(8, 8), 8u); - ASSERT_EQ(Align(16, 8), 16u); - ASSERT_EQ(Align(24, 8), 24u); - ASSERT_EQ(Align(256, 256), 256u); - ASSERT_EQ(Align(512, 256), 512u); - ASSERT_EQ(Align(768, 256), 768u); + ASSERT_EQ(Align(8u, 8), 8u); + ASSERT_EQ(Align(16u, 8), 16u); + ASSERT_EQ(Align(24u, 8), 24u); + ASSERT_EQ(Align(256u, 256), 256u); + ASSERT_EQ(Align(512u, 256), 512u); + ASSERT_EQ(Align(768u, 256), 768u); // Alignment with 1 is self for (uint32_t i = 0; i < 128; ++i) { @@ -157,6 +157,10 @@ TEST(Math, Align) { for (uint32_t i = 1; i <= 64; ++i) { ASSERT_EQ(Align(64 + i, 64), 128u); } + + // Test extrema + ASSERT_EQ(Align(static_cast(0xFFFFFFFF), 4), 0x100000000u); + ASSERT_EQ(Align(static_cast(0xFFFFFFFFFFFFFFFF), 1), 0xFFFFFFFFFFFFFFFFull); } // Tests for IsPtrAligned diff --git a/src/tests/white_box/D3D12SmallTextureTests.cpp b/src/tests/white_box/D3D12ResourceHeapTests.cpp similarity index 66% rename from src/tests/white_box/D3D12SmallTextureTests.cpp rename to src/tests/white_box/D3D12ResourceHeapTests.cpp index fdf5d81b12..450cb9e08d 100644 --- a/src/tests/white_box/D3D12SmallTextureTests.cpp +++ b/src/tests/white_box/D3D12ResourceHeapTests.cpp @@ -14,12 +14,18 @@ #include "tests/DawnTest.h" +#include "dawn_native/d3d12/BufferD3D12.h" #include "dawn_native/d3d12/TextureD3D12.h" using namespace dawn_native::d3d12; -class D3D12SmallTextureTests : public DawnTest { +class D3D12ResourceHeapTests : public DawnTest { protected: + void SetUp() override { + DawnTest::SetUp(); + DAWN_SKIP_TEST_IF(UsesWire()); + } + std::vector GetRequiredExtensions() override { mIsBCFormatSupported = SupportsExtensions({"texture_compression_bc"}); if (!mIsBCFormatSupported) { @@ -38,8 +44,7 @@ class D3D12SmallTextureTests : public DawnTest { }; // Verify that creating a small compressed textures will be 4KB aligned. -TEST_P(D3D12SmallTextureTests, AlignSmallCompressedTexture) { - DAWN_SKIP_TEST_IF(UsesWire()); +TEST_P(D3D12ResourceHeapTests, AlignSmallCompressedTexture) { DAWN_SKIP_TEST_IF(!IsBCFormatSupported()); // TODO(http://crbug.com/dawn/282): Investigate GPU/driver rejections of small alignment. @@ -73,4 +78,28 @@ TEST_P(D3D12SmallTextureTests, AlignSmallCompressedTexture) { static_cast(D3D12_DEFAULT_RESOURCE_PLACEMENT_ALIGNMENT)); } -DAWN_INSTANTIATE_TEST(D3D12SmallTextureTests, D3D12Backend()); +// Verify creating a UBO will always be 256B aligned. +TEST_P(D3D12ResourceHeapTests, AlignUBO) { + // Create a small UBO + wgpu::BufferDescriptor descriptor; + descriptor.size = 4 * 1024; + descriptor.usage = wgpu::BufferUsage::Uniform; + + wgpu::Buffer buffer = device.CreateBuffer(&descriptor); + Buffer* d3dBuffer = reinterpret_cast(buffer.Get()); + + EXPECT_TRUE((d3dBuffer->GetD3D12Resource()->GetDesc().Width % + static_cast(D3D12_CONSTANT_BUFFER_DATA_PLACEMENT_ALIGNMENT)) == 0u); + + // Create a larger UBO + descriptor.size = (4 * 1024 * 1024) + 255; + descriptor.usage = wgpu::BufferUsage::Uniform; + + buffer = device.CreateBuffer(&descriptor); + d3dBuffer = reinterpret_cast(buffer.Get()); + + EXPECT_TRUE((d3dBuffer->GetD3D12Resource()->GetDesc().Width % + static_cast(D3D12_CONSTANT_BUFFER_DATA_PLACEMENT_ALIGNMENT)) == 0u); +} + +DAWN_INSTANTIATE_TEST(D3D12ResourceHeapTests, D3D12Backend());