From d1a5f93630a0bee14fbb2eb537ed61a80c100d01 Mon Sep 17 00:00:00 2001 From: shrekshao Date: Thu, 29 Sep 2022 19:02:11 +0000 Subject: [PATCH] Add maxBufferSize limit Also emits a warning when create buffer size exceeds the maxBufferSize limit during the deprecation period. Bug: dawn:1525 Change-Id: I7b47ae5c85b116035fdcea8b68fb574c0a550729 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/103660 Kokoro: Kokoro Reviewed-by: Corentin Wallez Auto-Submit: Shrek Shao Commit-Queue: Shrek Shao --- dawn.json | 1 + docs/support.md | 2 +- src/dawn/common/Constants.h | 2 + src/dawn/native/Adapter.cpp | 3 +- src/dawn/native/Device.cpp | 8 +++ src/dawn/native/Limits.cpp | 63 ++++++++++--------- src/dawn/native/d3d12/AdapterD3D12.cpp | 2 + src/dawn/native/metal/BackendMTL.mm | 1 + src/dawn/native/vulkan/AdapterVk.cpp | 10 +++ src/dawn/native/vulkan/VulkanExtensions.cpp | 4 ++ src/dawn/native/vulkan/VulkanExtensions.h | 2 + src/dawn/native/vulkan/VulkanInfo.cpp | 5 +- src/dawn/native/vulkan/VulkanInfo.h | 1 + src/dawn/tests/end2end/BufferTests.cpp | 15 +++-- src/dawn/tests/end2end/DeprecatedAPITests.cpp | 16 +++++ .../validation/BufferValidationTests.cpp | 3 +- 16 files changed, 101 insertions(+), 37 deletions(-) diff --git a/dawn.json b/dawn.json index ce90abf67b..1080426177 100644 --- a/dawn.json +++ b/dawn.json @@ -1263,6 +1263,7 @@ {"name": "min uniform buffer offset alignment", "type": "uint32_t", "default": "WGPU_LIMIT_U32_UNDEFINED"}, {"name": "min storage buffer offset alignment", "type": "uint32_t", "default": "WGPU_LIMIT_U32_UNDEFINED"}, {"name": "max vertex buffers", "type": "uint32_t", "default": "WGPU_LIMIT_U32_UNDEFINED"}, + {"name": "max buffer size", "type": "uint64_t", "default": "WGPU_LIMIT_U64_UNDEFINED"}, {"name": "max vertex attributes", "type": "uint32_t", "default": "WGPU_LIMIT_U32_UNDEFINED"}, {"name": "max vertex buffer array stride", "type": "uint32_t", "default": "WGPU_LIMIT_U32_UNDEFINED"}, {"name": "max inter stage shader components", "type": "uint32_t", "default": "WGPU_LIMIT_U32_UNDEFINED"}, diff --git a/docs/support.md b/docs/support.md index c4e38bff53..690a5fed10 100644 --- a/docs/support.md +++ b/docs/support.md @@ -10,7 +10,7 @@ Vulkan is supported as best effort on other platforms (e.g. Windows and macOS). **Required features**: `depthBiasClamp`, `fragmentStoresAndAtomics`, `fullDrawIndexUint32`, `imageCubeArray`, `independentBlend`, `sampleRateShading`, and either `textureCompressionBC` or both of `textureCompressionETC` and `textureCompressionASTC_LDR`. -**Required limites**: they are too detailed to describe here, but in general should be wildly supported. +**Required limits**: they are too detailed to describe here, but in general should be wildly supported. See the [WebGPU limits](https://gpuweb.github.io/gpuweb/#limits) that mostly correspond to Vulkan limits. **Operating system support**: diff --git a/src/dawn/common/Constants.h b/src/dawn/common/Constants.h index 57ef80fc54..bf1f132b6c 100644 --- a/src/dawn/common/Constants.h +++ b/src/dawn/common/Constants.h @@ -27,6 +27,8 @@ static constexpr uint8_t kMaxColorAttachments = 8u; static constexpr uint32_t kTextureBytesPerRowAlignment = 256u; static constexpr uint32_t kMaxInterStageShaderComponents = 60u; static constexpr uint32_t kMaxInterStageShaderVariables = 16u; +static constexpr uint64_t kAssumedMaxBufferSize = + 0x80000000u; // Use 2 GB when the limit is unavailable // Per stage limits static constexpr uint32_t kMaxSampledTexturesPerShaderStage = 16; diff --git a/src/dawn/native/Adapter.cpp b/src/dawn/native/Adapter.cpp index c3063866d8..2c3fafbeec 100644 --- a/src/dawn/native/Adapter.cpp +++ b/src/dawn/native/Adapter.cpp @@ -49,7 +49,8 @@ MaybeError AdapterBase::Initialize() { "backend=%s type=%s)", mName, mDriverDescription, mVendorId, mDeviceId, mBackend, mAdapterType); - // Enforce internal Dawn constants. + // Enforce internal Dawn constants for some limits to ensure they don't go over fixed-size + // arrays in Dawn's internal code. mLimits.v1.maxVertexBufferArrayStride = std::min(mLimits.v1.maxVertexBufferArrayStride, kMaxVertexBufferArrayStride); mLimits.v1.maxColorAttachments = diff --git a/src/dawn/native/Device.cpp b/src/dawn/native/Device.cpp index 94cd63abe1..56ca7494f2 100644 --- a/src/dawn/native/Device.cpp +++ b/src/dawn/native/Device.cpp @@ -1432,6 +1432,14 @@ ResultOrError> DeviceBase::CreateBuffer(const BufferDescriptor* DAWN_TRY(ValidateIsAlive()); if (IsValidationEnabled()) { DAWN_TRY_CONTEXT(ValidateBufferDescriptor(this, descriptor), "validating %s", descriptor); + + // TODO(dawn:1525): Change to validation error after the deprecation period. + if (descriptor->size > mLimits.v1.maxBufferSize) { + std::string warning = + absl::StrFormat("Buffer size (%u) exceeds the max buffer size limit (%u).", + descriptor->size, mLimits.v1.maxBufferSize); + EmitDeprecationWarning(warning.c_str()); + } } Ref buffer; diff --git a/src/dawn/native/Limits.cpp b/src/dawn/native/Limits.cpp index 63d7732d2e..dfa06509e8 100644 --- a/src/dawn/native/Limits.cpp +++ b/src/dawn/native/Limits.cpp @@ -28,47 +28,54 @@ #define LIMITS_STORAGE_BUFFER_BINDING_SIZE(X) \ X(Maximum, maxStorageBufferBindingSize, 134217728, 1073741824, 2147483647, 4294967295) +// Tiers are 256Mb, 1Gb, 2Gb. +#define LIMITS_MAX_BUFFER_SIZE(X) \ + X(Maximum, maxBufferSize, 0x10000000, 0x40000000, 0x80000000) + // TODO(crbug.com/dawn/685): // These limits don't have tiers yet. Define two tiers with the same values since the macros // in this file expect more than one tier. -#define LIMITS_OTHER(X) \ - X(Maximum, maxTextureDimension1D, 8192, 8192) \ - X(Maximum, maxTextureDimension2D, 8192, 8192) \ - X(Maximum, maxTextureDimension3D, 2048, 2048) \ - X(Maximum, maxTextureArrayLayers, 256, 256) \ - X(Maximum, maxBindGroups, 4, 4) \ - X(Maximum, maxBindingsPerBindGroup, 640, 640) \ - X(Maximum, maxDynamicUniformBuffersPerPipelineLayout, 8, 8) \ - X(Maximum, maxDynamicStorageBuffersPerPipelineLayout, 4, 4) \ - X(Maximum, maxSampledTexturesPerShaderStage, 16, 16) \ - X(Maximum, maxSamplersPerShaderStage, 16, 16) \ - X(Maximum, maxStorageBuffersPerShaderStage, 8, 8) \ - X(Maximum, maxStorageTexturesPerShaderStage, 4, 4) \ - X(Maximum, maxUniformBuffersPerShaderStage, 12, 12) \ - X(Maximum, maxUniformBufferBindingSize, 65536, 65536) \ - X(Alignment, minUniformBufferOffsetAlignment, 256, 256) \ - X(Alignment, minStorageBufferOffsetAlignment, 256, 256) \ - X(Maximum, maxVertexBuffers, 8, 8) \ - X(Maximum, maxVertexAttributes, 16, 16) \ - X(Maximum, maxVertexBufferArrayStride, 2048, 2048) \ - X(Maximum, maxInterStageShaderComponents, 60, 60) \ - X(Maximum, maxInterStageShaderVariables, 16, 16) \ - X(Maximum, maxColorAttachments, 8, 8) \ - X(Maximum, maxComputeInvocationsPerWorkgroup, 256, 256) \ - X(Maximum, maxComputeWorkgroupSizeX, 256, 256) \ - X(Maximum, maxComputeWorkgroupSizeY, 256, 256) \ - X(Maximum, maxComputeWorkgroupSizeZ, 64, 64) \ - X(Maximum, maxComputeWorkgroupsPerDimension, 65535, 65535) +#define LIMITS_OTHER(X) \ + X(Maximum, maxTextureDimension1D, 8192, 8192) \ + X(Maximum, maxTextureDimension2D, 8192, 8192) \ + X(Maximum, maxTextureDimension3D, 2048, 2048) \ + X(Maximum, maxTextureArrayLayers, 256, 256) \ + X(Maximum, maxBindGroups, 4, 4) \ + X(Maximum, maxBindingsPerBindGroup, 640, 640) \ + X(Maximum, maxDynamicUniformBuffersPerPipelineLayout, 8, 8) \ + X(Maximum, maxDynamicStorageBuffersPerPipelineLayout, 4, 4) \ + X(Maximum, maxSampledTexturesPerShaderStage, 16, 16) \ + X(Maximum, maxSamplersPerShaderStage, 16, 16) \ + X(Maximum, maxStorageBuffersPerShaderStage, 8, 8) \ + X(Maximum, maxStorageTexturesPerShaderStage, 4, 4) \ + X(Maximum, maxUniformBuffersPerShaderStage, 12, 12) \ + X(Maximum, maxUniformBufferBindingSize, 65536, 65536) \ + X(Alignment, minUniformBufferOffsetAlignment, 256, 256) \ + X(Alignment, minStorageBufferOffsetAlignment, 256, 256) \ + X(Maximum, maxVertexBuffers, 8, 8) \ + X(Maximum, maxBufferSize, 268435456, 268435456) \ + X(Maximum, maxVertexAttributes, 16, 16) \ + X(Maximum, maxVertexBufferArrayStride, 2048, 2048) \ + X(Maximum, maxInterStageShaderComponents, 60, 60) \ + X(Maximum, maxInterStageShaderVariables, 16, 16) \ + X(Maximum, maxColorAttachments, 8, 8) \ + X(Maximum, maxComputeInvocationsPerWorkgroup, 256, 256) \ + X(Maximum, maxComputeWorkgroupSizeX, 256, 256) \ + X(Maximum, maxComputeWorkgroupSizeY, 256, 256) \ + X(Maximum, maxComputeWorkgroupSizeZ, 64, 64) \ + X(Maximum, maxComputeWorkgroupsPerDimension, 65535, 65535) // clang-format on #define LIMITS_EACH_GROUP(X) \ X(LIMITS_WORKGROUP_STORAGE_SIZE) \ X(LIMITS_STORAGE_BUFFER_BINDING_SIZE) \ + X(LIMITS_MAX_BUFFER_SIZE) \ X(LIMITS_OTHER) #define LIMITS(X) \ LIMITS_WORKGROUP_STORAGE_SIZE(X) \ LIMITS_STORAGE_BUFFER_BINDING_SIZE(X) \ + LIMITS_MAX_BUFFER_SIZE(X) \ LIMITS_OTHER(X) namespace dawn::native { diff --git a/src/dawn/native/d3d12/AdapterD3D12.cpp b/src/dawn/native/d3d12/AdapterD3D12.cpp index 842e49e537..d186e85875 100644 --- a/src/dawn/native/d3d12/AdapterD3D12.cpp +++ b/src/dawn/native/d3d12/AdapterD3D12.cpp @@ -304,6 +304,8 @@ MaybeError Adapter::InitializeSupportedLimitsImpl(CombinedLimits* limits) { limits->v1.maxUniformBufferBindingSize = D3D12_REQ_CONSTANT_BUFFER_ELEMENT_COUNT * 16; // D3D12 has no documented limit on the size of a storage buffer binding. limits->v1.maxStorageBufferBindingSize = 4294967295; + // D3D12 has no documented limit on the buffer size. + limits->v1.maxBufferSize = kAssumedMaxBufferSize; // Using base limits for: // TODO(crbug.com/dawn/685): diff --git a/src/dawn/native/metal/BackendMTL.mm b/src/dawn/native/metal/BackendMTL.mm index 76dbce815f..c2e75af928 100644 --- a/src/dawn/native/metal/BackendMTL.mm +++ b/src/dawn/native/metal/BackendMTL.mm @@ -598,6 +598,7 @@ class Adapter : public AdapterBase { limits->v1.minStorageBufferOffsetAlignment = mtlLimits.minBufferOffsetAlignment; uint64_t maxBufferSize = Buffer::QueryMaxBufferLength(*mDevice); + limits->v1.maxBufferSize = maxBufferSize; // Metal has no documented limit on the size of a binding. Use the maximum // buffer size. diff --git a/src/dawn/native/vulkan/AdapterVk.cpp b/src/dawn/native/vulkan/AdapterVk.cpp index bd4d5df31d..35cd36783f 100644 --- a/src/dawn/native/vulkan/AdapterVk.cpp +++ b/src/dawn/native/vulkan/AdapterVk.cpp @@ -309,6 +309,16 @@ MaybeError Adapter::InitializeSupportedLimitsImpl(CombinedLimits* limits) { return DAWN_INTERNAL_ERROR("Insufficient Vulkan limits for framebufferDepthSampleCounts"); } + if (mDeviceInfo.HasExt(DeviceExt::Maintenance3)) { + limits->v1.maxBufferSize = mDeviceInfo.propertiesMaintenance3.maxMemoryAllocationSize; + if (mDeviceInfo.propertiesMaintenance3.maxMemoryAllocationSize < + baseLimits.v1.maxBufferSize) { + return DAWN_INTERNAL_ERROR("Insufficient Vulkan maxBufferSize limit"); + } + } else { + limits->v1.maxBufferSize = kAssumedMaxBufferSize; + } + // Only check maxFragmentCombinedOutputResources on mobile GPUs. Desktop GPUs drivers seem // to put incorrect values for this limit with things like 8 or 16 when they can do bindless // storage buffers. Mesa llvmpipe driver also puts 8 here. diff --git a/src/dawn/native/vulkan/VulkanExtensions.cpp b/src/dawn/native/vulkan/VulkanExtensions.cpp index 7e61d121b8..f713f5e108 100644 --- a/src/dawn/native/vulkan/VulkanExtensions.cpp +++ b/src/dawn/native/vulkan/VulkanExtensions.cpp @@ -134,6 +134,8 @@ static constexpr std::array sDeviceExtInfos{{ // {DeviceExt::BindMemory2, "VK_KHR_bind_memory2", VulkanVersion_1_1}, {DeviceExt::Maintenance1, "VK_KHR_maintenance1", VulkanVersion_1_1}, + {DeviceExt::Maintenance2, "VK_KHR_maintenance2", VulkanVersion_1_1}, + {DeviceExt::Maintenance3, "VK_KHR_maintenance3", VulkanVersion_1_1}, {DeviceExt::StorageBufferStorageClass, "VK_KHR_storage_buffer_storage_class", VulkanVersion_1_1}, {DeviceExt::GetPhysicalDeviceProperties2, "VK_KHR_get_physical_device_properties2", @@ -210,6 +212,7 @@ DeviceExtSet EnsureDependencies(const DeviceExtSet& advertisedExts, case DeviceExt::BindMemory2: case DeviceExt::GetMemoryRequirements2: case DeviceExt::Maintenance1: + case DeviceExt::Maintenance2: case DeviceExt::ImageFormatList: case DeviceExt::StorageBufferStorageClass: hasDependencies = true; @@ -224,6 +227,7 @@ DeviceExtSet EnsureDependencies(const DeviceExtSet& advertisedExts, // advertises the extension. So if we didn't have this check, we'd risk a calling // a nullptr. case DeviceExt::GetPhysicalDeviceProperties2: + case DeviceExt::Maintenance3: hasDependencies = instanceExts[InstanceExt::GetPhysicalDeviceProperties2]; break; case DeviceExt::ExternalMemoryCapabilities: diff --git a/src/dawn/native/vulkan/VulkanExtensions.h b/src/dawn/native/vulkan/VulkanExtensions.h index 77cade8215..9141ab930e 100644 --- a/src/dawn/native/vulkan/VulkanExtensions.h +++ b/src/dawn/native/vulkan/VulkanExtensions.h @@ -76,6 +76,8 @@ enum class DeviceExt { // Promoted to 1.1 BindMemory2, Maintenance1, + Maintenance2, + Maintenance3, StorageBufferStorageClass, GetPhysicalDeviceProperties2, GetMemoryRequirements2, diff --git a/src/dawn/native/vulkan/VulkanInfo.cpp b/src/dawn/native/vulkan/VulkanInfo.cpp index d033e291ca..97b3b73224 100644 --- a/src/dawn/native/vulkan/VulkanInfo.cpp +++ b/src/dawn/native/vulkan/VulkanInfo.cpp @@ -223,9 +223,12 @@ ResultOrError GatherDeviceInfo(const Adapter& adapter) { VkPhysicalDeviceProperties2 properties2 = {}; properties2.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_PROPERTIES_2; - features2.pNext = nullptr; + PNextChainBuilder propertiesChain(&properties2); + propertiesChain.Add(&info.propertiesMaintenance3, + VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_MAINTENANCE_3_PROPERTIES); + if (info.extensions[DeviceExt::ShaderFloat16Int8]) { featuresChain.Add(&info.shaderFloat16Int8Features, VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SHADER_FLOAT16_INT8_FEATURES_KHR); diff --git a/src/dawn/native/vulkan/VulkanInfo.h b/src/dawn/native/vulkan/VulkanInfo.h index 5bb925736c..0de660de05 100644 --- a/src/dawn/native/vulkan/VulkanInfo.h +++ b/src/dawn/native/vulkan/VulkanInfo.h @@ -61,6 +61,7 @@ struct VulkanDeviceKnobs { struct VulkanDeviceInfo : VulkanDeviceKnobs { VkPhysicalDeviceProperties properties; + VkPhysicalDeviceMaintenance3Properties propertiesMaintenance3; VkPhysicalDeviceDriverProperties driverProperties; VkPhysicalDeviceSubgroupSizeControlPropertiesEXT subgroupSizeControlProperties; VkPhysicalDeviceShaderIntegerDotProductPropertiesKHR shaderIntegerDotProductProperties; diff --git a/src/dawn/tests/end2end/BufferTests.cpp b/src/dawn/tests/end2end/BufferTests.cpp index 443fe64c61..3ba92610ce 100644 --- a/src/dawn/tests/end2end/BufferTests.cpp +++ b/src/dawn/tests/end2end/BufferTests.cpp @@ -812,11 +812,13 @@ TEST_P(BufferTests, CreateBufferOOM) { descriptor.usage = wgpu::BufferUsage::CopyDst; descriptor.size = std::numeric_limits::max(); - ASSERT_DEVICE_ERROR(device.CreateBuffer(&descriptor)); + // TODO(dawn:1525): remove warning expectation after the deprecation period. + ASSERT_DEVICE_ERROR(EXPECT_DEPRECATION_WARNING(device.CreateBuffer(&descriptor))); // UINT64_MAX may be special cased. Test a smaller, but really large buffer also fails descriptor.size = 1ull << 50; - ASSERT_DEVICE_ERROR(device.CreateBuffer(&descriptor)); + // TODO(dawn:1525): remove warning expectation after the deprecation period. + ASSERT_DEVICE_ERROR(EXPECT_DEPRECATION_WARNING(device.CreateBuffer(&descriptor))); } // Test that a very large buffer mappedAtCreation fails gracefully. @@ -846,7 +848,8 @@ TEST_P(BufferTests, BufferMappedAtCreationOOM) { // UINT64_MAX may be special cased. Test a smaller, but really large buffer also fails descriptor.size = 1ull << 50; - ASSERT_DEVICE_ERROR(device.CreateBuffer(&descriptor)); + // TODO(dawn:1525): remove warning expectation after the deprecation period. + ASSERT_DEVICE_ERROR(EXPECT_DEPRECATION_WARNING(device.CreateBuffer(&descriptor))); } // Test mappable buffer @@ -865,7 +868,8 @@ TEST_P(BufferTests, BufferMappedAtCreationOOM) { // UINT64_MAX may be special cased. Test a smaller, but really large buffer also fails descriptor.size = 1ull << 50; - ASSERT_DEVICE_ERROR(device.CreateBuffer(&descriptor)); + // TODO(dawn:1525): remove warning expectation after the deprecation period. + ASSERT_DEVICE_ERROR(EXPECT_DEPRECATION_WARNING(device.CreateBuffer(&descriptor))); } } @@ -878,7 +882,8 @@ TEST_P(BufferTests, CreateBufferOOMMapAsync) { auto RunTest = [this](const wgpu::BufferDescriptor& descriptor) { wgpu::Buffer buffer; - ASSERT_DEVICE_ERROR(buffer = device.CreateBuffer(&descriptor)); + // TODO(dawn:1525): remove warning expectation after the deprecation period. + ASSERT_DEVICE_ERROR(EXPECT_DEPRECATION_WARNING(buffer = device.CreateBuffer(&descriptor))); bool done = false; ASSERT_DEVICE_ERROR(buffer.MapAsync( diff --git a/src/dawn/tests/end2end/DeprecatedAPITests.cpp b/src/dawn/tests/end2end/DeprecatedAPITests.cpp index 28960b4e5f..3a00744de6 100644 --- a/src/dawn/tests/end2end/DeprecatedAPITests.cpp +++ b/src/dawn/tests/end2end/DeprecatedAPITests.cpp @@ -168,6 +168,22 @@ TEST_P(DeprecationTests, Dispatch) { pass.End(); } +// Test that creating a buffer with size exceeding the maximum buffer size limit should emits a +// warning. (dawn:1525) +TEST_P(DeprecationTests, MaxBufferSizeValidation) { + wgpu::BufferDescriptor descriptor; + descriptor.usage = wgpu::BufferUsage::CopySrc | wgpu::BufferUsage::CopyDst; + + descriptor.size = 256; + device.CreateBuffer(&descriptor); + + descriptor.size = GetSupportedLimits().limits.maxBufferSize; + device.CreateBuffer(&descriptor); + + descriptor.size = GetSupportedLimits().limits.maxBufferSize + 1; + EXPECT_DEPRECATION_WARNING(device.CreateBuffer(&descriptor)); +} + DAWN_INSTANTIATE_TEST(DeprecationTests, D3D12Backend(), MetalBackend(), diff --git a/src/dawn/tests/unittests/validation/BufferValidationTests.cpp b/src/dawn/tests/unittests/validation/BufferValidationTests.cpp index af95b82816..7c5fcbb034 100644 --- a/src/dawn/tests/unittests/validation/BufferValidationTests.cpp +++ b/src/dawn/tests/unittests/validation/BufferValidationTests.cpp @@ -992,8 +992,9 @@ TEST_F(BufferValidationTest, CreationParameterReflectionForOOMBuffer) { desc.size = kAmazinglyLargeSize; // OOM! + // TODO(dawn:1525): remove warning expectation after the deprecation period. wgpu::Buffer buf; - ASSERT_DEVICE_ERROR(buf = device.CreateBuffer(&desc)); + ASSERT_DEVICE_ERROR(EXPECT_DEPRECATION_WARNING(buf = device.CreateBuffer(&desc))); // Reflection data is still exactly what was in the descriptor. EXPECT_EQ(wgpu::BufferUsage::Storage, buf.GetUsage());