Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified src/resources/cache/webgpu/shader/execution/bitcast.bin
Binary file not shown.
93 changes: 93 additions & 0 deletions src/webgpu/api/validation/compute_pipeline.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ Note: entry point matching tests are in shader_module/entry_point.spec.ts
import { AllFeaturesMaxLimitsGPUTest } from '../.././gpu_test.js';
import { makeTestGroup } from '../../../common/framework/test_group.js';
import { keysOf } from '../../../common/util/data_tables.js';
import { getGPU } from '../../../common/util/navigator_gpu.js';
import { assert, supportsImmediateData } from '../../../common/util/util.js';
import {
isTextureFormatUsableWithStorageAccessMode,
kPossibleStorageTextureFormats,
Expand Down Expand Up @@ -811,3 +813,94 @@ generates a validation error at createComputePipeline(Async)
};
vtu.doCreateComputePipelineTest(t, isAsync, success, descriptor);
});

g.test('pipeline_creation_immediate_size_mismatch')
.desc(
`
Validate that creating a pipeline fails if the shader uses immediate data
larger than the immediateSize specified in the pipeline layout, or larger than
maxImmediateSize if layout is 'auto'.
Also validates that using less or equal size is allowed.
`
)
.params(u => {
const kNumericCases = [
{ shaderSize: 16, layoutSize: 16 }, // Equal
{ shaderSize: 12, layoutSize: 16 }, // Shader smaller
{ shaderSize: 20, layoutSize: 16 }, // Shader larger (small diff)
{ shaderSize: 32, layoutSize: 16 }, // Shader larger
] as const;
const kMaxLimitsCases = [
{ shaderSize: 'max', layoutSize: 'auto' }, // Shader equal to limit (auto layout)
{ shaderSize: 'exceedLimits', layoutSize: 'auto' }, // Shader larger than limit (auto layout)
] as const;
return u
.combine('isAsync', [true, false])
.combineWithParams([...kNumericCases, ...kMaxLimitsCases] as const);
})
Comment on lines +826 to +840
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This pattern is interesting, but I think I find it slightly harder to read than the more typical pattern that makes it clearer that these two arrays will be part of the same combineWithParams call:

.params(u => u
  .combine('isAsync', [true, false])
  .combineWithParams([
    // Numeric cases
    /* ... */
    // Max-limit cases
    /* ... */
  ])

Though the header comments aren't really necessary because it's already obvious what it means.

Suggested change
.params(u => {
const kNumericCases = [
{ shaderSize: 16, layoutSize: 16 }, // Equal
{ shaderSize: 12, layoutSize: 16 }, // Shader smaller
{ shaderSize: 20, layoutSize: 16 }, // Shader larger (small diff)
{ shaderSize: 32, layoutSize: 16 }, // Shader larger
] as const;
const kMaxLimitsCases = [
{ shaderSize: 'max', layoutSize: 'auto' }, // Shader equal to limit (auto layout)
{ shaderSize: 'exceedLimits', layoutSize: 'auto' }, // Shader larger than limit (auto layout)
] as const;
return u
.combine('isAsync', [true, false])
.combineWithParams([...kNumericCases, ...kMaxLimitsCases] as const);
})
.params(u => u
.combine('isAsync', [true, false])
.combineWithParams([
{ shaderSize: 16, layoutSize: 16 }, // Equal
{ shaderSize: 12, layoutSize: 16 }, // Shader smaller
{ shaderSize: 20, layoutSize: 16 }, // Shader larger (small diff)
{ shaderSize: 32, layoutSize: 16 }, // Shader larger
{ shaderSize: 'max', layoutSize: 'auto' }, // Shader equal to limit (auto layout)
{ shaderSize: 'exceedLimits', layoutSize: 'auto' }, // Shader larger than limit (auto layout)
] as const)
)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Learned this shape from copilot comments :) Glad to back to the common pattern.

