From ef5d2294db7a31d581f46d530c03a6089a63ac16 Mon Sep 17 00:00:00 2001 From: Kai Ninomiya Date: Mon, 30 Oct 2023 20:14:34 -0700 Subject: [PATCH 01/11] Test that DOMExceptions from WebGPU always have stacks (#3105) --- src/common/framework/fixture.ts | 31 +++++++++++++++---- src/common/util/util.ts | 20 ++++++++++-- src/unittests/loaders_and_trees.spec.ts | 5 ++- src/unittests/test_group.spec.ts | 2 +- .../operation/adapter/requestDevice.spec.ts | 2 ++ .../api/validation/buffer/mapping.spec.ts | 1 + .../features/texture_formats.spec.ts | 28 +++++++++++------ .../capability_checks/limits/limit_utils.ts | 10 +++--- src/webgpu/examples.spec.ts | 8 ++--- src/webgpu/util/device_pool.ts | 8 ++--- 10 files changed, 82 insertions(+), 33 deletions(-) diff --git a/src/common/framework/fixture.ts b/src/common/framework/fixture.ts index 381d60ba047a..795532406bd2 100644 --- a/src/common/framework/fixture.ts +++ b/src/common/framework/fixture.ts @@ -1,6 +1,6 @@ import { TestCaseRecorder } from '../internal/logging/test_case_recorder.js'; import { JSONWithUndefined } from '../internal/params_utils.js'; -import { assert, unreachable } from '../util/util.js'; +import { assert, ExceptionCheckOptions, unreachable } from '../util/util.js'; export class SkipTestCase extends Error {} export class UnexpectedPassError extends Error {} @@ -237,16 +237,26 @@ export class Fixture { } /** Expect that the provided promise rejects, with the provided exception name. */ - shouldReject(expectedName: string, p: Promise, msg?: string): void { + shouldReject( + expectedName: string, + p: Promise, + { allowMissingStack = false, message }: ExceptionCheckOptions = {} + ): void { this.eventualAsyncExpectation(async niceStack => { - const m = msg ? ': ' + msg : ''; + const m = message ? ': ' + message : ''; try { await p; niceStack.message = 'DID NOT REJECT' + m; this.rec.expectationFailed(niceStack); } catch (ex) { - niceStack.message = 'rejected as expected' + m; this.expectErrorValue(expectedName, ex, niceStack); + if (!allowMissingStack) { + if (!(ex instanceof Error && typeof ex.stack === 'string')) { + const exMessage = ex instanceof Error ? ex.message : '?'; + niceStack.message = `rejected as expected, but missing stack (${exMessage})${m}`; + this.rec.expectationFailed(niceStack); + } + } } }); } @@ -257,8 +267,12 @@ export class Fixture { * * MAINTENANCE_TODO: Change to `string | false` so the exception name is always checked. */ - shouldThrow(expectedError: string | boolean, fn: () => void, msg?: string): void { - const m = msg ? ': ' + msg : ''; + shouldThrow( + expectedError: string | boolean, + fn: () => void, + { allowMissingStack = false, message }: ExceptionCheckOptions = {} + ) { + const m = message ? ': ' + message : ''; try { fn(); if (expectedError === false) { @@ -271,6 +285,11 @@ export class Fixture { this.rec.expectationFailed(new Error('threw unexpectedly' + m)); } else { this.expectErrorValue(expectedError, ex, new Error(m)); + if (!allowMissingStack) { + if (!(ex instanceof Error && typeof ex.stack === 'string')) { + this.rec.expectationFailed(new Error('threw as expected, but missing stack' + m)); + } + } } } } diff --git a/src/common/util/util.ts b/src/common/util/util.ts index be109fc9d422..1da380036382 100644 --- a/src/common/util/util.ts +++ b/src/common/util/util.ts @@ -47,15 +47,29 @@ export function assertOK(value: Error | T): T { return value; } +/** Options for assertReject, shouldReject, and friends. */ +export type ExceptionCheckOptions = { allowMissingStack?: boolean; message?: string }; + /** * Resolves if the provided promise rejects; rejects if it does not. */ -export async function assertReject(p: Promise, msg?: string): Promise { +export async function assertReject( + expectedName: string, + p: Promise, + { allowMissingStack = false, message }: ExceptionCheckOptions = {} +): Promise { try { await p; - unreachable(msg); + unreachable(message); } catch (ex) { - // Assertion OK + // Asserted as expected + if (!allowMissingStack) { + const m = message ? ` (${message})` : ''; + assert( + ex instanceof Error && typeof ex.stack === 'string', + 'threw as expected, but missing stack' + m + ); + } } } diff --git a/src/unittests/loaders_and_trees.spec.ts b/src/unittests/loaders_and_trees.spec.ts index c7ff1fa43a84..b396b0525940 100644 --- a/src/unittests/loaders_and_trees.spec.ts +++ b/src/unittests/loaders_and_trees.spec.ts @@ -703,7 +703,10 @@ async function testIterateCollapsed( subqueriesToExpand: expectations, }); if (expectedResult === 'throws') { - t.shouldReject('Error', treePromise, 'loadTree should have thrown Error'); + t.shouldReject('Error', treePromise, { + // Some errors here use StacklessError to print nicer command line outputs. + allowMissingStack: true, + }); return; } const tree = await treePromise; diff --git a/src/unittests/test_group.spec.ts b/src/unittests/test_group.spec.ts index 8e1129411c74..526f577c9edf 100644 --- a/src/unittests/test_group.spec.ts +++ b/src/unittests/test_group.spec.ts @@ -206,7 +206,7 @@ g.test('invalid_test_name').fn(t => { () => { g.test(name).fn(() => {}); }, - name + { message: name } ); } }); diff --git a/src/webgpu/api/operation/adapter/requestDevice.spec.ts b/src/webgpu/api/operation/adapter/requestDevice.spec.ts index 7d930a5e19df..314da6356eb7 100644 --- a/src/webgpu/api/operation/adapter/requestDevice.spec.ts +++ b/src/webgpu/api/operation/adapter/requestDevice.spec.ts @@ -118,6 +118,7 @@ g.test('stale') // Cause a type error by requesting with an unknown feature. if (awaitInitialError) { await assertReject( + 'TypeError', adapter.requestDevice({ requiredFeatures: ['unknown-feature' as GPUFeatureName] }) ); } else { @@ -131,6 +132,7 @@ g.test('stale') // Cause an operation error by requesting with an alignment limit that is not a power of 2. if (awaitInitialError) { await assertReject( + 'OperationError', adapter.requestDevice({ requiredLimits: { minUniformBufferOffsetAlignment: 255 } }) ); } else { diff --git a/src/webgpu/api/validation/buffer/mapping.spec.ts b/src/webgpu/api/validation/buffer/mapping.spec.ts index c6f3c782af6d..58d7f2767aee 100644 --- a/src/webgpu/api/validation/buffer/mapping.spec.ts +++ b/src/webgpu/api/validation/buffer/mapping.spec.ts @@ -45,6 +45,7 @@ class F extends ValidationTest { assert(expectation.rejectName === null, 'mapAsync unexpectedly passed'); } catch (ex) { assert(ex instanceof Error, 'mapAsync rejected with non-error'); + assert(typeof ex.stack === 'string', 'mapAsync rejected without a stack'); assert(expectation.rejectName === ex.name, `mapAsync rejected unexpectedly with: ${ex}`); assert( expectation.earlyRejection === rejectedEarly, diff --git a/src/webgpu/api/validation/capability_checks/features/texture_formats.spec.ts b/src/webgpu/api/validation/capability_checks/features/texture_formats.spec.ts index 8654bc6feba6..eb7005dd29d0 100644 --- a/src/webgpu/api/validation/capability_checks/features/texture_formats.spec.ts +++ b/src/webgpu/api/validation/capability_checks/features/texture_formats.spec.ts @@ -274,6 +274,7 @@ g.test('color_target_state') ) .params(u => u + .combine('isAsync', [false, true]) .combine('format', kOptionalTextureFormats) .filter(t => !!kTextureFormatInfo[t.format].colorRender) .combine('enable_required_feature', [true, false]) @@ -287,10 +288,12 @@ g.test('color_target_state') } }) .fn(t => { - const { format, enable_required_feature } = t.params; + const { isAsync, format, enable_required_feature } = t.params; - t.shouldThrow(enable_required_feature ? false : 'TypeError', () => { - t.device.createRenderPipeline({ + t.doCreateRenderPipelineTest( + isAsync, + enable_required_feature, + { layout: 'auto', vertex: { module: t.device.createShaderModule({ @@ -313,8 +316,9 @@ g.test('color_target_state') entryPoint: 'main', targets: [{ format }], }, - }); - }); + }, + 'TypeError' + ); }); g.test('depth_stencil_state') @@ -326,6 +330,7 @@ g.test('depth_stencil_state') ) .params(u => u + .combine('isAsync', [false, true]) .combine('format', kOptionalTextureFormats) .filter(t => !!(kTextureFormatInfo[t.format].depth || kTextureFormatInfo[t.format].stencil)) .combine('enable_required_feature', [true, false]) @@ -339,10 +344,12 @@ g.test('depth_stencil_state') } }) .fn(t => { - const { format, enable_required_feature } = t.params; + const { isAsync, format, enable_required_feature } = t.params; - t.shouldThrow(enable_required_feature ? false : 'TypeError', () => { - t.device.createRenderPipeline({ + t.doCreateRenderPipelineTest( + isAsync, + enable_required_feature, + { layout: 'auto', vertex: { module: t.device.createShaderModule({ @@ -370,8 +377,9 @@ g.test('depth_stencil_state') entryPoint: 'main', targets: [{ format: 'rgba8unorm' }], }, - }); - }); + }, + 'TypeError' + ); }); g.test('render_bundle_encoder_descriptor_color_format') 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 f6b0f96aa805..47a5a468d7d1 100644 --- a/src/webgpu/api/validation/capability_checks/limits/limit_utils.ts +++ b/src/webgpu/api/validation/capability_checks/limits/limit_utils.ts @@ -330,7 +330,9 @@ export class LimitTestsImpl extends GPUTestBase { requiredFeatures?: GPUFeatureName[] ) { if (shouldReject) { - this.shouldReject('OperationError', adapter.requestDevice({ requiredLimits })); + this.shouldReject('OperationError', adapter.requestDevice({ requiredLimits }), { + allowMissingStack: true, + }); return undefined; } else { return await adapter.requestDevice({ requiredLimits, requiredFeatures }); @@ -562,12 +564,12 @@ export class LimitTestsImpl extends GPUTestBase { expectedName: string, p: Promise, shouldReject: boolean, - msg?: string + message?: string ): Promise { if (shouldReject) { - this.shouldReject(expectedName, p, msg); + this.shouldReject(expectedName, p, { message }); } else { - this.shouldResolve(p, msg); + this.shouldResolve(p, message); } // We need to explicitly wait for the promise because the device may be diff --git a/src/webgpu/examples.spec.ts b/src/webgpu/examples.spec.ts index 35969543741f..4864393eca91 100644 --- a/src/webgpu/examples.spec.ts +++ b/src/webgpu/examples.spec.ts @@ -47,7 +47,7 @@ g.test('basic').fn(t => { throw new TypeError(); }, // Log message. - 'function should throw Error' + { message: 'function should throw Error' } ); }); @@ -59,17 +59,17 @@ g.test('basic,async').fn(t => { // Promise expected to reject. Promise.reject(new TypeError()), // Log message. - 'Promise.reject should reject' + { message: 'Promise.reject should reject' } ); - // Promise can also be an IIFE. + // Promise can also be an IIFE (immediately-invoked function expression). t.shouldReject( 'TypeError', // eslint-disable-next-line @typescript-eslint/require-await (async () => { throw new TypeError(); })(), - 'Promise.reject should reject' + { message: 'Promise.reject should reject' } ); }); diff --git a/src/webgpu/util/device_pool.ts b/src/webgpu/util/device_pool.ts index e8584df19613..843d6dc83e1a 100644 --- a/src/webgpu/util/device_pool.ts +++ b/src/webgpu/util/device_pool.ts @@ -378,10 +378,10 @@ class DeviceHolder implements DeviceProvider { await this.device.queue.onSubmittedWorkDone(); } - await assertReject( - this.device.popErrorScope(), - 'There was an extra error scope on the stack after a test' - ); + await assertReject('OperationError', this.device.popErrorScope(), { + allowMissingStack: true, + message: 'There was an extra error scope on the stack after a test', + }); if (gpuOutOfMemoryError !== null) { assert(gpuOutOfMemoryError instanceof GPUOutOfMemoryError); From ccee5a98c3690540184a2539aca5bee4242c35b8 Mon Sep 17 00:00:00 2001 From: Kai Ninomiya Date: Tue, 31 Oct 2023 11:12:45 -0700 Subject: [PATCH 02/11] dev_server: Serve on localhost by default (#3115) * dev_server: serve on localhost only by default * Limit characters in route for /out/*/listing.js --- src/common/tools/dev_server.ts | 57 ++++++++++++++++++++++++---------- 1 file changed, 41 insertions(+), 16 deletions(-) diff --git a/src/common/tools/dev_server.ts b/src/common/tools/dev_server.ts index 2e0aca21ddd4..7d576a9a25be 100644 --- a/src/common/tools/dev_server.ts +++ b/src/common/tools/dev_server.ts @@ -14,6 +14,19 @@ import { makeListing } from './crawl.js'; // Make sure that makeListing doesn't cache imported spec files. See crawl(). process.env.STANDALONE_DEV_SERVER = '1'; +function usage(rc: number): void { + console.error(`\ +Usage: + tools/dev_server + tools/dev_server 0.0.0.0 + npm start + npm start 0.0.0.0 + +By default, serves on localhost only. If the argument 0.0.0.0 is passed, serves on all interfaces. +`); + process.exit(rc); +} + const srcDir = path.resolve(__dirname, '../../'); // Import the project's babel.config.js. We'll use the same config for the runtime compiler. @@ -110,7 +123,7 @@ app.use('/out-wpt', express.static(path.resolve(srcDir, '../out-wpt'))); app.use('/docs/tsdoc', express.static(path.resolve(srcDir, '../docs/tsdoc'))); // Serve a suite's listing.js file by crawling the filesystem for all tests. -app.get('/out/:suite/listing.js', async (req, res, next) => { +app.get('/out/:suite([a-zA-Z0-9_-]+)/listing.js', async (req, res, next) => { const suite = req.params['suite']; if (listingCache.has(suite)) { @@ -162,28 +175,40 @@ app.get('/out/**/*.js', async (req, res, next) => { } }); -const host = '0.0.0.0'; -const port = 8080; -// Find an available port, starting at 8080. -portfinder.getPort({ host, port }, (err, port) => { - if (err) { - throw err; +// Serve everything else (not .js) as static, and directories as directory listings. +app.use('/out', serveIndex(path.resolve(srcDir, '../src'))); +app.use('/out', express.static(path.resolve(srcDir, '../src'))); + +void (async () => { + let host = '127.0.0.1'; + if (process.argv.length >= 3) { + if (process.argv.length !== 3) usage(1); + if (process.argv[2] === '0.0.0.0') { + host = '0.0.0.0'; + } else { + usage(1); + } } + + console.log(`Finding an available port on ${host}...`); + const kPortFinderStart = 8080; + const port = await portfinder.getPortPromise({ host, port: kPortFinderStart }); + watcher.on('ready', () => { // Listen on the available port. app.listen(port, host, () => { console.log('Standalone test runner running at:'); - for (const iface of Object.values(os.networkInterfaces())) { - for (const details of iface || []) { - if (details.family === 'IPv4') { - console.log(` http://${details.address}:${port}/standalone/`); + if (host === '0.0.0.0') { + for (const iface of Object.values(os.networkInterfaces())) { + for (const details of iface || []) { + if (details.family === 'IPv4') { + console.log(` http://${details.address}:${port}/standalone/`); + } } } + } else { + console.log(` http://${host}:${port}/standalone/`); } }); }); -}); - -// Serve everything else (not .js) as static, and directories as directory listings. -app.use('/out', serveIndex(path.resolve(srcDir, '../src'))); -app.use('/out', express.static(path.resolve(srcDir, '../src'))); +})(); From fc58db8aad61b990ab734d8450d3500c6988bef7 Mon Sep 17 00:00:00 2001 From: Ryan Harrison Date: Tue, 31 Oct 2023 16:42:01 -0400 Subject: [PATCH 03/11] wgsl: Convert `quantizeToF16` to used `hfround` (#3118) Instead of passing the input through a F16Array, use the library provided function hfround. hfround is a fast look up table based rounding function for f16. Benchmarking locally this provides a ~20% improvement to fma interval calculations, which are particularly sensitive to quantization cost. Overall I was seeing more on the order of ~10% improvement. --- src/webgpu/util/math.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/webgpu/util/math.ts b/src/webgpu/util/math.ts index 380832e1b857..018d350a984d 100644 --- a/src/webgpu/util/math.ts +++ b/src/webgpu/util/math.ts @@ -3,6 +3,7 @@ import { assert } from '../../common/util/util.js'; import { Float16Array, getFloat16, + hfround, setFloat16, } from '../../external/petamoriken/float16/float16.js'; @@ -2021,13 +2022,9 @@ export function quantizeToF32(num: number): number { return quantizeToF32Data[0]; } -/** Statically allocate working data, so it doesn't need per-call creation */ -const quantizeToF16Data = new Float16Array(new ArrayBuffer(2)); - /** @returns the closest 16-bit floating point value to the input */ export function quantizeToF16(num: number): number { - quantizeToF16Data[0] = num; - return quantizeToF16Data[0]; + return hfround(num); } /** Statically allocate working data, so it doesn't need per-call creation */ From cb5b33c7d391b2beb1a14105ffe765c4930660b1 Mon Sep 17 00:00:00 2001 From: Ryan Harrison Date: Tue, 31 Oct 2023 16:55:38 -0400 Subject: [PATCH 04/11] wgsl: Convert `quantizeToF32` to used `Math.fround` (#3119) Instead of passing the input through a F32Array, use the builtin Math.fround. This leads to a ~5% improvement benchmarking locally. This is less than the equivalent f16 change, because F32Array is provided by the runtime, whereas F16Array is being polyfilled, so is probably more efficient to begin with. --- src/webgpu/util/math.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/webgpu/util/math.ts b/src/webgpu/util/math.ts index 018d350a984d..9b901bfa6da4 100644 --- a/src/webgpu/util/math.ts +++ b/src/webgpu/util/math.ts @@ -2013,13 +2013,9 @@ export interface QuantizeFunc { (num: number): number; } -/** Statically allocate working data, so it doesn't need per-call creation */ -const quantizeToF32Data = new Float32Array(new ArrayBuffer(4)); - /** @returns the closest 32-bit floating point value to the input */ export function quantizeToF32(num: number): number { - quantizeToF32Data[0] = num; - return quantizeToF32Data[0]; + return Math.fround(num); } /** @returns the closest 16-bit floating point value to the input */ From 2f3b68c51f5a759c2496885e30643fbcd98465fd Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Tue, 31 Oct 2023 20:39:25 +0000 Subject: [PATCH 05/11] Fix cache files being padded with trailing 0's buffer() was offseting the array instead of truncating the returned array. --- src/unittests/serialization.spec.ts | 14 +++++++------- .../shader/execution/expression/case_cache.ts | 10 +++++----- src/webgpu/util/binary_stream.ts | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/unittests/serialization.spec.ts b/src/unittests/serialization.spec.ts index 7f5384ad9cb5..9717ba3ecf84 100644 --- a/src/unittests/serialization.spec.ts +++ b/src/unittests/serialization.spec.ts @@ -209,7 +209,7 @@ g.test('value').fn(t => { ]) { const s = new BinaryStream(new Uint8Array(1024).buffer); serializeValue(s, value); - const d = new BinaryStream(s.buffer()); + const d = new BinaryStream(s.buffer().buffer); const deserialized = deserializeValue(d); t.expect( objectEquals(value, deserialized), @@ -246,7 +246,7 @@ g.test('fpinterval_f32').fn(t => { ]) { const s = new BinaryStream(new Uint8Array(1024).buffer); serializeFPInterval(s, interval); - const d = new BinaryStream(s.buffer()); + const d = new BinaryStream(s.buffer().buffer); const deserialized = deserializeFPInterval(d); t.expect( objectEquals(interval, deserialized), @@ -282,7 +282,7 @@ g.test('fpinterval_f16').fn(t => { ]) { const s = new BinaryStream(new Uint8Array(1024).buffer); serializeFPInterval(s, interval); - const d = new BinaryStream(s.buffer()); + const d = new BinaryStream(s.buffer().buffer); const deserialized = deserializeFPInterval(d); t.expect( objectEquals(interval, deserialized), @@ -318,7 +318,7 @@ g.test('fpinterval_abstract').fn(t => { ]) { const s = new BinaryStream(new Uint8Array(1024).buffer); serializeFPInterval(s, interval); - const d = new BinaryStream(s.buffer()); + const d = new BinaryStream(s.buffer().buffer); const deserialized = deserializeFPInterval(d); t.expect( objectEquals(interval, deserialized), @@ -340,7 +340,7 @@ g.test('expression_expectation').fn(t => { ]) { const s = new BinaryStream(new Uint8Array(1024).buffer); serializeExpectation(s, expectation); - const d = new BinaryStream(s.buffer()); + const d = new BinaryStream(s.buffer().buffer); const deserialized = deserializeExpectation(d); t.expect( objectEquals(expectation, deserialized), @@ -370,7 +370,7 @@ g.test('anyOf').fn(t => { ]) { const s = new BinaryStream(new Uint8Array(1024).buffer); serializeComparator(s, c.comparator); - const d = new BinaryStream(s.buffer()); + const d = new BinaryStream(s.buffer().buffer); const deserialized = deserializeComparator(d); for (const val of c.testCases) { const got = deserialized.compare(val); @@ -398,7 +398,7 @@ g.test('skipUndefined').fn(t => { ]) { const s = new BinaryStream(new Uint8Array(1024).buffer); serializeComparator(s, c.comparator); - const d = new BinaryStream(s.buffer()); + const d = new BinaryStream(s.buffer().buffer); const deserialized = deserializeComparator(d); for (const val of c.testCases) { const got = deserialized.compare(val); diff --git a/src/webgpu/shader/execution/expression/case_cache.ts b/src/webgpu/shader/execution/expression/case_cache.ts index daee31993161..ff82792d647d 100644 --- a/src/webgpu/shader/execution/expression/case_cache.ts +++ b/src/webgpu/shader/execution/expression/case_cache.ts @@ -166,13 +166,13 @@ export class CaseCache implements Cacheable> { */ serialize(data: Record): Uint8Array { const maxSize = 32 << 20; // 32MB - max size for a file - const s = new BinaryStream(new Uint8Array(maxSize).buffer); - s.writeU32(Object.keys(data).length); + const stream = new BinaryStream(new ArrayBuffer(maxSize)); + stream.writeU32(Object.keys(data).length); for (const name in data) { - s.writeString(name); - s.writeArray(data[name], serializeCase); + stream.writeString(name); + stream.writeArray(data[name], serializeCase); } - return new Uint8Array(s.buffer()); + return stream.buffer(); } /** diff --git a/src/webgpu/util/binary_stream.ts b/src/webgpu/util/binary_stream.ts index 2b32db9b3e06..a6512020e631 100644 --- a/src/webgpu/util/binary_stream.ts +++ b/src/webgpu/util/binary_stream.ts @@ -19,8 +19,8 @@ export default class BinaryStream { } /** buffer() returns the stream's buffer sliced to the 8-byte rounded read or write offset */ - buffer(): ArrayBufferLike { - return new Uint8Array(this.view.buffer, align(this.offset, 8)).buffer; + buffer(): Uint8Array { + return new Uint8Array(this.view.buffer, 0, align(this.offset, 8)); } /** writeBool() writes a boolean as 255 or 0 to the buffer at the next byte offset */ From ab09ed4f6c0811289d2d27a968b69f469cbf1d0c Mon Sep 17 00:00:00 2001 From: Ryan Harrison Date: Wed, 1 Nov 2023 11:00:01 -0400 Subject: [PATCH 06/11] wgsl: Convert `quantizeToI32/U32` to used `Math.trunc` (#3120) Another small bump (~5%) to be gained through using a builtin instead of trampolining through a TypedArray. --- src/webgpu/util/math.ts | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/src/webgpu/util/math.ts b/src/webgpu/util/math.ts index 9b901bfa6da4..851db40c7157 100644 --- a/src/webgpu/util/math.ts +++ b/src/webgpu/util/math.ts @@ -2023,22 +2023,32 @@ export function quantizeToF16(num: number): number { return hfround(num); } -/** Statically allocate working data, so it doesn't need per-call creation */ -const quantizeToI32Data = new Int32Array(new ArrayBuffer(4)); - -/** @returns the closest 32-bit signed integer value to the input */ +/** + * @returns the closest 32-bit signed integer value to the input, rounding + * towards 0, if not already an integer + */ export function quantizeToI32(num: number): number { - quantizeToI32Data[0] = num; - return quantizeToI32Data[0]; + if (num >= kValue.i32.positive.max) { + return kValue.i32.positive.max; + } + if (num <= kValue.i32.negative.min) { + return kValue.i32.negative.min; + } + return Math.trunc(num); } -/** Statically allocate working data, so it doesn't need per-call creation */ -const quantizeToU32Data = new Uint32Array(new ArrayBuffer(4)); - -/** @returns the closest 32-bit signed integer value to the input */ +/** + * @returns the closest 32-bit unsigned integer value to the input, rounding + * towards 0, if not already an integer + */ export function quantizeToU32(num: number): number { - quantizeToU32Data[0] = num; - return quantizeToU32Data[0]; + if (num >= kValue.u32.max) { + return kValue.u32.max; + } + if (num <= 0) { + return 0; + } + return Math.trunc(num); } /** @returns whether the number is an integer and a power of two */ From 0fac55fb096b4a6ca6213830f63829d56f854d19 Mon Sep 17 00:00:00 2001 From: Gregg Tavares Date: Wed, 1 Nov 2023 13:43:38 +0900 Subject: [PATCH 07/11] Prevent Double Runs and make run clearer. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prevents clicking a run button while tests are running. I seem to do this once in a while and get errors about DevicePool acquire device in use 😅 Also made the progress bar have a background color so it's slightly clearer that tests are running. --- src/common/runtime/standalone.ts | 9 +++++++++ standalone/index.html | 9 ++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/common/runtime/standalone.ts b/src/common/runtime/standalone.ts index be5887c1721e..1676f0ea58ad 100644 --- a/src/common/runtime/standalone.ts +++ b/src/common/runtime/standalone.ts @@ -427,11 +427,20 @@ function makeTreeNodeHeaderHTML( .attr('alt', runtext) .attr('title', runtext) .on('click', async () => { + if (runDepth > 0) { + showInfo('tests are already running'); + return; + } + showInfo(''); console.log(`Starting run for ${n.query}`); + // turn off all run buttons + $('#resultsVis').addClass('disable-run'); const startTime = performance.now(); await runSubtree(); const dt = performance.now() - startTime; const dtMinutes = dt / 1000 / 60; + // turn on all run buttons + $('#resultsVis').removeClass('disable-run'); console.log(`Finished run: ${dt.toFixed(1)} ms = ${dtMinutes.toFixed(1)} min`); }) .appendTo(header); diff --git a/standalone/index.html b/standalone/index.html index 3c7d41e80bd2..85ce1a9e6e93 100644 --- a/standalone/index.html +++ b/standalone/index.html @@ -148,7 +148,7 @@ width: 100%; left: 0; top: 0; - background-color: #000; + background-color: #068; color: #fff; align-items: center; } @@ -170,6 +170,13 @@ border-right: 1px solid var(--results-fg-color); } + /* PS: this does not disable using the keyboard to click */ + #resultsVis.disable-run button.leafrun, + #resultsVis.disable-run button.subtreerun { + pointer-events: none; + opacity: 25%; + } + /* tree nodes */ .nodeheader { From a71c816d0bed065d7b3108788f36aacc67524d15 Mon Sep 17 00:00:00 2001 From: Kai Ninomiya Date: Wed, 1 Nov 2023 17:43:47 -0700 Subject: [PATCH 08/11] Reduce draw validation test parameterization (#3122) * Reduce buffer_binding_overlap subcases by 89% * vertex_buffer_OOB: no-op refactor subcase parameterization * vertex_buffer_OOB: reduce subcases by 43% * vertex_buffer_OOB: reduce subcases by 50% * vertex_buffer_OOB: more subcases per case This helps a lot in standalone (18s -> 12s) but it probably won't help quite as much in other modes. * vertex_buffer_OOB: reduce cases by 44% * Revert buffer_binding_overlap changes, add TODO --- .../encoding/cmds/render/draw.spec.ts | 62 +++++++++++++------ 1 file changed, 43 insertions(+), 19 deletions(-) diff --git a/src/webgpu/api/validation/encoding/cmds/render/draw.spec.ts b/src/webgpu/api/validation/encoding/cmds/render/draw.spec.ts index 1efd16483430..aaac9b3b4363 100644 --- a/src/webgpu/api/validation/encoding/cmds/render/draw.spec.ts +++ b/src/webgpu/api/validation/encoding/cmds/render/draw.spec.ts @@ -404,27 +404,46 @@ success/error as expected. Such set of buffer parameters should include cases li u // type of draw call .combine('type', ['draw', 'drawIndexed', 'drawIndirect', 'drawIndexedIndirect'] as const) - // the state of vertex step mode vertex buffer bound size - .combine('VBSize', ['zero', 'exile', 'enough'] as const) - // the state of instance step mode vertex buffer bound size - .combine('IBSize', ['zero', 'exile', 'enough'] as const) + // VBSize: the state of vertex step mode vertex buffer bound size + // IBSize: the state of instance step mode vertex buffer bound size + .combineWithParams([ + { VBSize: 'exact', IBSize: 'exact' }, + { VBSize: 'zero', IBSize: 'exact' }, + { VBSize: 'oneTooSmall', IBSize: 'exact' }, + { VBSize: 'exact', IBSize: 'zero' }, + { VBSize: 'exact', IBSize: 'oneTooSmall' }, + ] as const) + // the state of array stride + .combine('AStride', ['zero', 'exact', 'oversize'] as const) + .beginSubcases() // should the vertex stride count be zero .combine('VStride0', [false, true] as const) // should the instance stride count be zero .combine('IStride0', [false, true] as const) - // the state of array stride - .combine('AStride', ['zero', 'exact', 'oversize'] as const) // the factor for offset of attributes in vertex layout .combine('offset', [0, 1, 2, 7]) // the offset of attribute will be factor * MIN(4, sizeof(vertexFormat)) - .beginSubcases() - .combine('setBufferOffset', [0, 200]) // must be a multiple of 4 + .combine('setBufferOffset', [200]) // must be a multiple of 4 .combine('attributeFormat', ['snorm8x2', 'float32', 'float16x4'] as GPUVertexFormat[]) - .combine('vertexCount', [0, 1, 10000]) - .combine('firstVertex', [0, 10000]) - .filter(p => p.VStride0 === (p.firstVertex + p.vertexCount === 0)) - .combine('instanceCount', [0, 1, 10000]) - .combine('firstInstance', [0, 10000]) - .filter(p => p.IStride0 === (p.firstInstance + p.instanceCount === 0)) + .expandWithParams(p => + p.VStride0 + ? [{ firstVertex: 0, vertexCount: 0 }] + : [ + { firstVertex: 0, vertexCount: 1 }, + { firstVertex: 0, vertexCount: 10000 }, + { firstVertex: 10000, vertexCount: 0 }, + { firstVertex: 10000, vertexCount: 10000 }, + ] + ) + .expandWithParams(p => + p.IStride0 + ? [{ firstInstance: 0, instanceCount: 0 }] + : [ + { firstInstance: 0, instanceCount: 1 }, + { firstInstance: 0, instanceCount: 10000 }, + { firstInstance: 10000, instanceCount: 0 }, + { firstInstance: 10000, instanceCount: 10000 }, + ] + ) .unless(p => p.vertexCount === 10000 && p.instanceCount === 10000) ) .fn(t => { @@ -459,7 +478,7 @@ success/error as expected. Such set of buffer parameters should include cases li } const calcSetBufferSize = ( - boundBufferSizeState: 'zero' | 'exile' | 'enough', + boundBufferSizeState: 'zero' | 'oneTooSmall' | 'exact', strideCount: number ): number => { let requiredBufferSize: number; @@ -475,11 +494,11 @@ success/error as expected. Such set of buffer parameters should include cases li setBufferSize = 0; break; } - case 'exile': { + case 'oneTooSmall': { setBufferSize = requiredBufferSize - 1; break; } - case 'enough': { + case 'exact': { setBufferSize = requiredBufferSize; break; } @@ -566,11 +585,11 @@ success/error as expected. Such set of buffer parameters should include cases li } const isVertexBufferOOB = - boundVertexBufferSizeState !== 'enough' && + boundVertexBufferSizeState !== 'exact' && drawType === 'draw' && // drawIndirect, drawIndexed, and drawIndexedIndirect do not validate vertex step mode buffer !zeroVertexStrideCount; // vertex step mode buffer never OOB if stride count = 0 const isInstanceBufferOOB = - boundInstanceBufferSizeState !== 'enough' && + boundInstanceBufferSizeState !== 'exact' && (drawType === 'draw' || drawType === 'drawIndexed') && // drawIndirect and drawIndexedIndirect do not validate instance step mode buffer !zeroInstanceStrideCount; // vertex step mode buffer never OOB if stride count = 0 const isFinishSuccess = !isVertexBufferOOB && !isInstanceBufferOOB; @@ -586,6 +605,11 @@ g.test(`buffer_binding_overlap`) In this test we test that binding one GPU buffer to multiple vertex buffer slot or both vertex buffer slot and index buffer will cause no validation error, with completely/partial overlap. - x= all draw types + + TODO: The "Factor" parameters don't necessarily guarantee that we test all configurations + of buffers overlapping or not. This test should be refactored to test specific overlap cases, + and have fewer total parameterizations. + See https://github.com/gpuweb/cts/pull/3122#discussion_r1378623214 ` ) .params(u => From eb70aafc253c1c6f6b493f4c8e6bb18868e53429 Mon Sep 17 00:00:00 2001 From: Kai Ninomiya Date: Wed, 1 Nov 2023 18:08:05 -0700 Subject: [PATCH 09/11] Optimize some checkElements* implementations (#3123) * remove call through checkElementsEqualGenerated (about 10% faster) * split checkElementsPassPredicate in half (about 40% faster??) * Implement checkElementsEqual/Generated directly (about 10% faster) ... instead of by calling into checkElementsPassPredicate. --- src/webgpu/util/check_contents.ts | 81 +++++++++++++++++++++++++++---- 1 file changed, 72 insertions(+), 9 deletions(-) diff --git a/src/webgpu/util/check_contents.ts b/src/webgpu/util/check_contents.ts index 5e0d2dfcebdf..298e7ae4a9e9 100644 --- a/src/webgpu/util/check_contents.ts +++ b/src/webgpu/util/check_contents.ts @@ -44,7 +44,28 @@ export function checkElementsEqual( ): ErrorWithExtra | undefined { assert(actual.constructor === expected.constructor, 'TypedArray type mismatch'); assert(actual.length === expected.length, 'size mismatch'); - return checkElementsEqualGenerated(actual, i => expected[i]); + + let failedElementsFirstMaybe: number | undefined = undefined; + /** Sparse array with `true` for elements that failed. */ + const failedElements: (true | undefined)[] = []; + for (let i = 0; i < actual.length; ++i) { + if (actual[i] !== expected[i]) { + failedElementsFirstMaybe ??= i; + failedElements[i] = true; + } + } + + if (failedElementsFirstMaybe === undefined) { + return undefined; + } + + const failedElementsFirst = failedElementsFirstMaybe; + return failCheckElements({ + actual, + failedElements, + failedElementsFirst, + predicatePrinter: [{ leftHeader: 'expected ==', getValueForCell: index => expected[index] }], + }); } /** @@ -117,11 +138,29 @@ export function checkElementsEqualGenerated( actual: TypedArrayBufferView, generator: CheckElementsGenerator ): ErrorWithExtra | undefined { - const error = checkElementsPassPredicate(actual, (index, value) => value === generator(index), { + let failedElementsFirstMaybe: number | undefined = undefined; + /** Sparse array with `true` for elements that failed. */ + const failedElements: (true | undefined)[] = []; + for (let i = 0; i < actual.length; ++i) { + if (actual[i] !== generator(i)) { + failedElementsFirstMaybe ??= i; + failedElements[i] = true; + } + } + + if (failedElementsFirstMaybe === undefined) { + return undefined; + } + + const failedElementsFirst = failedElementsFirstMaybe; + const error = failCheckElements({ + actual, + failedElements, + failedElementsFirst, predicatePrinter: [{ leftHeader: 'expected ==', getValueForCell: index => generator(index) }], }); - // If there was an error, extend it with additional extras. - return error ? new ErrorWithExtra(error, () => ({ generator })) : undefined; + // Add more extras to the error. + return new ErrorWithExtra(error, () => ({ generator })); } /** @@ -133,14 +172,10 @@ export function checkElementsPassPredicate( predicate: CheckElementsPredicate, { predicatePrinter }: { predicatePrinter?: CheckElementsSupplementalTableRows } ): ErrorWithExtra | undefined { - const size = actual.length; - const ctor = actual.constructor as TypedArrayBufferViewConstructor; - const printAsFloat = ctor === Float16Array || ctor === Float32Array || ctor === Float64Array; - let failedElementsFirstMaybe: number | undefined = undefined; /** Sparse array with `true` for elements that failed. */ const failedElements: (true | undefined)[] = []; - for (let i = 0; i < size; ++i) { + for (let i = 0; i < actual.length; ++i) { if (!predicate(i, actual[i])) { failedElementsFirstMaybe ??= i; failedElements[i] = true; @@ -150,7 +185,35 @@ export function checkElementsPassPredicate( if (failedElementsFirstMaybe === undefined) { return undefined; } + const failedElementsFirst = failedElementsFirstMaybe; + return failCheckElements({ actual, failedElements, failedElementsFirst, predicatePrinter }); +} + +interface CheckElementsFailOpts { + actual: TypedArrayBufferView; + failedElements: (true | undefined)[]; + failedElementsFirst: number; + predicatePrinter?: CheckElementsSupplementalTableRows; +} + +/** + * Implements the failure case of some checkElementsX helpers above. This allows those functions to + * implement their checks directly without too many function indirections in between. + * + * Note: Separating this into its own function significantly speeds up the non-error case in + * Chromium (though this may be V8-specific behavior). + */ +function failCheckElements({ + actual, + failedElements, + failedElementsFirst, + predicatePrinter, +}: CheckElementsFailOpts): ErrorWithExtra { + const size = actual.length; + const ctor = actual.constructor as TypedArrayBufferViewConstructor; + const printAsFloat = ctor === Float16Array || ctor === Float32Array || ctor === Float64Array; + const failedElementsLast = failedElements.length - 1; // Include one extra non-failed element at the beginning and end (if they exist), for context. From d13a52a037243090fe907bee68184bc58082b17d Mon Sep 17 00:00:00 2001 From: Kai Ninomiya Date: Wed, 1 Nov 2023 22:50:56 -0700 Subject: [PATCH 10/11] Restore missing test path validation (#3129) * Restore missing test path validation This fixes tools/validate so it actually does all the validation it was supposed to do. Also deduplicates recently-introduced (benign) duplicate subcases that it would have caught. Accidentally regressed in #2770. * Fix duplicate in unittest --- src/common/tools/crawl.ts | 2 ++ src/unittests/floating_point.spec.ts | 8 -------- .../operation/vertex_state/correctness.spec.ts | 18 ++++++++---------- 3 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/common/tools/crawl.ts b/src/common/tools/crawl.ts index cb1e5f6fc737..9e060f485903 100644 --- a/src/common/tools/crawl.ts +++ b/src/common/tools/crawl.ts @@ -83,6 +83,8 @@ export async function crawl(suiteDir: string, validate: boolean): Promise { + .expand('offsetVariant', function* (p) { const formatInfo = kVertexFormatInfo[p.format]; const formatSize = formatInfo.byteSize; - return [ - { mult: 0, add: 0 }, - { mult: 0, add: formatSize }, - { mult: 0, add: 4 }, - { mult: 0.5, add: 0 }, - { mult: 1, add: -formatSize * 2 }, - { mult: 1, add: -formatSize - 4 }, - { mult: 1, add: -formatSize }, - ]; + yield { mult: 0, add: 0 }; + yield { mult: 0, add: 4 }; + if (formatSize !== 4) yield { mult: 0, add: formatSize }; + yield { mult: 0.5, add: 0 }; + yield { mult: 1, add: -formatSize * 2 }; + if (formatSize !== 4) yield { mult: 1, add: -formatSize - 4 }; + yield { mult: 1, add: -formatSize }; }) ) .fn(t => { From a644dd01e42d0b175c22c826da0b4e342141433e Mon Sep 17 00:00:00 2001 From: Kai Ninomiya Date: Wed, 1 Nov 2023 23:20:59 -0700 Subject: [PATCH 11/11] Remove preview workflow, do WPT lint in PR workflow (#3131) --- .github/workflows/pr.yml | 21 ++++----- .github/workflows/workflow.yml | 80 ---------------------------------- .github/workflows/wpt.yml | 26 ----------- 3 files changed, 9 insertions(+), 118 deletions(-) delete mode 100644 .github/workflows/workflow.yml delete mode 100644 .github/workflows/wpt.yml diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 0ae4a0a3104f..a398bf13ac82 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -8,21 +8,18 @@ jobs: build: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2.3.1 + - uses: actions/checkout@v3 with: persist-credentials: false - - run: | - git fetch origin ${{ github.event.pull_request.head.sha }} - git checkout ${{ github.event.pull_request.head.sha }} - - uses: actions/setup-node@v2-beta + - uses: actions/setup-node@v3 with: node-version: "16.x" - run: npm ci - run: npm test - - run: | - mkdir deploy-build/ - cp -r README.md src standalone out docs deploy-build/ - - uses: actions/upload-artifact@v2 - with: - name: pr-artifact - path: deploy-build/ + - name: copy out-wpt to wpt tree + run: | + git clone --depth 2 https://github.com/web-platform-tests/wpt.git + rsync -av out-wpt/ wpt/webgpu + - name: test wpt lint + run: ./wpt lint + working-directory: ./wpt diff --git a/.github/workflows/workflow.yml b/.github/workflows/workflow.yml deleted file mode 100644 index 0d475a269e7b..000000000000 --- a/.github/workflows/workflow.yml +++ /dev/null @@ -1,80 +0,0 @@ -name: Workflow CI - -on: - workflow_run: - workflows: - - "Pull Request CI" - types: - - completed - -jobs: - build: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2.3.1 - with: - persist-credentials: false - - run: | - PR=$(curl https://api.github.com/search/issues?q=${{ github.event.workflow_run.head_sha }} | - grep -Po "(?<=${{ github.event.workflow_run.repository.full_name }}\/pulls\/)\d*" | head -1) - echo "PR=$PR" >> $GITHUB_ENV - - uses: actions/github-script@v3 - id: pr-artifact - with: - github-token: ${{secrets.GITHUB_TOKEN}} - result-encoding: string - script: | - const artifacts_url = context.payload.workflow_run.artifacts_url - const artifacts_req = await github.request(artifacts_url) - const artifact = artifacts_req.data.artifacts[0] - const download = await github.actions.downloadArtifact({ - owner: context.repo.owner, - repo: context.repo.repo, - artifact_id: artifact.id, - archive_format: "zip" - }) - return download.url - - run: | - rm -rf * - curl -L -o "pr-artifact.zip" "${{ steps.pr-artifact.outputs.result }}" - unzip -o pr-artifact.zip - rm pr-artifact.zip - - run: | - cat << EOF >> firebase.json - { - "hosting": { - "public": ".", - "ignore": [ - "firebase.json", - "**/.*", - "**/node_modules/**" - ] - } - } - EOF - cat << EOF >> .firebaserc - { - "projects": { - "default": "gpuweb-cts" - } - } - EOF - - id: deployment - continue-on-error: true - uses: FirebaseExtended/action-hosting-deploy@v0 - with: - firebaseServiceAccount: ${{ secrets.FIREBASE_SERVICE_ACCOUNT_CTS }} - expires: 10d - channelId: cts-prs-${{ env.PR }}-${{ github.event.workflow_run.head_sha }} - - uses: peter-evans/create-or-update-comment@v1 - continue-on-error: true - if: ${{ steps.deployment.outcome == 'success' }} - with: - issue-number: ${{ env.PR }} - body: | - Previews, as seen when this [build job](https://github.com/${{github.repository}}/actions/runs/${{github.run_id}}) started (${{ github.event.workflow_run.head_sha }}): - [**Run tests**](${{ steps.deployment.outputs.details_url }}/standalone/) | [**View tsdoc**](${{ steps.deployment.outputs.details_url }}/docs/tsdoc/) - diff --git a/.github/workflows/wpt.yml b/.github/workflows/wpt.yml deleted file mode 100644 index 729fd131c63d..000000000000 --- a/.github/workflows/wpt.yml +++ /dev/null @@ -1,26 +0,0 @@ -name: WPT Lint Test - -on: - pull_request: - branches: [main] - workflow_dispatch: - -jobs: - test-wpt-lint: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v3 - with: - persist-credentials: false - - uses: actions/setup-node@v3 - with: - node-version: "16.x" - - run: npm ci - - run: npm run wpt - - name: copy out-wpt to wpt tree - run: | - git clone --depth 2 https://github.com/web-platform-tests/wpt.git - rsync -av out-wpt/ wpt/webgpu - - name: test wpt lint - run: ./wpt lint - working-directory: ./wpt