From 0a68bf71fb07b7ecd117d84d919fbaf81e302403 Mon Sep 17 00:00:00 2001 From: Greggman Date: Thu, 7 Mar 2024 20:59:20 -0800 Subject: [PATCH] Add AdapterLimitsGPUTest that sets all limits to the adapter's (#3466) Note: I'm not sure how I refactored device_pool.ts is good but, the old code created one initial device with no descriptor the first time in acquire. It then created another of the type actually wanted by the user. That no longer works becauase the adapter is passed in and so if the first requestDevice succeeds, then the adapter has been used and can't be used for the second one. Also, assuming this change is approved, I can (and should?) probably refactor LimitsTest to use this vs how it's doing it now. That would end up using cached devices. --- .../api/operation/rendering/draw.spec.ts | 4 +- src/webgpu/gpu_test.ts | 90 +++++++++++++++++-- src/webgpu/util/device_pool.ts | 32 ++++--- 3 files changed, 104 insertions(+), 22 deletions(-) diff --git a/src/webgpu/api/operation/rendering/draw.spec.ts b/src/webgpu/api/operation/rendering/draw.spec.ts index 6ed4be08fd24..fc8fdf2224d1 100644 --- a/src/webgpu/api/operation/rendering/draw.spec.ts +++ b/src/webgpu/api/operation/rendering/draw.spec.ts @@ -11,10 +11,10 @@ import { TypedArrayBufferView, TypedArrayBufferViewConstructor, } from '../../../../common/util/util.js'; -import { GPUTest, TextureTestMixin } from '../../../gpu_test.js'; +import { AdapterLimitsGPUTest, TextureTestMixin } from '../../../gpu_test.js'; import { PerPixelComparison } from '../../../util/texture/texture_ok.js'; -class DrawTest extends TextureTestMixin(GPUTest) { +class DrawTest extends TextureTestMixin(AdapterLimitsGPUTest) { checkTriangleDraw(opts: { firstIndex: number | undefined; count: number; diff --git a/src/webgpu/gpu_test.ts b/src/webgpu/gpu_test.ts index 1f021218d290..a2ae8f1b901f 100644 --- a/src/webgpu/gpu_test.ts +++ b/src/webgpu/gpu_test.ts @@ -93,11 +93,47 @@ export function initUncanonicalizedDeviceDescriptor( } } +/** + * Gets the adapter limits as a standard JavaScript object. + */ +function getAdapterLimitsAsDeviceRequiredLimits(adapter: GPUAdapter) { + const requiredLimits: Record = {}; + const adapterLimits = adapter.limits as unknown as Record; + for (const key in adapter.limits) { + requiredLimits[key] = adapterLimits[key]; + } + return requiredLimits; +} + +/** + * Conditionally applies adapter limits to device descriptor + * but does not overwrite existing requested limits. + */ +function conditionallyApplyAdapterLimitsToDeviceDescriptor( + adapter: GPUAdapter, + useAdapterLimits: boolean, + descriptor: UncanonicalizedDeviceDescriptor | undefined +): UncanonicalizedDeviceDescriptor { + return { + ...(descriptor || {}), + requiredLimits: { + ...(useAdapterLimits && getAdapterLimitsAsDeviceRequiredLimits(adapter)), + ...(descriptor?.requiredLimits || {}), + }, + }; +} + export class GPUTestSubcaseBatchState extends SubcaseBatchState { /** Provider for default device. */ private provider: Promise | undefined; /** Provider for mismatched device. */ private mismatchedProvider: Promise | undefined; + /** True if device should be created with adapter limits */ + private useAdapterLimits = false; + + constructor(recorder: TestCaseRecorder, params: TestParams) { + super(recorder, params); + } override async postInit(): Promise { // Skip all subcases if there's no device. @@ -123,6 +159,14 @@ export class GPUTestSubcaseBatchState extends SubcaseBatchState { return this.provider; } + useAdapterLimitsForDevice() { + assert( + this.provider === undefined, + 'useAdapterLimits must be called before getting the device' + ); + this.useAdapterLimits = true; + } + get isCompatibility() { return globalTestConfig.compatibility; } @@ -140,10 +184,18 @@ export class GPUTestSubcaseBatchState extends SubcaseBatchState { */ selectDeviceOrSkipTestCase(descriptor: DeviceSelectionDescriptor): void { assert(this.provider === undefined, "Can't selectDeviceOrSkipTestCase() multiple times"); - this.provider = devicePool.acquire( - this.recorder, - initUncanonicalizedDeviceDescriptor(descriptor) - ); + this.provider = devicePool + .requestAdapter(this.recorder) + .then(adapter => + devicePool.acquire( + adapter, + conditionallyApplyAdapterLimitsToDeviceDescriptor( + adapter, + this.useAdapterLimits, + initUncanonicalizedDeviceDescriptor(descriptor) + ) + ) + ); // Suppress uncaught promise rejection (we'll catch it later). this.provider.catch(() => {}); } @@ -201,10 +253,18 @@ export class GPUTestSubcaseBatchState extends SubcaseBatchState { "Can't selectMismatchedDeviceOrSkipTestCase() multiple times" ); - this.mismatchedProvider = mismatchedDevicePool.acquire( - this.recorder, - initUncanonicalizedDeviceDescriptor(descriptor) - ); + this.mismatchedProvider = mismatchedDevicePool + .requestAdapter(this.recorder) + .then(adapter => + mismatchedDevicePool.acquire( + adapter, + conditionallyApplyAdapterLimitsToDeviceDescriptor( + adapter, + this.useAdapterLimits, + initUncanonicalizedDeviceDescriptor(descriptor) + ) + ) + ); // Suppress uncaught promise rejection (we'll catch it later). this.mismatchedProvider.catch(() => {}); } @@ -1203,6 +1263,20 @@ export class GPUTest extends GPUTestBase { } } +/** + * A version of GPUTest that requires the adapter limits. + */ +export class AdapterLimitsGPUTest extends GPUTest { + public static override MakeSharedState( + recorder: TestCaseRecorder, + params: TestParams + ): GPUTestSubcaseBatchState { + const state = new GPUTestSubcaseBatchState(recorder, params); + state.useAdapterLimitsForDevice(); + return state; + } +} + /** * Texture expectation mixin can be applied on top of GPUTest to add texture * related expectation helpers. diff --git a/src/webgpu/util/device_pool.ts b/src/webgpu/util/device_pool.ts index 4e45aac76bb8..d0d1e6c0b6b5 100644 --- a/src/webgpu/util/device_pool.ts +++ b/src/webgpu/util/device_pool.ts @@ -24,16 +24,27 @@ export class TestOOMedShouldAttemptGC extends Error {} export class DevicePool { private holders: 'uninitialized' | 'failed' | DescriptorToHolderMap = 'uninitialized'; + async requestAdapter(recorder: TestCaseRecorder) { + const gpu = getGPU(recorder); + const adapter = await gpu.requestAdapter(); + assert(adapter !== null, 'requestAdapter returned null'); + return adapter; + } + /** Acquire a device from the pool and begin the error scopes. */ async acquire( - recorder: TestCaseRecorder, + adapter: GPUAdapter, descriptor?: UncanonicalizedDeviceDescriptor ): Promise { - let errorMessage = ''; + let holder; if (this.holders === 'uninitialized') { this.holders = new DescriptorToHolderMap(); + } + + let errorMessage = ''; + if (this.holders !== 'failed') { try { - await this.holders.getOrCreate(recorder, undefined); + holder = await this.holders.getOrCreate(adapter, descriptor); } catch (ex) { this.holders = 'failed'; if (ex instanceof Error) { @@ -46,9 +57,7 @@ export class DevicePool { this.holders !== 'failed', `WebGPU device failed to initialize${errorMessage}; not retrying` ); - - const holder = await this.holders.getOrCreate(recorder, descriptor); - + assert(!!holder); assert(holder.state === 'free', 'Device was in use on DevicePool.acquire'); holder.state = 'acquired'; holder.beginTestScope(); @@ -138,7 +147,7 @@ class DescriptorToHolderMap { * Throws SkipTestCase if devices with this descriptor are unsupported. */ async getOrCreate( - recorder: TestCaseRecorder, + adapter: GPUAdapter, uncanonicalizedDescriptor: UncanonicalizedDeviceDescriptor | undefined ): Promise { const [descriptor, key] = canonicalizeDescriptor(uncanonicalizedDescriptor); @@ -163,7 +172,7 @@ class DescriptorToHolderMap { // No existing item was found; add a new one. let value; try { - value = await DeviceHolder.create(recorder, descriptor); + value = await DeviceHolder.create(adapter, descriptor); } catch (ex) { if (ex instanceof FeaturesNotSupported) { this.unsupported.add(key); @@ -298,15 +307,14 @@ class DeviceHolder implements DeviceProvider { // Gets a device and creates a DeviceHolder. // If the device is lost, DeviceHolder.lost gets set. static async create( - recorder: TestCaseRecorder, + adapter: GPUAdapter, descriptor: CanonicalDeviceDescriptor | undefined ): Promise { - const gpu = getGPU(recorder); - const adapter = await gpu.requestAdapter(); - assert(adapter !== null, 'requestAdapter returned null'); + assert(adapter !== null, 'requestAdapter is null'); if (!supportsFeature(adapter, descriptor)) { throw new FeaturesNotSupported('One or more features are not supported'); } + const device = await adapter.requestDevice(descriptor); assert(device !== null, 'requestDevice returned null');