.fn(t => {
t.skipIf(!supportsImmediateData(getGPU(t.rec)), 'Immediate data not supported');

const { isAsync, shaderSize, layoutSize } = t.params;

assert(t.device.limits.maxImmediateSize !== undefined);
const maxImmediateSize = t.device.limits.maxImmediateSize;
Comment on lines +846 to +847
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
assert(t.device.limits.maxImmediateSize !== undefined);
const maxImmediateSize = t.device.limits.maxImmediateSize;
const maxImmediateSize = t.device.limits.maxImmediateSize;
assert(maxImmediateSize !== undefined);


let actualLayout: GPUPipelineLayout | 'auto';
let validSize: number;

if (layoutSize === 'auto') {
actualLayout = 'auto';
validSize = maxImmediateSize;
} else {
actualLayout = t.device.createPipelineLayout({
bindGroupLayouts: [],
immediateSize: layoutSize as number,
});
validSize = layoutSize as number;
}

let actualShaderSize: number;
if (shaderSize === 'max') {
actualShaderSize = validSize;
} else if (shaderSize === 'exceedLimits') {
actualShaderSize = validSize + 4;
} else {
actualShaderSize = shaderSize as number;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think the type narrowing makes this cast unnecessary

Suggested change
actualShaderSize = shaderSize as number;
actualShaderSize = shaderSize;

}

// Ensure the test's fixed sizes fit within the device limit.
if (shaderSize !== 'exceedLimits') {
assert(
actualShaderSize <= maxImmediateSize,
`shaderSize (${actualShaderSize}) must be <= maxImmediateSize (${maxImmediateSize})`
);
}

const numFields = actualShaderSize / 4;
const fields = Array.from({ length: numFields }, (_, i) => `m${i}: u32`).join(', ');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: use range from src/common/util/util.ts

const code = `
struct Immediates { ${fields} }
var<immediate> data: Immediates;
fn use_data() { _ = data.m0; }
@compute @workgroup_size(1) fn main_compute() { use_data(); }
`;

const shouldError = actualShaderSize > validSize;

// When the shader exceeds the device's maxImmediateSize, the error occurs
// at shader module creation time, not pipeline creation time.
Comment on lines +891 to +892
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should be validated here, if we can avoid it. createShaderModule doesn't know whether a given entry point is going to be used or not. I am pretty sure WebGPU doesn't normally validate features or limits in createShaderModule.

Also, if we do end up allowing arrays, then overrides might be able to change the size of the immediate data, in which case we would have to duplicate the validation in pipeline creation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, makes sense. Then I think that's another implementation bug.

if (layoutSize === 'auto' && shouldError) {
t.expectValidationError(() => {
t.device.createShaderModule({ code });
});
return;
}

vtu.doCreateComputePipelineTest(t, isAsync, !shouldError, {
layout: actualLayout,
compute: {
module: t.device.createShaderModule({ code }),
},
});
});
141 changes: 139 additions & 2 deletions src/webgpu/api/validation/render_pipeline/misc.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ misc createRenderPipeline and createRenderPipelineAsync validation tests.
`;

import { makeTestGroup } from '../../../../common/framework/test_group.js';
import { getGPU } from '../../../../common/util/navigator_gpu.js';
import { assert, supportsImmediateData } from '../../../../common/util/util.js';
import {
isTextureFormatUsableWithStorageAccessMode,
kPossibleStorageTextureFormats,
Expand Down Expand Up @@ -119,8 +121,10 @@ g.test('pipeline_layout,device_mismatch')
});

g.test('external_texture')
.desc('Tests createRenderPipeline() with an external_texture')
.desc('Tests createRenderPipeline(Async) with an external_texture')
.params(u => u.combine('isAsync', [false, true]))
.fn(t => {
const { isAsync } = t.params;
const shader = t.device.createShaderModule({
code: `
@vertex
Expand Down Expand Up @@ -149,7 +153,7 @@ g.test('external_texture')
},
};

vtu.doCreateRenderPipelineTest(t, false, true, descriptor);
vtu.doCreateRenderPipelineTest(t, isAsync, true, descriptor);
});

g.test('storage_texture,format')
Expand Down Expand Up @@ -192,3 +196,136 @@ generates a validation error at createComputePipeline(Async)
};
vtu.doCreateRenderPipelineTest(t, isAsync, success, descriptor);
});

g.test('pipeline_creation_immediate_size_mismatch')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The render and compute tests are extremely similar, I would prefer to put them together in the same file (and probably share some code), e.g. api/validation/pipeline/immediates.spec.ts

Not sure how easy this is to do - consider it, but it's not that important.

