Bug 198644 - [WHLSL] Hook up compute
Summary: [WHLSL] Hook up compute
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGPU (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P1 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
: 198001 198002 (view as bug list)
Depends on: 198163 198837
Blocks: 198600
  Show dependency treegraph
 
Reported: 2019-06-06 22:14 PDT by Myles C. Maxfield
Modified: 2019-06-13 22:20 PDT (History)
13 users (show)

See Also:


Attachments
WIP (33.42 KB, patch)
2019-06-06 22:19 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (63.30 KB, patch)
2019-06-07 14:48 PDT, Myles C. Maxfield
sbarati: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2019-06-06 22:14:17 PDT
Hook up compute
Comment 1 Myles C. Maxfield 2019-06-06 22:19:44 PDT
Created attachment 371561 [details]
WIP
Comment 2 Myles C. Maxfield 2019-06-06 22:20:49 PDT
*** Bug 198001 has been marked as a duplicate of this bug. ***
Comment 3 Myles C. Maxfield 2019-06-06 22:21:13 PDT
*** Bug 198002 has been marked as a duplicate of this bug. ***
Comment 4 Myles C. Maxfield 2019-06-07 14:48:50 PDT
Created attachment 371612 [details]
Patch
Comment 5 Myles C. Maxfield 2019-06-10 00:17:32 PDT
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.
Comment 6 Saam Barati 2019-06-10 01:01:46 PDT
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");

👌🏼
Comment 7 Radar WebKit Bug Importer 2019-06-11 19:09:50 PDT
<rdar://problem/51651567>
Comment 8 Myles C. Maxfield 2019-06-12 22:48:40 PDT
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.
Comment 9 Myles C. Maxfield 2019-06-12 23:07:04 PDT
Committed r246396: <https://trac.webkit.org/changeset/246396>
Comment 10 Antoine Quint 2019-06-12 23:28:11 PDT
Committed r246397: <https://trac.webkit.org/changeset/246397>
Comment 11 Antoine Quint 2019-06-12 23:29:17 PDT
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.
Comment 12 WebKit Commit Bot 2019-06-13 12:04:50 PDT
Re-opened since this is blocked by bug 198837
Comment 13 Shawn Roberts 2019-06-13 12:07:31 PDT
Rolled out in https://trac.webkit.org/changeset/246409/webkit 

Build logs were emailed to Myles
Comment 14 Shawn Roberts 2019-06-13 13:38:20 PDT
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.
Comment 15 Myles C. Maxfield 2019-06-13 22:20:28 PDT
Committed r246427: <https://trac.webkit.org/changeset/246427>