From 15f11d63b447ca51fc5fcbc81c982354470b708e Mon Sep 17 00:00:00 2001 From: Gregg Tavares Date: Fri, 27 Dec 2024 13:22:00 -0800 Subject: [PATCH] Compat: createBGL, createPipelineLayout, etc... Refactor createBindGroupLayout, createPipelineLayout, vertex_state,correctness, and requestDevice tests for 0 storage buffers/textures in vertex/fragment stage. Note: I needed to add `maxStorage(Buffers/Textures)In(Fragment/Vertex)Stage` to `capability_info.ts`. That had cascading effects which is why so many files are changed. For one, since these new limits are marked as optional, any place that uses limits has to check it's not undefined or least add a `!` to tell TS to stop complaining. I'm kind of wondering if we should change them to be required. They'll be required in the spec eventually, or at least that's the plan. In any case, this adds them. Another issue was the existing structure of `kPerStageBindingLimits` in `capability_info.ts`. I refactored that to have per stage limits and added `getBindingLimitsForBindingType` where you pass in the type you're using and the visibility and returns the limit for those stages. I also moved `getDefaultLimit/s` from GPUTestBase to LimitTestImpl as it seems like only the limits test should care about the default. All other tests should use what's on the device. --- .../operation/adapter/requestDevice.spec.ts | 26 ++++-- .../vertex_state/correctness.spec.ts | 18 +---- .../capability_checks/limits/limit_utils.ts | 14 +++- .../maxComputeInvocationsPerWorkgroup.spec.ts | 7 +- .../validation/createBindGroupLayout.spec.ts | 79 ++++++++++++++++--- .../validation/createPipelineLayout.spec.ts | 20 +++-- src/webgpu/capability_info.ts | 56 +++++++++++-- src/webgpu/gpu_test.ts | 21 +---- 8 files changed, 168 insertions(+), 73 deletions(-) diff --git a/src/webgpu/api/operation/adapter/requestDevice.spec.ts b/src/webgpu/api/operation/adapter/requestDevice.spec.ts index 9aa5ca22e20c..42701660386c 100644 --- a/src/webgpu/api/operation/adapter/requestDevice.spec.ts +++ b/src/webgpu/api/operation/adapter/requestDevice.spec.ts @@ -274,7 +274,7 @@ g.test('limits,supported') result = value; break; case 'adapter': - value = adapter.limits[limit]; + value = adapter.limits[limit]!; result = value; break; case 'undefined': @@ -283,11 +283,27 @@ g.test('limits,supported') break; } - const device = await t.requestDeviceTracked(adapter, { requiredLimits: { [limit]: value } }); + const requiredLimits: Record = { [limit]: value }; + + if ( + limit === 'maxStorageBuffersInFragmentStage' || + limit === 'maxStorageBuffersInVertexStage' + ) { + requiredLimits['maxStorageBuffersPerShaderStage'] = value; + } + + if ( + limit === 'maxStorageTexturesInFragmentStage' || + limit === 'maxStorageTexturesInVertexStage' + ) { + requiredLimits['maxStorageTexturesPerShaderStage'] = value; + } + + const device = await t.requestDeviceTracked(adapter, { requiredLimits }); assert(device !== null); t.expect( device.limits[limit] === result, - 'Devices reported limit should match the required limit' + `Devices reported limit for ${limit}(${device.limits[limit]}) should match the required limit (${result})` ); }); @@ -327,7 +343,7 @@ g.test('limit,better_than_supported') assert(adapter !== null); const limitInfo = getDefaultLimitsForAdapter(adapter); - const value = adapter.limits[limit] * mul + add; + const value = adapter.limits[limit]! * mul + add; const requiredLimits = { [limit]: clamp(value, { min: 0, max: limitInfo[limit].maximumValue }), }; @@ -381,7 +397,7 @@ g.test('limit,out_of_range') const errorName = value < 0 || value > Number.MAX_SAFE_INTEGER ? 'TypeError' - : limitInfo.class === 'maximum' && value > adapter.limits[limit] + : limitInfo.class === 'maximum' && value > adapter.limits[limit]! ? 'OperationError' : limitInfo.class === 'alignment' && (value > 2 ** 31 || !isPowerOfTwo(value)) ? 'OperationError' diff --git a/src/webgpu/api/operation/vertex_state/correctness.spec.ts b/src/webgpu/api/operation/vertex_state/correctness.spec.ts index 2ef947ba0cc4..5ed1a3e00e04 100644 --- a/src/webgpu/api/operation/vertex_state/correctness.spec.ts +++ b/src/webgpu/api/operation/vertex_state/correctness.spec.ts @@ -11,11 +11,7 @@ import { memcpy, unreachable, } from '../../../../common/util/util.js'; -import { - kPerStageBindingLimits, - kVertexFormatInfo, - kVertexFormats, -} from '../../../capability_info.js'; +import { kVertexFormatInfo, kVertexFormats } from '../../../capability_info.js'; import { GPUTest, MaxLimitsTestMixin } from '../../../gpu_test.js'; import { float32ToFloat16Bits, normalizedIntegerAsFloat } from '../../../util/conversion.js'; import { align, clamp } from '../../../util/math.js'; @@ -105,18 +101,6 @@ class VertexStateTest extends GPUTest { vertexCount: number, instanceCount: number ): string { - // In the base WebGPU spec maxVertexAttributes is larger than maxUniformBufferPerStage. We'll - // use a combination of uniform and storage buffers to cover all possible attributes. This - // happens to work because maxUniformBuffer + maxStorageBuffer = 12 + 8 = 20 which is larger - // than maxVertexAttributes = 16. - // However this might not work in the future for implementations that allow even more vertex - // attributes so there will need to be larger changes when that happens. - const maxUniformBuffers = this.getDefaultLimit(kPerStageBindingLimits['uniformBuf'].maxLimit); - assert( - maxUniformBuffers + this.getDefaultLimit(kPerStageBindingLimits['storageBuf'].maxLimit) >= - this.device.limits.maxVertexAttributes - ); - let vsInputs = ''; let vsChecks = ''; let providedDataDefs = ''; diff --git a/src/webgpu/api/validation/capability_checks/limits/limit_utils.ts b/src/webgpu/api/validation/capability_checks/limits/limit_utils.ts index c2f002509e34..8ab31f04b9d7 100644 --- a/src/webgpu/api/validation/capability_checks/limits/limit_utils.ts +++ b/src/webgpu/api/validation/capability_checks/limits/limit_utils.ts @@ -2,7 +2,11 @@ import { kUnitCaseParamsBuilder } from '../../../../../common/framework/params_b import { makeTestGroup } from '../../../../../common/framework/test_group.js'; import { getGPU } from '../../../../../common/util/navigator_gpu.js'; import { assert, range, reorder, ReorderOrder } from '../../../../../common/util/util.js'; -import { getDefaultLimitsForAdapter } from '../../../../capability_info.js'; +import { + getDefaultLimits, + getDefaultLimitsForAdapter, + kLimits, +} from '../../../../capability_info.js'; import { GPUTestBase } from '../../../../gpu_test.js'; type GPUSupportedLimit = keyof GPUSupportedLimits; @@ -362,6 +366,14 @@ export class LimitTestsImpl extends GPUTestBase { return this._device; } + getDefaultLimits() { + return getDefaultLimits(this.isCompatibility ? 'compatibility' : 'core'); + } + + getDefaultLimit(limit: (typeof kLimits)[number]) { + return this.getDefaultLimits()[limit].default; + } + async requestDeviceWithLimits( adapter: GPUAdapter, requiredLimits: Record, diff --git a/src/webgpu/api/validation/capability_checks/limits/maxComputeInvocationsPerWorkgroup.spec.ts b/src/webgpu/api/validation/capability_checks/limits/maxComputeInvocationsPerWorkgroup.spec.ts index a3858a62214d..b8c0dc1e6d3c 100644 --- a/src/webgpu/api/validation/capability_checks/limits/maxComputeInvocationsPerWorkgroup.spec.ts +++ b/src/webgpu/api/validation/capability_checks/limits/maxComputeInvocationsPerWorkgroup.spec.ts @@ -1,10 +1,9 @@ -import { GPUTestBase } from '../../../../gpu_test.js'; - import { kMaximumLimitBaseParams, MaximumLimitValueTest, MaximumTestValue, makeLimitTestGroup, + LimitTestsImpl, } from './limit_utils.js'; /** @@ -77,7 +76,7 @@ function getDeviceLimitToRequest( } function getTestWorkgroupSize( - t: GPUTestBase, + t: LimitTestsImpl, testValueName: MaximumTestValue, requestedLimit: number ) { @@ -96,7 +95,7 @@ function getTestWorkgroupSize( } function getDeviceLimitToRequestAndValueToTest( - t: GPUTestBase, + t: LimitTestsImpl, limitValueTest: MaximumLimitValueTest, testValueName: MaximumTestValue, defaultLimit: number, diff --git a/src/webgpu/api/validation/createBindGroupLayout.spec.ts b/src/webgpu/api/validation/createBindGroupLayout.spec.ts index b09adc2af187..35471ff641af 100644 --- a/src/webgpu/api/validation/createBindGroupLayout.spec.ts +++ b/src/webgpu/api/validation/createBindGroupLayout.spec.ts @@ -17,8 +17,10 @@ import { bufferBindingTypeInfo, kBufferBindingTypes, BGLEntry, + getBindingLimitForBindingType, } from '../../capability_info.js'; import { kAllTextureFormats, kTextureFormatInfo } from '../../format_info.js'; +import { MaxLimitsTestMixin } from '../../gpu_test.js'; import { ValidationTest } from './validation_test.js'; @@ -26,7 +28,53 @@ function clone(descriptor: T): T { return JSON.parse(JSON.stringify(descriptor)); } -export const g = makeTestGroup(ValidationTest); +function isValidBufferTypeForStages( + device: GPUDevice, + visibility: number, + type: GPUBufferBindingType | undefined +) { + if (type === 'read-only-storage' || type === 'storage') { + if (visibility & GPUShaderStage.VERTEX) { + if (!(device.limits.maxStorageBuffersInVertexStage! > 0)) { + return false; + } + } + + if (visibility & GPUShaderStage.FRAGMENT) { + if (!(device.limits.maxStorageBuffersInFragmentStage! > 0)) { + return false; + } + } + } + + return true; +} + +function isValidStorageTextureForStages(device: GPUDevice, visibility: number) { + if (visibility & GPUShaderStage.VERTEX) { + if (!(device.limits.maxStorageTexturesInVertexStage! > 0)) { + return false; + } + } + + if (visibility & GPUShaderStage.FRAGMENT) { + if (!(device.limits.maxStorageTexturesInFragmentStage! > 0)) { + return false; + } + } + + return true; +} + +function isValidBGLEntryForStages(device: GPUDevice, visibility: number, entry: BGLEntry) { + return entry.storageTexture + ? isValidStorageTextureForStages(device, visibility) + : entry.buffer + ? isValidBufferTypeForStages(device, visibility, entry.buffer?.type) + : true; +} + +export const g = makeTestGroup(MaxLimitsTestMixin(ValidationTest)); g.test('duplicate_bindings') .desc('Test that uniqueness of binding numbers across entries is enforced.') @@ -107,7 +155,9 @@ g.test('visibility') const { visibility, entry } = t.params; const info = bindingTypeInfo(entry); - const success = (visibility & ~info.validStages) === 0; + const success = + (visibility & ~info.validStages) === 0 && + isValidBGLEntryForStages(t.device, visibility, entry); t.expectValidationError(() => { t.device.createBindGroupLayout({ @@ -132,7 +182,9 @@ g.test('visibility,VERTEX_shader_stage_buffer_type') .fn(t => { const { shaderStage, type } = t.params; - const success = !(type === 'storage' && shaderStage & GPUShaderStage.VERTEX); + const success = + !(type === 'storage' && shaderStage & GPUShaderStage.VERTEX) && + isValidBufferTypeForStages(t.device, shaderStage, type); t.expectValidationError(() => { t.device.createBindGroupLayout({ @@ -164,10 +216,11 @@ g.test('visibility,VERTEX_shader_stage_storage_texture_access') const { shaderStage, access } = t.params; const appliedAccess = access ?? 'write-only'; - const success = !( - // If visibility includes VERETX, storageTexture.access must be "read-only" - (shaderStage & GPUShaderStage.VERTEX && appliedAccess !== 'read-only') - ); + const success = + !( + // If visibility includes VERETX, storageTexture.access must be "read-only" + (shaderStage & GPUShaderStage.VERTEX && appliedAccess !== 'read-only') + ) && isValidStorageTextureForStages(t.device, shaderStage); t.expectValidationError(() => { t.device.createBindGroupLayout({ @@ -235,9 +288,9 @@ g.test('max_dynamic_buffers') const info = bufferBindingTypeInfo({ type }); const limitName = info.perPipelineLimitClass.maxDynamicLimit; - const bufferCount = limitName ? t.getDefaultLimit(limitName) : 0; + const bufferCount = limitName ? t.device.limits[limitName]! : 0; const dynamicBufferCount = bufferCount + extraDynamicBuffers; - const perStageLimit = t.getDefaultLimit(info.perStageLimitClass.maxLimit); + const perStageLimit = t.device.limits[info.perStageLimitClass.maxLimits.COMPUTE]!; const entries = []; for (let i = 0; i < dynamicBufferCount; i++) { @@ -319,9 +372,11 @@ g.test('max_resources_per_stage,in_bind_group_layout') .fn(t => { const { maxedEntry, extraEntry, maxedVisibility, extraVisibility } = t.params; const maxedTypeInfo = bindingTypeInfo(maxedEntry); - const maxedCount = t.getDefaultLimit(maxedTypeInfo.perStageLimitClass.maxLimit); + const maxedCount = getBindingLimitForBindingType(t.device, maxedVisibility, maxedEntry); const extraTypeInfo = bindingTypeInfo(extraEntry); + t.skipIf(!isValidBGLEntryForStages(t.device, extraVisibility, extraEntry)); + const maxResourceBindings: GPUBindGroupLayoutEntry[] = []; for (let i = 0; i < maxedCount; i++) { maxResourceBindings.push({ @@ -370,9 +425,11 @@ g.test('max_resources_per_stage,in_pipeline_layout') .fn(t => { const { maxedEntry, extraEntry, maxedVisibility, extraVisibility } = t.params; const maxedTypeInfo = bindingTypeInfo(maxedEntry); - const maxedCount = t.getDefaultLimit(maxedTypeInfo.perStageLimitClass.maxLimit); + const maxedCount = getBindingLimitForBindingType(t.device, maxedVisibility, maxedEntry); const extraTypeInfo = bindingTypeInfo(extraEntry); + t.skipIf(!isValidBGLEntryForStages(t.device, extraVisibility, extraEntry)); + const maxResourceBindings: GPUBindGroupLayoutEntry[] = []; for (let i = 0; i < maxedCount; i++) { maxResourceBindings.push({ diff --git a/src/webgpu/api/validation/createPipelineLayout.spec.ts b/src/webgpu/api/validation/createPipelineLayout.spec.ts index 6f7b80fa4a99..b2ec1dff747d 100644 --- a/src/webgpu/api/validation/createPipelineLayout.spec.ts +++ b/src/webgpu/api/validation/createPipelineLayout.spec.ts @@ -6,8 +6,13 @@ TODO: review existing tests, write descriptions, and make sure tests are complet import { makeTestGroup } from '../../../common/framework/test_group.js'; import { count } from '../../../common/util/util.js'; -import { bufferBindingTypeInfo, kBufferBindingTypes } from '../../capability_info.js'; +import { + bufferBindingTypeInfo, + getBindingLimitForBindingType, + kBufferBindingTypes, +} from '../../capability_info.js'; import { GPUConst } from '../../constants.js'; +import { MaxLimitsTestMixin } from '../../gpu_test.js'; import { ValidationTest } from './validation_test.js'; @@ -15,7 +20,7 @@ function clone(descriptor: T): T { return JSON.parse(JSON.stringify(descriptor)); } -export const g = makeTestGroup(ValidationTest); +export const g = makeTestGroup(MaxLimitsTestMixin(ValidationTest)); g.test('number_of_dynamic_buffers_exceeds_the_maximum_value') .desc( @@ -37,11 +42,10 @@ g.test('number_of_dynamic_buffers_exceeds_the_maximum_value') const { type, visibility } = t.params; const info = bufferBindingTypeInfo({ type }); const { maxDynamicLimit } = info.perPipelineLimitClass; - const perStageLimit = t.getDefaultLimit(info.perStageLimitClass.maxLimit); - const maxDynamic = Math.min( - maxDynamicLimit ? t.getDefaultLimit(maxDynamicLimit) : 0, - perStageLimit - ); + const limit = getBindingLimitForBindingType(t.device, visibility, { buffer: { type } }); + const maxDynamic = Math.min(maxDynamicLimit ? t.device.limits[maxDynamicLimit]! : 0, limit); + + t.skipIf(limit === 0, `binding limit for ${type} === 0`); const maxDynamicBufferBindings: GPUBindGroupLayoutEntry[] = []; for (let binding = 0; binding < maxDynamic; binding++) { @@ -60,7 +64,7 @@ g.test('number_of_dynamic_buffers_exceeds_the_maximum_value') entries: [{ binding: 0, visibility, buffer: { type, hasDynamicOffset: false } }], }; - if (perStageLimit > maxDynamic) { + if (limit > maxDynamic) { const goodPipelineLayoutDescriptor = { bindGroupLayouts: [ maxDynamicBufferBindGroupLayout, diff --git a/src/webgpu/capability_info.ts b/src/webgpu/capability_info.ts index a87ccc1fd7f6..661812cd4beb 100644 --- a/src/webgpu/capability_info.ts +++ b/src/webgpu/capability_info.ts @@ -387,18 +387,18 @@ export const kPerStageBindingLimits: { /** Which `PerShaderStage` binding limit class. */ readonly class: k; /** Maximum number of allowed bindings in that class. */ - readonly maxLimit: (typeof kLimits)[number]; + readonly maxLimits: { [key in ShaderStageKey]: (typeof kLimits)[number] }; // Add fields as needed }; } = /* prettier-ignore */ { - 'uniformBuf': { class: 'uniformBuf', maxLimit: 'maxUniformBuffersPerShaderStage', }, - 'storageBuf': { class: 'storageBuf', maxLimit: 'maxStorageBuffersPerShaderStage', }, - 'sampler': { class: 'sampler', maxLimit: 'maxSamplersPerShaderStage', }, - 'sampledTex': { class: 'sampledTex', maxLimit: 'maxSampledTexturesPerShaderStage', }, - 'readonlyStorageTex': { class: 'readonlyStorageTex', maxLimit: 'maxStorageTexturesPerShaderStage', }, - 'writeonlyStorageTex': { class: 'writeonlyStorageTex', maxLimit: 'maxStorageTexturesPerShaderStage', }, - 'readwriteStorageTex': { class: 'readwriteStorageTex', maxLimit: 'maxStorageTexturesPerShaderStage', }, + 'uniformBuf': { class: 'uniformBuf', maxLimits: { COMPUTE: 'maxUniformBuffersPerShaderStage', FRAGMENT: 'maxUniformBuffersPerShaderStage', VERTEX: 'maxUniformBuffersPerShaderStage' } }, + 'storageBuf': { class: 'storageBuf', maxLimits: { COMPUTE: 'maxStorageBuffersPerShaderStage', FRAGMENT: 'maxStorageBuffersInFragmentStage', VERTEX: 'maxStorageBuffersInVertexStage' } }, + 'sampler': { class: 'sampler', maxLimits: { COMPUTE: 'maxSamplersPerShaderStage', FRAGMENT: 'maxSamplersPerShaderStage', VERTEX: 'maxSamplersPerShaderStage' } }, + 'sampledTex': { class: 'sampledTex', maxLimits: { COMPUTE: 'maxSampledTexturesPerShaderStage', FRAGMENT: 'maxSampledTexturesPerShaderStage', VERTEX: 'maxSampledTexturesPerShaderStage' } }, + 'readonlyStorageTex': { class: 'readonlyStorageTex', maxLimits: { COMPUTE: 'maxStorageTexturesPerShaderStage', FRAGMENT: 'maxStorageTexturesInFragmentStage', VERTEX: 'maxStorageTexturesInVertexStage' } }, + 'writeonlyStorageTex': { class: 'writeonlyStorageTex', maxLimits: { COMPUTE: 'maxStorageTexturesPerShaderStage', FRAGMENT: 'maxStorageTexturesInFragmentStage', VERTEX: 'maxStorageTexturesInVertexStage' } }, + 'readwriteStorageTex': { class: 'readwriteStorageTex', maxLimits: { COMPUTE: 'maxStorageTexturesPerShaderStage', FRAGMENT: 'maxStorageTexturesInFragmentStage', VERTEX: 'maxStorageTexturesInVertexStage'} }, }; /** @@ -730,7 +730,11 @@ const [kLimitInfoKeys, kLimitInfoDefaults, kLimitInfoData] = 'maxDynamicStorageBuffersPerPipelineLayout': [ , 4, 4, ], 'maxSampledTexturesPerShaderStage': [ , 16, 16, ], 'maxSamplersPerShaderStage': [ , 16, 16, ], + 'maxStorageBuffersInFragmentStage': [ , 8, 0, ], + 'maxStorageBuffersInVertexStage': [ , 8, 0, ], 'maxStorageBuffersPerShaderStage': [ , 8, 4, ], + 'maxStorageTexturesInFragmentStage': [ , 4, 0, ], + 'maxStorageTexturesInVertexStage': [ , 4, 0, ], 'maxStorageTexturesPerShaderStage': [ , 4, 4, ], 'maxUniformBuffersPerShaderStage': [ , 12, 12, ], @@ -805,6 +809,42 @@ export function getDefaultLimitsForAdapter(adapter: GPUAdapter) { ); } +const kEachStage = [ + GPUConst.ShaderStage.COMPUTE, + GPUConst.ShaderStage.FRAGMENT, + GPUConst.ShaderStage.VERTEX, +]; +function shaderStageFlagToStageName(stage: GPUShaderStageFlags) { + switch (stage) { + case GPUConst.ShaderStage.COMPUTE: + return 'COMPUTE'; + case GPUConst.ShaderStage.FRAGMENT: + return 'FRAGMENT'; + case GPUConst.ShaderStage.VERTEX: + return 'VERTEX'; + default: + unreachable(); + } +} + +/** + * Get the limit of the number of things you can bind for + * a given BGLEntry given the specified visibility. This is + * the minimum across stages for the given visibility. + */ +export function getBindingLimitForBindingType( + device: GPUDevice, + visibility: GPUShaderStageFlags, + e: BGLEntry +) { + const info = bindingTypeInfo(e); + const maxLimits = info.perStageLimitClass.maxLimits; + const limits = kEachStage + .filter(stage => stage & visibility) + .map(stage => device.limits[maxLimits[shaderStageFlagToStageName(stage)]]!); + return limits.length > 0 ? Math.min(...limits) : 0; +} + /** List of all entries of GPUSupportedLimits. */ export const kLimits = keysOf(kLimitInfoCore); diff --git a/src/webgpu/gpu_test.ts b/src/webgpu/gpu_test.ts index 2719679b512e..53b286d8103c 100644 --- a/src/webgpu/gpu_test.ts +++ b/src/webgpu/gpu_test.ts @@ -21,12 +21,7 @@ import { unreachable, } from '../common/util/util.js'; -import { - getDefaultLimits, - kLimits, - kQueryTypeInfo, - WGSLLanguageFeature, -} from './capability_info.js'; +import { kLimits, kQueryTypeInfo, WGSLLanguageFeature } from './capability_info.js'; import { InterpolationType, InterpolationSampling } from './constants.js'; import { kTextureFormatInfo, @@ -143,10 +138,6 @@ export class GPUTestSubcaseBatchState extends SubcaseBatchState { return globalTestConfig.compatibility; } - getDefaultLimits() { - return getDefaultLimits(this.isCompatibility ? 'compatibility' : 'core'); - } - /** * Some tests or cases need particular feature flags or limits to be enabled. * Call this function with a descriptor or feature name (or `undefined`) to select a @@ -380,16 +371,8 @@ export class GPUTestBase extends Fixture { return globalTestConfig.compatibility; } - getDefaultLimits() { - return getDefaultLimits(this.isCompatibility ? 'compatibility' : 'core'); - } - - getDefaultLimit(limit: (typeof kLimits)[number]) { - return this.getDefaultLimits()[limit].default; - } - makeLimitVariant(limit: (typeof kLimits)[number], variant: ValueTestVariant) { - return makeValueTestVariant(this.device.limits[limit], variant); + return makeValueTestVariant(this.device.limits[limit]!, variant); } canCallCopyTextureToBufferWithTextureFormat(format: GPUTextureFormat) {