Bug 198644

Summary: [WHLSL] Hook up compute
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: WebGPUAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dino, ews-watchlist, fpizlo, graouts, graouts, jonlee, justin_fan, kondapallykalyan, rmorisset, saam, sroberts, webkit-bug-importer
Priority: P1 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=198841
Bug Depends on: 198163, 198837    
Bug Blocks: 198600    
Attachments:
Description Flags
WIP
none
Patch saam: review+

Myles C. Maxfield
Reported 2019-06-06 22:14:17 PDT
Hook up compute
Attachments
WIP (33.42 KB, patch)
2019-06-06 22:19 PDT, Myles C. Maxfield 		no flags
DetailsFormatted DiffDiff
Patch (63.30 KB, patch)
2019-06-07 14:48 PDT, Myles C. Maxfield 		saam: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Myles C. Maxfield
Comment 1 2019-06-06 22:19:44 PDT
Created attachment 371561 [details] WIP
Myles C. Maxfield
Comment 2 2019-06-06 22:20:49 PDT
*** Bug 198001 has been marked as a duplicate of this bug. ***
Myles C. Maxfield
Comment 3 2019-06-06 22:21:13 PDT
*** Bug 198002 has been marked as a duplicate of this bug. ***
Myles C. Maxfield
Comment 4 2019-06-07 14:48:50 PDT
Created attachment 371612 [details] Patch
Myles C. Maxfield
Comment 5 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.
Saam Barati
Comment 6 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"); 👌🏼
Radar WebKit Bug Importer
Comment 7 2019-06-11 19:09:50 PDT
<rdar://problem/51651567>
Myles C. Maxfield
Comment 8 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.
Myles C. Maxfield
Comment 9 2019-06-12 23:07:04 PDT
Committed r246396: <https://trac.webkit.org/changeset/246396>
Antoine Quint
Comment 10 2019-06-12 23:28:11 PDT
Committed r246397: <https://trac.webkit.org/changeset/246397>
Antoine Quint
Comment 11 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.
WebKit Commit Bot
Comment 12 2019-06-13 12:04:50 PDT
Re-opened since this is blocked by bug 198837
Shawn Roberts
Comment 13 2019-06-13 12:07:31 PDT
Rolled out in https://trac.webkit.org/changeset/246409/webkit Build logs were emailed to Myles
Shawn Roberts
Comment 14 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.
Myles C. Maxfield
Comment 15 2019-06-13 22:20:28 PDT
Committed r246427: <https://trac.webkit.org/changeset/246427>
Note You need to log in before you can comment on or make changes to this bug.