Bug 198644 – [WHLSL] Hook up compute

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

[Myles C. Maxfield]

Hook up compute

[Myles C. Maxfield]

Created attachment 371561 [details]
WIP

[Myles C. Maxfield]

*** Bug 198001 has been marked as a duplicate of this bug. ***

[Myles C. Maxfield]

*** Bug 198002 has been marked as a duplicate of this bug. ***

[Myles C. Maxfield]

Created attachment 371612 [details]
Patch

[Myles C. Maxfield]

Comment on attachment 371612 [details]
Patch

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

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLStandardLibrary.txt:444
> +/* native bool operator.x(bool2);

No need to comment out any of these functions.

[Saam Barati]

Comment on attachment 371612 [details]
Patch

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

r=me

> Source/WebCore/ChangeLog:18
> +        doesn't always match WHLSL's (and HLSL's type). For example, in WHLSL and HLSL,        SV_DispatchThreadID variables have to be a float3, but in Metal, they are a uint3.

Why do they have to be float3 in WHLSL?

> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLEntryPointScaffolding.cpp:195
> +        ASSERT(builtInSemantic.variable() == AST::BuiltInSemantic::Variable::SVGroupThreadID);

Nit: I like to write this out as a case, and omit the “default”. Then, when anyone adds a new enum value, this becomes a compile error

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLComputeDimensions.cpp:54
> +        Visitor::visit(functionDeclaration);

Why?

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLStandardLibrary.txt:596
> +native float operator.w(float4);/*

And this too?

> LayoutTests/webgpu/whlsl-compute.html:9
> +[numthreads(2, 1, 1)]

Out of curiosity, why do we have thread IDs in three dimensions? Is this just a useful format for a lot of shaders to avoid doing some math or something?

> LayoutTests/webgpu/whlsl-compute.html:64
> +    shouldBe("resultsFloat32Array[0]", "2");

👌🏼

[Radar WebKit Bug Importer]

<rdar://problem/51651567>

[Myles C. Maxfield]

Comment on attachment 371612 [details]
Patch

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

>> Source/WebCore/ChangeLog:18
>> +        doesn't always match WHLSL's (and HLSL's type). For example, in WHLSL and HLSL,        SV_DispatchThreadID variables have to be a float3, but in Metal, they are a uint3.
> 
> Why do they have to be float3 in WHLSL?

For compatibility with HLSL.

>> Source/WebCore/Modules/webgpu/WHLSL/WHLSLComputeDimensions.cpp:54
>> +        Visitor::visit(functionDeclaration);
> 
> Why?

Good catch.

>> LayoutTests/webgpu/whlsl-compute.html:9
>> +[numthreads(2, 1, 1)]
> 
> Out of curiosity, why do we have thread IDs in three dimensions? Is this just a useful format for a lot of shaders to avoid doing some math or something?

Yep. It's just how all the native APIs are designed.

[Myles C. Maxfield]

Committed r246396: <https://trac.webkit.org/changeset/246396>

[Antoine Quint]

Committed r246397: <https://trac.webkit.org/changeset/246397>

[Antoine Quint]

This broke the build, as shown by the bots. I made one small fix for iOS release builds. There may be more breakage since the Mac bots are also red.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 198837

[Shawn Roberts]

Rolled out in https://trac.webkit.org/changeset/246409/webkit 

Build logs were emailed to Myles

[Shawn Roberts]

I skipped the test you deleted in https://bugs.webkit.org/show_bug.cgi?id=198841 so it will not crash on the High Sierra bots again.

[Myles C. Maxfield]

Committed r246427: <https://trac.webkit.org/changeset/246427>