| Summary: | [WHLSL] Hook up compute | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Myles C. Maxfield <mmaxfield> | ||||||
| Component: | WebGPU | Assignee: | 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
Myles C. Maxfield
2019-06-06 22:14:17 PDT
Created attachment 371561 [details]
WIP
*** Bug 198001 has been marked as a duplicate of this bug. *** *** Bug 198002 has been marked as a duplicate of this bug. *** Created attachment 371612 [details]
Patch
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 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 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. Committed r246396: <https://trac.webkit.org/changeset/246396> Committed r246397: <https://trac.webkit.org/changeset/246397> 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. Re-opened since this is blocked by bug 198837 Rolled out in https://trac.webkit.org/changeset/246409/webkit Build logs were emailed to Myles 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. Committed r246427: <https://trac.webkit.org/changeset/246427> |