D3D12: Align UBO sizes to 256B.

D3D debug layer uses the descriptor size (width) to
validate CBV bounds when directly allocating UBOs.
This causes validation failure when the buffer size
is misaligned (ie. not a multiple of 256B) even
through the underlying resource heap size is always
64KB aligned.

This change always aligns the buffer size to be 256B
to avoid such validation error should sub-allocation
fail.

BUG=dawn:506

Change-Id: Ic9072934cac65cfd25d0e2a20cb364bd3ca88e3b
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/26820
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Bryan Bernhart <bryan.bernhart@intel.com>

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<uint32_t>(alignment);
-    return (value + (alignment32 - 1)) & ~(alignment32 - 1);
-}
-
 uint16_t Float32ToFloat16(float fp32) {
     uint32_t fp32i = BitCast<uint32_t>(fp32);
     uint32_t sign16 = (fp32i & 0x80000000) >> 16;

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 <typename T>
+T Align(T value, size_t alignment) {
+    ASSERT(value <= std::numeric_limits<T>::max() - (alignment - 1));
+    ASSERT(IsPowerOfTwo(alignment));
+    ASSERT(alignment != 0);
+    T alignmentT = static_cast<T>(alignment);
+    return (value + (alignmentT - 1)) & ~(alignmentT - 1);
+}
 
 template <typename T>
 DAWN_FORCE_INLINE T* AlignPtr(T* ptr, size_t alignment) {

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;

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",
     ]
   }
 

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<uint64_t>(0xFFFFFFFF), 4), 0x100000000u);
+    ASSERT_EQ(Align(static_cast<uint64_t>(0xFFFFFFFFFFFFFFFF), 1), 0xFFFFFFFFFFFFFFFFull);
 }
 
 // Tests for IsPtrAligned

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<const char*> 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<uint64_t>(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*>(buffer.Get());
+
+    EXPECT_TRUE((d3dBuffer->GetD3D12Resource()->GetDesc().Width %
+                 static_cast<uint64_t>(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*>(buffer.Get());
+
+    EXPECT_TRUE((d3dBuffer->GetD3D12Resource()->GetDesc().Width %
+                 static_cast<uint64_t>(D3D12_CONSTANT_BUFFER_DATA_PLACEMENT_ALIGNMENT)) == 0u);
+}
+
+DAWN_INSTANTIATE_TEST(D3D12ResourceHeapTests, D3D12Backend());