From 973068171048a6ab4c9d0762d5efdae8c2c1c8c4 Mon Sep 17 00:00:00 2001 From: Greggman Date: Sat, 11 Jan 2025 06:45:52 +0900 Subject: [PATCH] Refactor maxStorage(Buffers/Textures)PerShaderStage tests (#4137) These tests can't just set `maxStorage(Buffers/Textures)In(Fragment/Vertex)Stage to the maximum limit because those can't be set above their `..PerShaderStage` counterpart. Further, these were trying to use writable storage in the vertex stage which is not allowed. --- .../capability_checks/limits/limit_utils.ts | 66 ++++++++++++++++--- .../maxStorageBuffersPerShaderStage.spec.ts | 35 ++++++++-- .../maxStorageTexturesPerShaderStage.spec.ts | 48 ++++++++++++-- 3 files changed, 132 insertions(+), 17 deletions(-) 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 3f72b90901e..3ef275f51b1 100644 --- a/src/webgpu/api/validation/capability_checks/limits/limit_utils.ts +++ b/src/webgpu/api/validation/capability_checks/limits/limit_utils.ts @@ -7,9 +7,10 @@ import { getDefaultLimitsForAdapter, kLimits, } from '../../../../capability_info.js'; +import { GPUConst } from '../../../../constants.js'; import { GPUTestBase } from '../../../../gpu_test.js'; -type GPUSupportedLimit = keyof GPUSupportedLimits; +type GPUSupportedLimit = keyof Omit; export const kCreatePipelineTypes = [ 'createRenderPipeline', @@ -52,14 +53,14 @@ export function getPipelineTypeForBindingCombination(bindingCombination: Binding export function getStageVisibilityForBinidngCombination(bindingCombination: BindingCombination) { switch (bindingCombination) { case 'vertex': - return GPUShaderStage.VERTEX; + return GPUConst.ShaderStage.VERTEX; case 'fragment': - return GPUShaderStage.FRAGMENT; + return GPUConst.ShaderStage.FRAGMENT; case 'vertexAndFragmentWithPossibleVertexStageOverflow': case 'vertexAndFragmentWithPossibleFragmentStageOverflow': - return GPUShaderStage.FRAGMENT | GPUShaderStage.VERTEX; + return GPUConst.ShaderStage.FRAGMENT | GPUConst.ShaderStage.VERTEX; case 'compute': - return GPUShaderStage.COMPUTE; + return GPUConst.ShaderStage.COMPUTE; } } @@ -245,7 +246,7 @@ export function getPerStageWGSLForBindingCombinationStorageTextures( export const kLimitModes = ['defaultLimit', 'adapterLimit'] as const; export type LimitMode = (typeof kLimitModes)[number]; -export type LimitsRequest = Record; +export type LimitsRequest = Record; export const kMaximumTestValues = ['atLimit', 'overLimit'] as const; export type MaximumTestValue = (typeof kMaximumTestValues)[number]; @@ -338,6 +339,53 @@ export const kMinimumLimitBaseParams = kUnitCaseParamsBuilder .combine('limitTest', kMinimumLimitValueTests) .combine('testValueName', kMinimumTestValues); +/** + * Adds a maximum limit upto a dependent limit. + * + * Example: + * You want to test `maxStorageBuffersPerShaderStage` in fragment stage + * so you need `maxStorageBuffersInFragmentStage` set as well. But, you + * don't know exactly what value will be used for `maxStorageBuffersPerShaderStage` + * since that is defined by an enum like `underDefault`. + * + * So, you want `maxStorageBuffersInFragmentStage` to be set as high as possible. + * You can't just set it to it's maximum value (adapter.limits.maxStorageBuffersInFragmentStage) + * because if it's greater than `maxStorageBuffersPerShaderStage` you'll get an error. + * + * So, use this function + * + * const limits: LimitsRequest = {}; + * addMaximumLimitUpToDependentLimit( + * adapter, + * limits, + * limit: 'maxStorageBuffersInFragmentStage', // the limit we want to add + * dependentLimitName: 'maxStorageBuffersPerShaderStage', // what the previous limit is dependent on + * dependentLimitTest: 'underDefault', // the enum used to decide the dependent limit + * ) + */ +export function addMaximumLimitUpToDependentLimit( + adapter: GPUAdapter, + limits: LimitsRequest, + limit: GPUSupportedLimit, + dependentLimitName: GPUSupportedLimit, + dependentLimitTest: MaximumLimitValueTest +) { + if (!(limit in adapter.limits)) { + return; + } + + const limitMaximum: number = adapter.limits[limit]!; + const dependentLimitMaximum: number = adapter.limits[dependentLimitName]!; + const testValue = getLimitValue( + getDefaultLimitForAdapter(adapter, dependentLimitName), + dependentLimitMaximum, + dependentLimitTest + ); + + const value = Math.min(testValue, dependentLimitMaximum, limitMaximum); + limits[limit] = value; +} + export class LimitTestsImpl extends GPUTestBase { _adapter: GPUAdapter | null = null; _device: GPUDevice | undefined = undefined; @@ -415,11 +463,13 @@ export class LimitTestsImpl extends GPUTestBase { requiredLimits[limit] = requestedLimit; if (extraLimits) { - for (const [extraLimitStr, limitMode] of Object.entries(extraLimits)) { + for (const [extraLimitStr, limitModeOrNumber] of Object.entries(extraLimits)) { const extraLimit = extraLimitStr as GPUSupportedLimit; if (adapter.limits[extraLimit] !== undefined) { requiredLimits[extraLimit] = - limitMode === 'defaultLimit' + typeof limitModeOrNumber === 'number' + ? limitModeOrNumber + : limitModeOrNumber === 'defaultLimit' ? getDefaultLimitForAdapter(adapter, extraLimit) : (adapter.limits[extraLimit] as number); } diff --git a/src/webgpu/api/validation/capability_checks/limits/maxStorageBuffersPerShaderStage.spec.ts b/src/webgpu/api/validation/capability_checks/limits/maxStorageBuffersPerShaderStage.spec.ts index 3891d323d1f..582c6203864 100644 --- a/src/webgpu/api/validation/capability_checks/limits/maxStorageBuffersPerShaderStage.spec.ts +++ b/src/webgpu/api/validation/capability_checks/limits/maxStorageBuffersPerShaderStage.spec.ts @@ -17,13 +17,13 @@ import { getPerStageWGSLForBindingCombination, LimitsRequest, getStageVisibilityForBinidngCombination, + addMaximumLimitUpToDependentLimit, + MaximumLimitValueTest, } from './limit_utils.js'; const kExtraLimits: LimitsRequest = { maxBindingsPerBindGroup: 'adapterLimit', maxBindGroups: 'adapterLimit', - maxStorageBuffersInFragmentStage: 'adapterLimit', - maxStorageBuffersInVertexStage: 'adapterLimit', }; const limit = 'maxStorageBuffersPerShaderStage'; @@ -49,6 +49,31 @@ function createBindGroupLayout( return device.createBindGroupLayout(bindGroupLayoutDescription); } +function addExtraRequiredLimits( + adapter: GPUAdapter, + limits: LimitsRequest, + limitTest: MaximumLimitValueTest +) { + const newLimits: LimitsRequest = { ...limits }; + + addMaximumLimitUpToDependentLimit( + adapter, + newLimits, + 'maxStorageBuffersInFragmentStage', + limit, + limitTest + ); + addMaximumLimitUpToDependentLimit( + adapter, + newLimits, + 'maxStorageBuffersInVertexStage', + limit, + limitTest + ); + + return newLimits; +} + g.test('createBindGroupLayout,at_over') .desc( ` @@ -86,7 +111,7 @@ g.test('createBindGroupLayout,at_over') createBindGroupLayout(device, visibility, type, order, testValue); }, shouldError); }, - kExtraLimits + addExtraRequiredLimits(t.adapter, kExtraLimits, limitTest) ); }); @@ -141,7 +166,7 @@ g.test('createPipelineLayout,at_over') shouldError ); }, - kExtraLimits + addExtraRequiredLimits(t.adapter, kExtraLimits, limitTest) ); }); @@ -197,6 +222,6 @@ g.test('createPipeline,at_over') `actualLimit: ${actualLimit}, testValue: ${testValue}\n:${code}` ); }, - kExtraLimits + addExtraRequiredLimits(t.adapter, kExtraLimits, limitTest) ); }); diff --git a/src/webgpu/api/validation/capability_checks/limits/maxStorageTexturesPerShaderStage.spec.ts b/src/webgpu/api/validation/capability_checks/limits/maxStorageTexturesPerShaderStage.spec.ts index e32afb6e8c3..56304cc9b26 100644 --- a/src/webgpu/api/validation/capability_checks/limits/maxStorageTexturesPerShaderStage.spec.ts +++ b/src/webgpu/api/validation/capability_checks/limits/maxStorageTexturesPerShaderStage.spec.ts @@ -22,12 +22,13 @@ import { LimitTestsImpl, kBindingCombinations, getStageVisibilityForBinidngCombination, + MaximumLimitValueTest, + addMaximumLimitUpToDependentLimit, } from './limit_utils.js'; const kExtraLimits: LimitsRequest = { maxBindingsPerBindGroup: 'adapterLimit', maxBindGroups: 'adapterLimit', - maxStorageTexturesInFragmentStage: 'adapterLimit', }; const limit = 'maxStorageTexturesPerShaderStage'; @@ -92,6 +93,37 @@ function skipIfAccessNotSupported(t: LimitTestsImpl, access: GPUStorageTextureAc ); } +function filterWriteAccessInVertexStage( + visibility: GPUShaderStageFlags, + access: GPUStorageTextureAccess +) { + return access === 'read-only' || (visibility & GPUConst.ShaderStage.VERTEX) === 0; +} + +function addExtraRequiredLimits( + adapter: GPUAdapter, + limits: LimitsRequest, + limitTest: MaximumLimitValueTest +) { + const newLimits: LimitsRequest = { ...limits }; + + addMaximumLimitUpToDependentLimit( + adapter, + newLimits, + 'maxStorageTexturesInFragmentStage', + limit, + limitTest + ); + addMaximumLimitUpToDependentLimit( + adapter, + newLimits, + 'maxStorageTexturesInVertexStage', + limit, + limitTest + ); + + return newLimits; +} g.test('createBindGroupLayout,at_over') .desc( ` @@ -105,6 +137,7 @@ g.test('createBindGroupLayout,at_over') kMaximumLimitBaseParams .combine('visibility', kShaderStageCombinationsWithStage) .combine('access', kStorageTextureAccessValues) + .filter(t => filterWriteAccessInVertexStage(t.visibility, t.access)) .combine('order', kReorderOrderKeys) ) .fn(async t => { @@ -126,7 +159,7 @@ g.test('createBindGroupLayout,at_over') shouldError ); }, - kExtraLimits + addExtraRequiredLimits(t.adapter, kExtraLimits, limitTest) ); }); @@ -143,6 +176,7 @@ g.test('createPipelineLayout,at_over') kMaximumLimitBaseParams .combine('visibility', kShaderStageCombinationsWithStage) .combine('access', kStorageTextureAccessValues) + .filter(t => filterWriteAccessInVertexStage(t.visibility, t.access)) .combine('order', kReorderOrderKeys) ) .fn(async t => { @@ -178,7 +212,7 @@ g.test('createPipelineLayout,at_over') shouldError ); }, - kExtraLimits + addExtraRequiredLimits(t.adapter, kExtraLimits, limitTest) ); }); @@ -196,6 +230,12 @@ g.test('createPipeline,at_over') .combine('async', [false, true] as const) .combine('bindingCombination', kBindingCombinations) .combine('access', kStorageTextureAccessValues) + .filter(t => + filterWriteAccessInVertexStage( + getStageVisibilityForBinidngCombination(t.bindingCombination), + t.access + ) + ) .beginSubcases() .combine('order', kReorderOrderKeys) .combine('bindGroupTest', kBindGroupTests) @@ -244,6 +284,6 @@ g.test('createPipeline,at_over') `actualLimit: ${actualLimit}, testValue: ${testValue}\n:${code}` ); }, - kExtraLimits + addExtraRequiredLimits(t.adapter, kExtraLimits, limitTest) ); });