.desc(
`
Validate that creating a pipeline fails if the shader uses immediate data
larger than the immediateSize specified in the pipeline layout, or larger than
maxImmediateSize if layout is 'auto'.
Also validates that using less or equal size is allowed.
`
)
.params(u => {
const kNumericCases = [
{ vertexSize: 16, fragmentSize: 16, layoutSize: 16 }, // Equal
{ vertexSize: 12, fragmentSize: 12, layoutSize: 16 }, // Shader smaller
{ vertexSize: 20, fragmentSize: 20, layoutSize: 16 }, // Shader larger (small diff)
{ vertexSize: 32, fragmentSize: 32, layoutSize: 16 }, // Shader larger
] as const;
const kMaxLimitsCases = [
{ vertexSize: 'max', fragmentSize: 0, layoutSize: 'auto' }, // Vertex = Limit (Control)
{ vertexSize: 0, fragmentSize: 'max', layoutSize: 'auto' }, // Fragment = Limit (Control)
{ vertexSize: 'max', fragmentSize: 'max', layoutSize: 'auto' }, // Both at Limit (Control)
{ vertexSize: 'exceedLimits', fragmentSize: 0, layoutSize: 'auto' }, // Vertex > Limit
{ vertexSize: 0, fragmentSize: 'exceedLimits', layoutSize: 'auto' }, // Fragment > Limit
] as const;
return u
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same suggestion here

.combine('isAsync', [true, false])
.combineWithParams([...kNumericCases, ...kMaxLimitsCases] as const);
})
.fn(t => {
t.skipIf(!supportsImmediateData(getGPU(t.rec)), 'Immediate data not supported');

const { isAsync, vertexSize, fragmentSize, layoutSize } = t.params;

assert(t.device.limits.maxImmediateSize !== undefined);
const maxImmediateSize = t.device.limits.maxImmediateSize;
Comment on lines +232 to +233
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same


const resolveSize = (sizeDescriptor: number | string) => {
if (typeof sizeDescriptor === 'number') return sizeDescriptor;
if (sizeDescriptor === 'max') return maxImmediateSize;
if (sizeDescriptor === 'exceedLimits') return maxImmediateSize + 4;
return 0;
};

const resolvedVertexImmediateSize = resolveSize(vertexSize);
const resolvedFragmentImmediateSize = resolveSize(fragmentSize);

// Ensure the test's fixed sizes fit within the device limit.
if (vertexSize !== 'exceedLimits') {
assert(
resolvedVertexImmediateSize <= maxImmediateSize,
`vertexSize (${resolvedVertexImmediateSize}) must be <= maxImmediateSize (${maxImmediateSize})`
);
}
if (fragmentSize !== 'exceedLimits') {
assert(
resolvedFragmentImmediateSize <= maxImmediateSize,
`fragmentSize (${resolvedFragmentImmediateSize}) must be <= maxImmediateSize (${maxImmediateSize})`
);
}

// Helper to generate a stage-specific shader module with the given immediate data size.
const makeShaderCode = (size: number, stage: 'vertex' | 'fragment') => {
if (size === 0) {
if (stage === 'vertex') {
return `@vertex fn main_vertex() -> @builtin(position) vec4<f32> { return vec4<f32>(0.0, 0.0, 0.0, 1.0); }`;
}
return `@fragment fn main_fragment() -> @location(0) vec4<f32> { return vec4<f32>(0.0, 1.0, 0.0, 1.0); }`;
}
const numFields = size / 4;
const fields = Array.from({ length: numFields }, (_, i) => `m${i}: u32`).join(', ');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

if (stage === 'vertex') {
return `
struct Immediates { ${fields} }
var<immediate> data: Immediates;
@vertex fn main_vertex() -> @builtin(position) vec4<f32> { _ = data.m0; return vec4<f32>(0.0, 0.0, 0.0, 1.0); }
`;
}
return `
struct Immediates { ${fields} }
var<immediate> data: Immediates;
@fragment fn main_fragment() -> @location(0) vec4<f32> { _ = data.m0; return vec4<f32>(0.0, 1.0, 0.0, 1.0); }
`;
};

let layout: GPUPipelineLayout | 'auto';
let validSize: number;

if (layoutSize === 'auto') {
layout = 'auto';
validSize = maxImmediateSize;
} else {
layout = t.device.createPipelineLayout({
bindGroupLayouts: [],
immediateSize: layoutSize as number,
});
validSize = layoutSize as number;
}

const vertexExceedsLimit = resolvedVertexImmediateSize > validSize;
const fragmentExceedsLimit = resolvedFragmentImmediateSize > validSize;
const shouldError = vertexExceedsLimit || fragmentExceedsLimit;

// When the shader exceeds the device's maxImmediateSize, the error occurs
// at shader module creation time, not pipeline creation time.
Comment on lines +301 to +302
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

// Create each shader module separately so the correct one gets the error.
const vertexCode = makeShaderCode(resolvedVertexImmediateSize, 'vertex');
const fragmentCode = makeShaderCode(resolvedFragmentImmediateSize, 'fragment');

if (layoutSize === 'auto' && vertexExceedsLimit) {
t.expectValidationError(() => {
t.device.createShaderModule({ code: vertexCode });
});
}
if (layoutSize === 'auto' && fragmentExceedsLimit) {
t.expectValidationError(() => {
t.device.createShaderModule({ code: fragmentCode });
});
}
if (layoutSize === 'auto' && shouldError) {
return;
}

vtu.doCreateRenderPipelineTest(t, isAsync, !shouldError, {
layout,
vertex: {
module: t.device.createShaderModule({ code: vertexCode }),
},
fragment: {
module: t.device.createShaderModule({ code: fragmentCode }),
targets: [{ format: 'rgba8unorm' }],
},
});
});