From dd82761fc016b3f88dd59465ce0ea340b4584eba Mon Sep 17 00:00:00 2001 From: Alan Baker Date: Mon, 4 Mar 2024 14:34:42 -0500 Subject: [PATCH 1/2] Shader validation tests for atomics * type validation tests * address space * type * invalid operations * builtin function tests * parameterizations --- src/webgpu/listing_meta.json | 4 + .../expression/call/builtin/atomics.spec.ts | 145 ++++++++++++++++-- .../shader/validation/types/atomics.spec.ts | 145 ++++++++++++++++++ 3 files changed, 281 insertions(+), 13 deletions(-) create mode 100644 src/webgpu/shader/validation/types/atomics.spec.ts diff --git a/src/webgpu/listing_meta.json b/src/webgpu/listing_meta.json index f0585f3365e3..8755d929e9ac 100644 --- a/src/webgpu/listing_meta.json +++ b/src/webgpu/listing_meta.json @@ -1826,6 +1826,7 @@ "webgpu:shader,validation,expression,call,builtin,atan:values:*": { "subcaseMS": 0.335 }, "webgpu:shader,validation,expression,call,builtin,atanh:integer_argument:*": { "subcaseMS": 0.912 }, "webgpu:shader,validation,expression,call,builtin,atanh:values:*": { "subcaseMS": 0.231 }, + "webgpu:shader,validation,expression,call,builtin,atomics:atomic_parameterization:*": { "subcaseMS": 1.346 }, "webgpu:shader,validation,expression,call,builtin,atomics:stage:*": { "subcaseMS": 1.346 }, "webgpu:shader,validation,expression,call,builtin,barriers:no_return_value:*": { "subcaseMS": 1.500 }, "webgpu:shader,validation,expression,call,builtin,barriers:only_in_compute:*": { "subcaseMS": 1.500 }, @@ -2128,6 +2129,9 @@ "webgpu:shader,validation,types,alias:no_indirect_recursion_via_struct_attribute:*": { "subcaseMS": 1.584 }, "webgpu:shader,validation,types,alias:no_indirect_recursion_via_struct_member:*": { "subcaseMS": 1.000 }, "webgpu:shader,validation,types,alias:no_indirect_recursion_via_vector_element:*": { "subcaseMS": 1.050 }, + "webgpu:shader,validation,types,atomics:address_space:*": { "subcaseMS": 1.050 }, + "webgpu:shader,validation,types,atomics:invalid_operations:*": { "subcaseMS": 1.050 }, + "webgpu:shader,validation,types,atomics:type:*": { "subcaseMS": 1.050 }, "webgpu:shader,validation,types,struct:no_direct_recursion:*": { "subcaseMS": 0.951 }, "webgpu:shader,validation,types,struct:no_indirect_recursion:*": { "subcaseMS": 0.901 }, "webgpu:shader,validation,types,struct:no_indirect_recursion_via_array_element:*": { "subcaseMS": 0.901 }, diff --git a/src/webgpu/shader/validation/expression/call/builtin/atomics.spec.ts b/src/webgpu/shader/validation/expression/call/builtin/atomics.spec.ts index 57c5aae61334..788b044fcf63 100644 --- a/src/webgpu/shader/validation/expression/call/builtin/atomics.spec.ts +++ b/src/webgpu/shader/validation/expression/call/builtin/atomics.spec.ts @@ -8,18 +8,44 @@ import { ShaderValidationTest } from '../../../shader_validation_test.js'; export const g = makeTestGroup(ShaderValidationTest); -const kAtomicOps = { - add: { src: 'atomicAdd(&a,1)' }, - sub: { src: 'atomicSub(&a,1)' }, - max: { src: 'atomicMax(&a,1)' }, - min: { src: 'atomicMin(&a,1)' }, - and: { src: 'atomicAnd(&a,1)' }, - or: { src: 'atomicOr(&a,1)' }, - xor: { src: 'atomicXor(&a,1)' }, - load: { src: 'atomicLoad(&a)' }, - store: { src: 'atomicStore(&a,1)' }, - exchange: { src: 'atomicExchange(&a,1)' }, - compareexchangeweak: { src: 'atomicCompareExchangeWeak(&a,1,1)' }, +interface stringToString { + (a: string): string; +} + +const kAtomicOps: Record = { + add: (a: string): string => { + return `atomicAdd(${a},1)`; + }, + sub: (a: string): string => { + return `atomicSub(${a},1)`; + }, + max: (a: string): string => { + return `atomicMax(${a},1)`; + }, + min: (a: string): string => { + return `atomicMin(${a},1)`; + }, + and: (a: string): string => { + return `atomicAnd(${a},1)`; + }, + or: (a: string): string => { + return `atomicOr(${a},1)`; + }, + xor: (a: string): string => { + return `atomicXor(${a},1)`; + }, + load: (a: string): string => { + return `atomicLoad(${a})`; + }, + store: (a: string): string => { + return `atomicStore(${a},1)`; + }, + exchange: (a: string): string => { + return `atomicExchange(${a},1)`; + }, + compareexchangeweak: (a: string): string => { + return `atomicCompareExchangeWeak(${a},1,1)`; + }, }; g.test('stage') @@ -35,7 +61,7 @@ Atomic built-in functions must not be used in a vertex shader stage. .combine('atomicOp', keysOf(kAtomicOps)) ) .fn(t => { - const atomicOp = kAtomicOps[t.params.atomicOp].src; + const atomicOp = kAtomicOps[t.params.atomicOp](`&a`); let code = ` @group(0) @binding(0) var a: atomic; `; @@ -68,3 +94,96 @@ Atomic built-in functions must not be used in a vertex shader stage. const pass = t.params.stage !== 'vertex'; t.expectCompileResult(pass, code); }); + +function generateAtomicCode( + type: string, + access: string, + aspace: string, + style: string, + op: string +): string { + let moduleVar = ``; + let functionVar = ``; + let param = ``; + let aParam = ``; + if (style === 'var') { + aParam = `&a`; + switch (aspace) { + case 'storage': + moduleVar = `@group(0) @binding(0) var a : atomic<${type}>;\n`; + break; + case 'workgroup': + moduleVar = `var a : atomic<${type}>;\n`; + break; + case 'uniform': + moduleVar = `@group(0) @binding(0) var a : atomic<${type}>;\n`; + break; + case 'private': + moduleVar = `var a : atomic<${type}>;\n`; + break; + case 'function': + functionVar = `var a : atomic<${type}>;\n`; + break; + default: + break; + } + } else { + const aspaceParam = aspace === 'storage' ? `, ${access}` : ``; + param = `p : ptr<${aspace}, atomic<${type}>${aspaceParam}>`; + aParam = `p`; + } + + return ` +${moduleVar} +fn foo(${param}) { + ${functionVar} + ${kAtomicOps[op](aParam)}; +} +`; +} + +g.test('atomic_parameterization') + .desc('Tests the valid atomic parameters') + .params(u => + u + .combine('op', keysOf(kAtomicOps)) + .beginSubcases() + .combine('aspace', ['storage', 'workgroup', 'private', 'uniform', 'function'] as const) + .combine('access', ['read', 'read_write'] as const) + .combine('type', ['i32', 'u32'] as const) + .combine('style', ['param', 'var'] as const) + .filter(t => { + switch (t.aspace) { + case 'uniform': + return t.style === 'param' && t.access === 'read'; + case 'workgroup': + return t.access === 'read_write'; + case 'function': + case 'private': + return t.style === 'param' && t.access === 'read_write'; + default: + return true; + } + }) + ) + .fn(t => { + if ( + t.params.style === 'param' && + !(t.params.aspace === 'function' || t.params.aspace === 'private') + ) { + t.skipIfLanguageFeatureNotSupported('unrestricted_pointer_parameters'); + } + + const aspaceOK = t.params.aspace === 'storage' || t.params.aspace === 'workgroup'; + const accessOK = t.params.access === 'read_write'; + t.expectCompileResult( + aspaceOK && accessOK, + generateAtomicCode( + t.params.type, + t.params.access, + t.params.aspace, + t.params.style, + t.params.op + ) + ); + }); diff --git a/src/webgpu/shader/validation/types/atomics.spec.ts b/src/webgpu/shader/validation/types/atomics.spec.ts new file mode 100644 index 000000000000..36c37176e8af --- /dev/null +++ b/src/webgpu/shader/validation/types/atomics.spec.ts @@ -0,0 +1,145 @@ +export const description = ` +Validation tests for atomic types + +Tests covered: +* Base type +* Address spaces +* Invalid operations (non-exhaustive) + +Note: valid operations (e.g. atomic built-in functions) are tested in the builtin tests. +`; + +import { makeTestGroup } from '../../../../common/framework/test_group.js'; +import { keysOf } from '../../../../common/util/data_tables.js'; +import { ShaderValidationTest } from '../shader_validation_test.js'; + +export const g = makeTestGroup(ShaderValidationTest); + +g.test('type') + .desc('Test of the underlying atomic data type') + .specURL('https://gpuweb.github.io/gpuweb/wgsl/#atomic-types') + .params(u => + u.combine('type', [ + 'u32', + 'i32', + 'f32', + 'f16', + 'bool', + 'vec2u', + 'vec3i', + 'vec4f', + 'mat2x2f', + 'R', + 'S', + 'array', + 'array', + 'array', + 'array', + 'atomic', + 'atomic', + ] as const) + ) + .beforeAllSubcases(t => { + if (t.params.type === 'f16') { + t.selectDeviceOrSkipTestCase('shader-f16'); + } + }) + .fn(t => { + const code = ` +struct S { + x : u32 +} +struct T { + x : i32 +} +struct R { + x : f32 +} + +struct Test { + x : atomic<${t.params.type}> +} +`; + + const expect = t.params.type === 'u32' || t.params.type === 'i32'; + t.expectCompileResult(expect, code); + }); + +g.test('address_space') + .desc('Test allowed address spaces for atomics') + .specURL('https://gpuweb.github.io/gpuweb/wgsl/#atomic-types') + .params(u => + u + .combine('aspace', [ + 'storage', + 'workgroup', + 'storage-ro', + 'uniform', + 'private', + 'function', + 'function-let', + ] as const) + .beginSubcases() + .combine('type', ['i32', 'u32'] as const) + ) + .fn(t => { + let moduleVar = ``; + let functionVar = ''; + switch (t.params.aspace) { + case 'storage-ro': + moduleVar = `@group(0) @binding(0) var x : atomic<${t.params.type}>;\n`; + break; + case 'storage': + moduleVar = `@group(0) @binding(0) var x : atomic<${t.params.type}>;\n`; + break; + case 'uniform': + moduleVar = `@group(0) @binding(0) var x : atomic<${t.params.type}>;\n`; + break; + case 'workgroup': + case 'private': + moduleVar = `var<${t.params.aspace}> x : atomic<${t.params.type}>;\n`; + break; + case 'function': + functionVar = `var x : atomic<${t.params.type}>;\n`; + break; + case 'function-let': + functionVar = `let x : atomic<${t.params.type}>;\n`; + break; + } + const code = ` +${moduleVar} + +fn foo() { + ${functionVar} +} +`; + + const expect = t.params.aspace === 'storage' || t.params.aspace === 'workgroup'; + t.expectCompileResult(expect, code); + }); + +const kInvalidOperations = { + add: `a1 + a2`, + load: `a1`, + store: `a1 = 1u`, + deref: `*a1 = 1u`, + equality: `a1 == a2`, + abs: `abs(a1)`, + address_abs: `abs(&a1)`, +}; + +g.test('invalid_operations') + .desc('Tests that a selection of invalid operations are invalid') + .params(u => u.combine('op', keysOf(kInvalidOperations))) + .fn(t => { + const code = ` +var a1 : atomic; +var a2 : atomic; + +fn foo() { + let x : u32 = ${kInvalidOperations[t.params.op]}; +} +`; + + t.expectCompileResult(false, code); + }); From c46c21293ee0f86c5a0ce4b08f0ed62ae714a69e Mon Sep 17 00:00:00 2001 From: Alan Baker Date: Tue, 5 Mar 2024 10:04:01 -0500 Subject: [PATCH 2/2] More atomic validation * Data type validation * Return type validation --- src/webgpu/listing_meta.json | 2 + .../expression/call/builtin/atomics.spec.ts | 90 +++++++++++++++++++ 2 files changed, 92 insertions(+) diff --git a/src/webgpu/listing_meta.json b/src/webgpu/listing_meta.json index 8755d929e9ac..ec5c059d5149 100644 --- a/src/webgpu/listing_meta.json +++ b/src/webgpu/listing_meta.json @@ -1827,6 +1827,8 @@ "webgpu:shader,validation,expression,call,builtin,atanh:integer_argument:*": { "subcaseMS": 0.912 }, "webgpu:shader,validation,expression,call,builtin,atanh:values:*": { "subcaseMS": 0.231 }, "webgpu:shader,validation,expression,call,builtin,atomics:atomic_parameterization:*": { "subcaseMS": 1.346 }, + "webgpu:shader,validation,expression,call,builtin,atomics:data_parameters:*": { "subcaseMS": 38.382 }, + "webgpu:shader,validation,expression,call,builtin,atomics:return_types:*": { "subcaseMS": 28.021 }, "webgpu:shader,validation,expression,call,builtin,atomics:stage:*": { "subcaseMS": 1.346 }, "webgpu:shader,validation,expression,call,builtin,barriers:no_return_value:*": { "subcaseMS": 1.500 }, "webgpu:shader,validation,expression,call,builtin,barriers:only_in_compute:*": { "subcaseMS": 1.500 }, diff --git a/src/webgpu/shader/validation/expression/call/builtin/atomics.spec.ts b/src/webgpu/shader/validation/expression/call/builtin/atomics.spec.ts index 788b044fcf63..fdb85664d254 100644 --- a/src/webgpu/shader/validation/expression/call/builtin/atomics.spec.ts +++ b/src/webgpu/shader/validation/expression/call/builtin/atomics.spec.ts @@ -187,3 +187,93 @@ g.test('atomic_parameterization') ) ); }); + +g.test('data_parameters') + .desc('Validates that data parameters must match atomic type (or be implicitly convertible)') + .params(u => + u + .combine('op', [ + 'atomicStore', + 'atomicAdd', + 'atomicSub', + 'atomicMax', + 'atomicMin', + 'atomicAnd', + 'atomicOr', + 'atomicXor', + 'atomicExchange', + 'atomicCompareExchangeWeak1', + 'atomicCompareExchangeWeak2', + ] as const) + .beginSubcases() + .combine('atomicType', ['i32', 'u32'] as const) + .combine('dataType', ['i32', 'u32', 'f32', 'AbstractInt'] as const) + ) + .fn(t => { + let dataValue = ''; + switch (t.params.dataType) { + case 'i32': + dataValue = '1i'; + break; + case 'u32': + dataValue = '1u'; + break; + case 'f32': + dataValue = '1f'; + break; + case 'AbstractInt': + dataValue = '1'; + break; + } + let op = ''; + switch (t.params.op) { + case 'atomicCompareExchangeWeak1': + op = `atomicCompareExchangeWeak(&a, ${dataValue}, 1)`; + break; + case 'atomicCompareExchangeWeak2': + op = `atomicCompareExchangeWeak(&a, 1, ${dataValue})`; + break; + default: + op = `${t.params.op}(&a, ${dataValue})`; + break; + } + const code = ` +var a : atomic<${t.params.atomicType}>; +fn foo() { + ${op}; +} +`; + + const expect = t.params.atomicType === t.params.dataType || t.params.dataType === 'AbstractInt'; + t.expectCompileResult(expect, code); + }); + +g.test('return_types') + .desc('Validates return types of atomics') + .params(u => + u + .combine('op', keysOf(kAtomicOps)) + .beginSubcases() + .combine('atomicType', ['i32', 'u32'] as const) + .combine('returnType', ['i32', 'u32', 'f32'] as const) + ) + .fn(t => { + let op = `${kAtomicOps[t.params.op]('&a')}`; + switch (t.params.op) { + case 'compareexchangeweak': + op = `let tmp : ${t.params.returnType} = ${op}.old_value`; + break; + default: + op = `let tmp : ${t.params.returnType} = ${op}`; + break; + } + const code = ` +var a : atomic<${t.params.atomicType}>; +fn foo() { + ${op}; +} +`; + + const expect = t.params.atomicType === t.params.returnType && t.params.op !== 'store'; + t.expectCompileResult(expect, code); + });