Skip to content

Commit d03e3d9

Browse files
authored
wgsl: Add coverage for uniform_buffer_standard_layout (#4484)
Change layout validation and execution tests to use the same constraints for `uniform` as `storage` when the language feature is supported.
1 parent 9504252 commit d03e3d9

File tree

7 files changed

+53
-29
lines changed

7 files changed

+53
-29
lines changed

src/webgpu/capability_info.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -936,6 +936,7 @@ export const kKnownWGSLLanguageFeatures = [
936936
'packed_4x8_integer_dot_product',
937937
'unrestricted_pointer_parameters',
938938
'pointer_composite_access',
939+
'uniform_buffer_standard_layout',
939940
] as const;
940941

941942
export type WGSLLanguageFeature = (typeof kKnownWGSLLanguageFeatures)[number];

src/webgpu/shader/execution/expression/access/array/index.spec.ts

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -24,18 +24,18 @@ g.test('concrete_scalar')
2424
.desc(`Test indexing of an array of concrete scalars`)
2525
.params(u =>
2626
u
27-
.combine(
28-
'inputSource',
29-
// 'uniform' address space requires array stride to be multiple of 16 bytes
30-
allInputSources.filter(s => s !== 'uniform')
31-
)
27+
.combine('inputSource', allInputSources)
3228
.combine('elementType', ['i32', 'u32', 'f32', 'f16'] as const)
3329
.combine('indexType', ['i32', 'u32'] as const)
3430
)
3531
.fn(async t => {
3632
if (t.params.elementType === 'f16') {
3733
t.skipIfDeviceDoesNotHaveFeature('shader-f16');
3834
}
35+
if (t.params.inputSource === 'uniform') {
36+
// 'uniform' address space requires array stride to be multiple of 16 bytes without this language feature.
37+
t.skipIfLanguageFeatureNotSupported('uniform_buffer_standard_layout');
38+
}
3939
const elementType = Type[t.params.elementType];
4040
const indexType = Type[t.params.indexType];
4141
const cases: Case[] = [
@@ -87,15 +87,14 @@ g.test('bool')
8787
.specURL('https://www.w3.org/TR/WGSL/#array-access-expr')
8888
.desc(`Test indexing of an array of booleans`)
8989
.params(u =>
90-
u
91-
.combine(
92-
'inputSource',
93-
// 'uniform' address space requires array stride to be multiple of 16 bytes
94-
allInputSources.filter(s => s !== 'uniform')
95-
)
96-
.combine('indexType', ['i32', 'u32'] as const)
90+
u.combine('inputSource', allInputSources).combine('indexType', ['i32', 'u32'] as const)
9791
)
9892
.fn(async t => {
93+
if (t.params.inputSource === 'uniform') {
94+
// 'uniform' address space requires array stride to be multiple of 16 bytes without this language feature.
95+
t.skipIfLanguageFeatureNotSupported('uniform_buffer_standard_layout');
96+
}
97+
9998
const indexType = Type[t.params.indexType];
10099
const cases: Case[] = [
101100
{
@@ -290,16 +289,16 @@ g.test('vector')
290289
.params(u =>
291290
u
292291
.combine('inputSource', allInputSources)
293-
.expand('elementType', t =>
294-
t.inputSource === 'uniform'
295-
? (['vec4i', 'vec4u', 'vec4f'] as const)
296-
: (['vec4i', 'vec4u', 'vec4f', 'vec4h'] as const)
297-
)
292+
.expand('elementType', _ => ['vec4i', 'vec4u', 'vec4f', 'vec4h'] as const)
298293
.combine('indexType', ['i32', 'u32'] as const)
299294
)
300295
.fn(async t => {
301296
if (t.params.elementType === 'vec4h') {
302297
t.skipIfDeviceDoesNotHaveFeature('shader-f16');
298+
if (t.params.inputSource === 'uniform') {
299+
// 'uniform' address space requires array stride to be multiple of 16 bytes without this language feature.
300+
t.skipIfLanguageFeatureNotSupported('uniform_buffer_standard_layout');
301+
}
303302
}
304303
const elementType = Type[t.params.elementType];
305304
const indexType = Type[t.params.indexType];
@@ -372,6 +371,14 @@ g.test('matrix')
372371
if (t.params.elementType === 'f16') {
373372
t.skipIfDeviceDoesNotHaveFeature('shader-f16');
374373
}
374+
if (
375+
t.params.inputSource === 'uniform' &&
376+
!t.hasLanguageFeature('uniform_buffer_standard_layout')
377+
) {
378+
// 'uniform' address space requires array stride to be multiple of 16 bytes without this language feature.
379+
const mat = Type.mat(t.params.columns, t.params.rows, Type[t.params.elementType]);
380+
t.skipIf((align(mat.size, mat.alignment) & 15) !== 0);
381+
}
375382
const elementType = Type[t.params.elementType];
376383
const indexType = Type[t.params.indexType];
377384
const matrixType = Type.mat(t.params.columns, t.params.rows, elementType);

src/webgpu/shader/execution/expression/expression.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ function sizeAndAlignmentOf(ty: Type, source: InputSource): { size: number; alig
102102

103103
if (ty instanceof ArrayType) {
104104
const out = sizeAndAlignmentOf(ty.elementType, source);
105+
// MAINTENANCE_TODO(#4485): Remove this when all implementors support uniform_buffer_standard_layout.
105106
if (source === 'uniform') {
106107
out.alignment = align(out.alignment, 16);
107108
}
@@ -155,6 +156,7 @@ export function structLayout(
155156
alignment = Math.max(alignment, sizeAndAlign.alignment);
156157
}
157158

159+
// MAINTENANCE_TODO(#4485): Remove this when all implementors support uniform_buffer_standard_layout.
158160
if (source === 'uniform') {
159161
alignment = align(alignment, 16);
160162
}

src/webgpu/shader/execution/memory_layout.spec.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -934,8 +934,12 @@ g.test('read_layout')
934934
`Skipping atomic test for non-storage address space`
935935
);
936936

937+
// If the `uniform_buffer_standard_layout` feature is supported, the `uniform` address space has
938+
// the same layout constraints as `storage`.
939+
const ubo_std_layout = t.hasLanguageFeature('uniform_buffer_standard_layout');
940+
937941
t.skipIf(
938-
testcase.skip_uniform === true && t.params.aspace === 'uniform',
942+
!ubo_std_layout && testcase.skip_uniform === true && t.params.aspace === 'uniform',
939943
`Uniform requires 16 byte alignment`
940944
);
941945
})

src/webgpu/shader/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,7 @@ export function* generateTypes({
298298
if (scalarInfo.layout) {
299299
// Compute the layout of the array type.
300300
// Adjust the array element count or element type as needed.
301+
// MAINTENANCE_TODO(#4485): Remove this when all implementors support uniform_buffer_standard_layout.
301302
if (addressSpace === 'uniform') {
302303
// Use a vec4 of the scalar type, to achieve a 16 byte alignment without internal padding.
303304
// This works for 4-byte scalar types, and does not work for f16.

src/webgpu/shader/validation/shader_io/align.spec.ts

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,10 @@ g.test('required_alignment')
202202
.beginSubcases()
203203
)
204204
.fn(t => {
205+
// If the `uniform_buffer_standard_layout` feature is supported, the `uniform` address space has
206+
// the same layout constraints as `storage`.
207+
const has_ubo_std_layout = t.hasLanguageFeature('uniform_buffer_standard_layout');
208+
205209
// While this would fail validation, it doesn't fail for any reasons related to alignment.
206210
// Atomics are not allowed in uniform address space as they have to be read_write.
207211
if (t.params.address_space === 'uniform' && t.params.type.name.startsWith('atomic')) {
@@ -213,20 +217,20 @@ g.test('required_alignment')
213217
code += 'enable f16;\n';
214218
}
215219

216-
// Testing the struct case, generate the structf
220+
// Testing the struct case, generate the struct
217221
if (t.params.type.name === 'S') {
218222
code += `struct S {
219223
a: mat4x2<f32>, // Align 8
220224
b: array<vec${
221-
t.params.address_space === 'storage' ? 2 : 4
225+
t.params.address_space === 'storage' || has_ubo_std_layout ? 2 : 4
222226
}<i32>, 2>, // Storage align 8, uniform 16
223227
}
224228
`;
225229
}
226230

227231
// Alignment value listed in the spec
228232
const min_align =
229-
t.params.address_space === 'storage'
233+
t.params.address_space === 'storage' || has_ubo_std_layout
230234
? `${t.params.type.storage}`
231235
: `${t.params.type.uniform}`;
232236
const align = t.params.align === 'alignment' ? min_align : t.params.align;
@@ -250,13 +254,14 @@ g.test('required_alignment')
250254
return vec4<f32>(.4, .2, .3, .1);
251255
}`;
252256

253-
// An array of `vec2` in uniform will not validate because, while the alignment on the array
254-
// itself is fine, the `vec2` element inside the array will have the wrong alignment. Uniform
255-
// requires that inner vec2 to have an align 16 which can only be done by specifying `vec4`
256-
// instead.
257-
const fails =
258-
(t.params.address_space === 'uniform' && t.params.type.name.startsWith('array<vec2')) ||
259-
align < min_align;
257+
let fails = align < min_align;
258+
if (!has_ubo_std_layout) {
259+
// An array of `vec2` in uniform will not validate because, while the alignment on the array
260+
// itself is fine, the `vec2` element inside the array will have the wrong alignment. Uniform
261+
// requires that inner vec2 to have an align 16 which can only be done by specifying `vec4`
262+
// instead.
263+
fails ||= t.params.address_space === 'uniform' && t.params.type.name.startsWith('array<vec2');
264+
}
260265

261266
t.expectCompileResult(!fails, code);
262267
});

src/webgpu/shader/validation/shader_io/layout_constraints.spec.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -525,11 +525,15 @@ ${decls}
525525
}
526526
code += `}\n`;
527527

528+
// If the `uniform_buffer_standard_layout` feature is supported, the `uniform` address space has
529+
// the same layout constraints as `storage`.
530+
const ubo_std_layout = t.hasLanguageFeature('uniform_buffer_standard_layout');
531+
528532
const is_interface = t.params.aspace === 'uniform' || t.params.aspace === 'storage';
529533
const supports_atomic = t.params.aspace === 'storage' || t.params.aspace === 'workgroup';
530534
const expect =
531535
testcase.validity === true ||
532-
(testcase.validity === 'non-uniform' && t.params.aspace !== 'uniform') ||
536+
(testcase.validity === 'non-uniform' && (t.params.aspace !== 'uniform' || ubo_std_layout)) ||
533537
(testcase.validity === 'non-interface' && !is_interface) ||
534538
(testcase.validity === 'storage' && t.params.aspace === 'storage') ||
535539
(testcase.validity === 'atomic' && supports_atomic);

0 commit comments

Comments
 (0)