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.

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

Add Pipeline Overrides for workgroup_size #6635

Merged
merged 15 commits into from
Dec 6, 2024
Merged

Conversation

kentslaney
Copy link
Contributor

@kentslaney kentslaney commented Nov 30, 2024

Connections
resolves #4450

Description
The WebGPU spec allows for pipeline overrides of workgroup_size. This is achieved by adding a workgroup_size_overrides field to crate::EntryPoint and registering any override expressions in workgroup_size as an override with the name __workgroup_size_{[012]} since identifiers aren't allowed to start with two underscores in a row. From there, they are resolved to constants with the other overrides and any Some fields in workgroup_size_overrides replace the corresponding workgroup_size.

Testing
This change adds an integration test via wgpu_test, which checks that the workgroup size equals the initialization value by default and that it changes with pipeline overrides.

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
    • --target wasm32-unknown-emscripten
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file.

@kentslaney kentslaney requested a review from a team as a code owner November 30, 2024 22:39
@kentslaney kentslaney requested a review from a team as a code owner December 1, 2024 21:06
Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Some nits on the integration tests, looks good on the wgpu side.

tests/tests/shader/workgroup_size_overrides.rs Outdated Show resolved Hide resolved
tests/tests/shader/workgroup_size_overrides.rs Outdated Show resolved Hide resolved
Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Tests look good :) Need a naga review still

naga/src/back/pipeline_constants.rs Outdated Show resolved Hide resolved
naga/src/lib.rs Outdated Show resolved Hide resolved
@cwfitzgerald
Copy link
Member

Don't forget to re-request review when things are addressed!

@cwfitzgerald cwfitzgerald requested a review from teoxoy December 4, 2024 07:42
Copy link
Member

@teoxoy teoxoy left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@teoxoy teoxoy merged commit b56960b into gfx-rs:trunk Dec 6, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support override-expression for workgroup_size
3 participants