webgpu: Support AdapterInfo #6862
Conversation
| @@ -146,6 +147,7 @@ export class WebGPUBackend extends KernelBackend { | |||
| this.currentCommandEncoder = null; | |||
| this.currentComputePass = null; | |||
| this.supportTimeQuery = device.features.has('timestamp-query'); | |||
| this.adapterInfo = adapterInfo; | |||
There was a problem hiding this comment.
WebGPUBackend is already very crowded. Can we put this and related methods in a standalone class, e.g., adapter?
| splitedDimInner = 32): string { | ||
| transposeA = false, tileInner = 32, splitK = false, splitedDimInner = 32, | ||
| sequentialAccess = | ||
| false /**By default, it's interval access in threads */): string { |
There was a problem hiding this comment.
It's better to move this comment to the earliest place we set this.
There was a problem hiding this comment.
This is the place that we introduce the sequentialAccess concept. So I think it's better to put the comments here. Also updated it with more details.
| @@ -220,7 +220,7 @@ export function conv2DImpl({ | |||
|
|
|||
| const program = new Conv2DMMProgram( | |||
| convInfo, dimAOuter, dimBOuter, dimInner, hasBias, activation, | |||
| hasPreluActivationWeights); | |||
| hasPreluActivationWeights, backend.isIntel()); | |||
There was a problem hiding this comment.
It's better to have a variable (e.g., sequentialAccess), and set it explicitly with backend.isIntel(). Some more comments are also needed here about this concept.
| @@ -270,10 +268,13 @@ const readDataFromSubASnippet = (transposeA: boolean) => { | |||
| 'let ACached = mm_Asub[tileRow + innerRow][k];'; | |||
| }; | |||
|
|
|||
| // sequentialAccess means that the adjacent threads are accessing contiguous | |||
| // data in memory. By default, it's interval access in threads, which means that | |||
| // the access is continuous in one thread, but not in theads. | |||
There was a problem hiding this comment.
theads -> threads. Maybe it's better to name it as sequentialAccessByThreads, and change the description as "sequentialAccessByThreads means sequential data in memory is accessed by threads, instead of a single thread (default behavior)".
| */ | ||
|
|
||
| export class AdapterInfo { | ||
| constructor(private adapterInfo: GPUAdapterInfo) {} |
There was a problem hiding this comment.
I think we need some class members here and initialize them with GPUAdapterInfo. I suggest to use vendor and architecture directly.
| @@ -50,7 +50,8 @@ if (isWebGPUSupported()) { | |||
| deviceDescriptor.requiredFeatures = ['timestamp-query']; | |||
| } | |||
| const device: GPUDevice = await adapter.requestDevice(deviceDescriptor); | |||
| return new WebGPUBackend(device); | |||
| const adapterInfo = await adapter.requestAdapterInfo(); | |||
| return new WebGPUBackend(device, adapterInfo); | |||
There was a problem hiding this comment.
Question: Can we only pass adapter to WebGPUBackend?
There was a problem hiding this comment.
We need to allow the user to specify the same device with tfjs to do interop. However, we can't do it if passing adapter to WebGPUBackend.
| return false; | ||
| } | ||
|
|
||
| isGen12GPU(): boolean { |
There was a problem hiding this comment.
Where is this used? If this is for other feature, we can add this later.
isGen12GPU is a bit too specific. What about Gen13 and future GPU gens?
upgrade webgpu/types Support sequential access mode Use sequential mode for intel device
| @@ -0,0 +1,37 @@ | |||
| /** | |||
| * @license | |||
| * Copyright 2022 Google LLC. All Rights Reserved. | |||
There was a problem hiding this comment.
According to #6871 (review), please remove "All Rights Reserved." for the new files.
Upgrade webgpu/types
Support sequential access mode
Use sequential mode for intel device
To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.
This change is