Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WGSL] Allow early compilation without hints #9441

Merged
merged 1 commit into from Feb 2, 2023
Merged

[WGSL] Allow early compilation without hints #9441

merged 1 commit into from Feb 2, 2023

Conversation

tadeuzagallo
Copy link
Member

@tadeuzagallo tadeuzagallo commented Feb 1, 2023 •
edited by webkit-early-warning-system

53a227e

[WGSL] Allow early compilation without hints
https://bugs.webkit.org/show_bug.cgi?id=251494
rdar://104908379

Reviewed by Myles C. Maxfield.

This is just a temporary workaround so we can start passing some CTS tests.
For now, since we are not yet using the layout hints, we can just try to
compile upfront even without them. Later we'll wire this up correctly so that
if no hints are provide the compilation is deferred.

* Source/WebGPU/WebGPU/ShaderModule.mm:
(WebGPU::Device::createShaderModule):

Canonical link: https://commits.webkit.org/259747@main

0a5dd89

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 🧪 merge ✅ 🛠 watch-sim

@tadeuzagallo tadeuzagallo self-assigned this Feb 1, 2023
@tadeuzagallo tadeuzagallo added the WebGPU For bugs in WebGPU label Feb 1, 2023
@webkit-early-warning-system
Copy link
Collaborator

EWS run on current version of this PR (hash 0a5dd89)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 🧪 merge ✅ 🛠 watch-sim

mwyrzykowski
mwyrzykowski approved these changes Feb 1, 2023
View reviewed changes
Copy link
Contributor

@mwyrzykowski mwyrzykowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should do this until we start failing tests because of it :)

litherum
litherum approved these changes Feb 2, 2023
View reviewed changes
Copy link
Contributor

@litherum litherum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't the hints necessary to generate code? If the goal is do something quickly so we can start implementing the compiler, it seems we should go the other direction and always defer compilation instead of never deferring compilation... That way we'll be able to emit correct bind group code.

@tadeuzagallo tadeuzagallo added the merge-queue Applied to send a pull request to merge-queue label Feb 2, 2023
@tadeuzagallo
Copy link
Member Author

I'm landing it for now to unblock the tests, as it's already how we're testing, but I'll prioritize moving the other direction.

@tadeuzagallo
[WGSL] Allow early compilation without hints
53a227e 
https://bugs.webkit.org/show_bug.cgi?id=251494
rdar://104908379

Reviewed by Myles C. Maxfield.

This is just a temporary workaround so we can start passing some CTS tests.
For now, since we are not yet using the layout hints, we can just try to
compile upfront even without them. Later we'll wire this up correctly so that
if no hints are provide the compilation is deferred.

* Source/WebGPU/WebGPU/ShaderModule.mm:
(WebGPU::Device::createShaderModule):

Canonical link: https://commits.webkit.org/259747@main
@webkit-early-warning-system webkit-early-warning-system force-pushed the eng/WGSL-Allow-early-compilation-without-hints branch from 0a5dd89 to 53a227e Compare February 2, 2023 10:46
@webkit-commit-queue
Copy link
Collaborator

Committed 259747@main (53a227e): https://commits.webkit.org/259747@main

Reviewed commits have been landed. Closing PR #9441 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system merged commit 53a227e into WebKit:main Feb 2, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Feb 2, 2023
webkit-early-warning-system pushed a commit to tadeuzagallo/WebKit that referenced this pull request Feb 7, 2023
@tadeuzagallo
[WGSL] Enable deferred compilation of compute shaders
f96ecde 
https://bugs.webkit.org/show_bug.cgi?id=251785
<rdar://problem/105075787>

Reviewed by Myles C. Maxfield.

In WebKit#9441 we allowed early compilation of shaders even when no hints were provided.
That was a workaround to start passing some of the CTS tests and unblock testing
the API. Now we remove that workaround and add the necessary functionality to defer
the compilation until the pipeline creation if no hints were provided.

* Source/WebGPU/WGSL/EntryPointRewriter.cpp:
(WGSL::EntryPointRewriter::EntryPointRewriter):
(WGSL::EntryPointRewriter::rewrite):
(WGSL::EntryPointRewriter::takeEntryPointInformation):
(WGSL::EntryPointRewriter::collectParameters):
(WGSL::EntryPointRewriter::checkReturnType):
(WGSL::EntryPointRewriter::appendBuiltins):
(WGSL::rewriteEntryPoints):
* Source/WebGPU/WGSL/EntryPointRewriter.h:
* Source/WebGPU/WGSL/MangleNames.cpp:
(WGSL::NameManglerVisitor::NameManglerVisitor):
(WGSL::NameManglerVisitor::run):
(WGSL::mangleNames):
* Source/WebGPU/WGSL/MangleNames.h:
* Source/WebGPU/WGSL/WGSL.cpp:
(WGSL::prepareImpl):
(WGSL::prepare):
* Source/WebGPU/WGSL/WGSL.h:
* Source/WebGPU/WebGPU/ShaderModule.mm:
(WebGPU::Device::createShaderModule):

Canonical link: https://commits.webkit.org/259954@main
@tadeuzagallo tadeuzagallo deleted the eng/WGSL-Allow-early-compilation-without-hints branch May 17, 2023 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@litherum litherum litherum approved these changes

@mwyrzykowski mwyrzykowski mwyrzykowski approved these changes

@djg djg Awaiting requested review from djg

@grorg grorg Awaiting requested review from grorg

Assignees

@tadeuzagallo tadeuzagallo

Labels
WebGPU For bugs in WebGPU
Projects
None yet
Milestone
No milestone
5 participants
@tadeuzagallo @webkit-early-warning-system @webkit-commit-queue @litherum @mwyrzykowski