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] Add typing for field access expressions #9743

Merged
merged 1 commit into from
Feb 13, 2023
Merged

[WGSL] Add typing for field access expressions #9743

merged 1 commit into from
Feb 13, 2023

Conversation

tadeuzagallo
Copy link
Member

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

7b9d9eb

[WGSL] Add typing for field access expressions
https://bugs.webkit.org/show_bug.cgi?id=251842
rdar://105120646

Reviewed by Myles C. Maxfield.

Now that we can infer struct types, add support for access struct members. Support
for accessing vector members will follow in a separate patch.

* Source/WebGPU/WGSL/TypeCheck.cpp:
(WGSL::TypeChecker::check):
(WGSL::TypeChecker::visitStructMembers):
(WGSL::TypeChecker::visit):
(WGSL::TypeChecker::inferred):

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

977726b

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk
✅ 🧪 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 7, 2023
@tadeuzagallo tadeuzagallo added the WebGPU For bugs in WebGPU label Feb 7, 2023
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 1d2abec)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk
✅ 🧪 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
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Feb 7, 2023
@tadeuzagallo tadeuzagallo mentioned this pull request Feb 7, 2023
Merged
litherum
litherum approved these changes Feb 10, 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.

Right, super straightforward.

djg
djg reviewed Feb 12, 2023
View reviewed changes
Source/WebGPU/WGSL/TypeCheck.cpp Outdated
Comment on lines 231 to 232
if (std::holds_alternative<Struct>(*baseType)) {
auto& structType = std::get<Struct>(*baseType);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: This feels like the pattern in WGSL::AST that is written:

    if (is<Struct>(*baseType)) {
        auto& structType = downcast<Struct>(*baseType);

I would like to err on the side of consistency in the compiler. (Also, after looking at built times with -ftime-trace, I hate how slow std::variants are to compile)

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I can refactor to match that in a separate commit. (Just because it's already used in several other places in the type checker)

@tadeuzagallo tadeuzagallo force-pushed the eng/WGSL-Add-typing-for-field-access-expressions branch from 1d2abec to 977726b Compare February 13, 2023 10:51
@webkit-early-warning-system
Copy link
Collaborator

EWS run on current version of this PR (hash 977726b)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk
✅ 🧪 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 added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Feb 13, 2023
@tadeuzagallo
[WGSL] Add typing for field access expressions
7b9d9eb 
https://bugs.webkit.org/show_bug.cgi?id=251842
rdar://105120646

Reviewed by Myles C. Maxfield.

Now that we can infer struct types, add support for access struct members. Support
for accessing vector members will follow in a separate patch.

* Source/WebGPU/WGSL/TypeCheck.cpp:
(WGSL::TypeChecker::check):
(WGSL::TypeChecker::visitStructMembers):
(WGSL::TypeChecker::visit):
(WGSL::TypeChecker::inferred):

Canonical link: https://commits.webkit.org/260196@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/WGSL-Add-typing-for-field-access-expressions branch from 977726b to 7b9d9eb Compare February 13, 2023 13:32
@webkit-commit-queue
Copy link
Collaborator

Committed 260196@main (7b9d9eb): https://commits.webkit.org/260196@main

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

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Feb 13, 2023
@webkit-commit-queue webkit-commit-queue merged commit 7b9d9eb into WebKit:main Feb 13, 2023
webkit-early-warning-system pushed a commit to tadeuzagallo/WebKit that referenced this pull request Feb 13, 2023
@tadeuzagallo
[WGSL] Add typing for accessing fields of vector
1139dc9 
https://bugs.webkit.org/show_bug.cgi?id=251843
<rdar://problem/105121069>

Reviewed by Myles C. Maxfield.

Continuing from WebKit#9743, expand the typing for field access expressions to also
work with vectors.

* Source/WebGPU/WGSL/TypeCheck.cpp:
(WGSL::TypeChecker::visit):
(WGSL::TypeChecker::vectorFieldAccess):

Canonical link: https://commits.webkit.org/260200@main
@tadeuzagallo tadeuzagallo deleted the eng/WGSL-Add-typing-for-field-access-expressions 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

@djg djg djg left review comments

@litherum litherum litherum approved these changes

@mwyrzykowski mwyrzykowski Awaiting requested review from mwyrzykowski mwyrzykowski is a code owner

@grorg grorg Awaiting requested review from grorg

Assignees

@tadeuzagallo tadeuzagallo

Labels
WebGPU For bugs in WebGPU
Projects
None yet
Milestone
No milestone
6 participants
@tadeuzagallo @webkit-early-warning-system @webkit-commit-queue @djg @litherum @webkit-ews-buildbot