Bug 198890 – [WHLSL] Make .length work

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGPU

[Saam Barati]

...

[Saam Barati]

patch forthcoming

[Saam Barati]

Created attachment 372267 [details]
patch

[Myles C. Maxfield]

Comment on attachment 372267 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=372267&action=review

> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLEntryPointScaffolding.cpp:318
> -                stringBuilder.append(makeString("size_t ", lengthTemporaryName, " = ", variableName, '.', lengthElementName, ".x;\n"));
> +                stringBuilder.append(makeString("size_t ", lengthTemporaryName, " = ", variableName, '.', lengthElementName, ".y;\n"));

👍

> LayoutTests/webgpu/whlsl-buffer-length.html:10
> +compute void computeShader(device float[] buffer : register(u0), float3 threadID : SV_DispatchThreadID) {

Can we turn the buffer into a uint[] buffer? That way no casts are needed.

Your "numthreads" directive above means that this code is racey. Instead you should just say numthreads(1, 1, 1) and change line 56 below to say (1, 1, 1).

> LayoutTests/webgpu/whlsl-buffer-length.html:11
> +    buffer[uint(threadID.x)] = buffer[uint(threadID.x)] * 2.0;

What's the purpose of this line?

> LayoutTests/webgpu/whlsl-buffer-length.html:40
> +    for (let i = 0; i < 1337; ++i) {
> +        bufferFloat32Array[i] = i + 1;
> +    }

Is there a need to fill the whole buffer? Can't you just fill the first item and let it be overridden by the shader?

[Saam Barati]

Spoke to Myles offline, fixing the test to just do the length calculation.

[Saam Barati]

landed in:
https://trac.webkit.org/changeset/246515/webkit

[Radar WebKit Bug Importer]

<rdar://problem/51822107>